JSON files are so compact that git diffs are problematic

Discussion in 'RPG Maker MV Improvement Boards' started by cmdted, Mar 31, 2019.

  1. cmdted

    cmdted Villager Member

    Messages:
    6
    Likes Received:
    5
    First Language:
    English
    Primarily Uses:
    RMMV
    I am trying to work on an RMMV project with multiple developers with the project stored in github.

    We notice that since many of the json files that RMMV saves (e.g., data/System.json) are very compact and do not have line breaks, git has a lot of trouble generating readable diffs, merging, and resolving conflicts.

    I would request that RMMV write out its JSON files in a prettier format (e.g., by using JSON.stringify(data, null, 4)).

    As a work around, we wrote a simple cleaner script at https://github.com/cmdted/rmmv_tools/blob/master/src/cleanRMMV.js (see docs at https://github.com/cmdted/rmmv_tools/wiki/Setup#getting-sane-diffs) to help clean files before putting into git. Still, it would be better if RMMV just wrote nicer JSON output. If space is an issue the packaging process could always condense things, strip out line breaks and so on.

    Feedback welcome on the cleaner script and the request in general.

    Thanks.
     
    #1
  2. professorbeej

    professorbeej Villager Member

    Messages:
    29
    Likes Received:
    10
    Location:
    Alabama
    First Language:
    English
    Primarily Uses:
    RMMV
    I just came here to post about this very thing, as I was asking in the plugin request forums whether any plugins existed for this. I will totally check out the script you wrote and see if it solves our problems with the one-line diffs.

    You rock for this.

    And yes, please please please implement unminified JSON during development and minify during deploy.
     
    #2
    Aloe Guvner likes this.
  3. Aloe Guvner

    Aloe Guvner Walrus Veteran

    Messages:
    1,552
    Likes Received:
    977
    Location:
    USA
    First Language:
    English
    Primarily Uses:
    RMMV
    Probably the simplest thing would be to create a bash alias that calls that node script before staging/committing.

    There might be an interesting possibility with git hooks:
    https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks
    I might play with that and write a tutorial if I come up with anything good.
     
    #3
    cmdted and professorbeej like this.
  4. professorbeej

    professorbeej Villager Member

    Messages:
    29
    Likes Received:
    10
    Location:
    Alabama
    First Language:
    English
    Primarily Uses:
    RMMV
    That would make sense. Maybe one of the beautify scripts running with an ‘update’ hook (or ‘pre-receive’)?

    I’ve never used any kind of custom hooks like that before.
     
    #4
    Aloe Guvner likes this.
  5. cmdted

    cmdted Villager Member

    Messages:
    6
    Likes Received:
    5
    First Language:
    English
    Primarily Uses:
    RMMV
    That's a good idea. I think a pre-commit hook could work to do the beautify. Another alternative would be to have a pre-commit hook which does the beautify in a temp directory and then complains if the temp beautified code does not match the code being commited. That would be safer since the pre-commit hook would not directly modify your code.
     
    #5
    Aloe Guvner likes this.
  6. professorbeej

    professorbeej Villager Member

    Messages:
    29
    Likes Received:
    10
    Location:
    Alabama
    First Language:
    English
    Primarily Uses:
    RMMV
    I have no idea how to actually set up the Git Hooks, but I did run the node command, and it works like a charm, @cmdted.

    The only issue that I ran into at all was if I had any extra JS files in there (like I had a folder of generator character JSON files in the /data), the script would freeze up.

    Once I figured out the problem was on my end, the Maps, System, etc. are b-e-a-utiful.

    For real, thank you. You're a fantastic human being.
     
    #6
    cmdted likes this.
  7. inqs

    inqs Villager Member

    Messages:
    15
    Likes Received:
    1
    First Language:
    English
    Primarily Uses:
    RMMV
    deleted
     
    #7
  8. cmdted

    cmdted Villager Member

    Messages:
    6
    Likes Received:
    5
    First Language:
    English
    Primarily Uses:
    RMMV
    You're welcome and thank you for the compliment. I think the issue you are having is because the cleaner script didn't expect to see a directory. I can make it so that it goes into sub-directories of /data and cleans those too instead of dying. Will do when I get some time.
     
    #8
  9. professorbeej

    professorbeej Villager Member

    Messages:
    29
    Likes Received:
    10
    Location:
    Alabama
    First Language:
    English
    Primarily Uses:
    RMMV
    If you want to, that's a-okay, but this has made my life so much better, don't worry about it on my account. For real, my partner and I were getting so frustrated about the stuff like system.json and mapinfos that I was going to start doing some reeeeeeeeeeeally hackey stuff with cloning branches and copy/pasting from different directories.

    This fixes that. ;)
     
    #9
  10. cmdted

    cmdted Villager Member

    Messages:
    6
    Likes Received:
    5
    First Language:
    English
    Primarily Uses:
    RMMV
    Awesome! Yeah, it was bothering me a lot too. It made me learn the meaning `git log --full-history` :)

    Anyway, I fixed things to descend into sub-directories. I also add a test file so that if other people want to hack, they can do `npm run test` (after `npm install -g jest`) to have a simple test of the cleaner script.

    Let me know if the new version of the script at https://raw.githubusercontent.com/cmdted/rmmv_tools/master/src/cleanRMMV.js solves your problem. Also, if you want to add a star to the github repo, that might help others find it.
     
    #10
  11. Aloe Guvner

    Aloe Guvner Walrus Veteran

    Messages:
    1,552
    Likes Received:
    977
    Location:
    USA
    First Language:
    English
    Primarily Uses:
    RMMV
    #11

Share This Page