rtorr / d2h

Quick git diff to html
0 stars 0 forks source link

Possible fully working solution #1

Closed rtfpessoa closed 7 years ago

rtfpessoa commented 7 years ago

Hi @rtorr

Just noticed you are using diff2html here to have a cli.

Just passed by to let you know, that you have a fully working cli that uses diff2html named rtfpessoa/diff2html-cli

Usage:

npm install -g diff2html-cli
diff2html -i file -- path/to/your/file

Let me know if it was useful and if you have any suggestions.

rtorr commented 7 years ago

@rtfpessoa does it work on giant diffs? That is the specific problem this tool fixes and it was just for a one off case at work

rtfpessoa commented 7 years ago

@rtorr maybe I misunderstood this, but I cannot see any change in the file index.js that might have a different behaviour.

Can you be more precise about what makes it more performant?

rtorr commented 7 years ago

@rtfpessoa Your cli tool does not handle large diffs gracefully, so I made this one off script just for this one purpose (I don't plan on marketing it over your tool, so don't worry). I needed to just get something working quick for some coworkers.

Try out these steps.

git clone git@github.com:facebook/react.git
cd react
git diff dae3043897ef581cf399e688739fb58d23b86c7d 1d4c1abd984bef0ba47f57d0f04abab387952a0f > tmp.diff
d2h -f tmp.diff
open tmp.diff.html 

now for diff2html

diff2html -i file -- tmp.diff

gives this error.

<--- Last few GCs --->

    5901 ms: Scavenge 1055.5 (1440.7) -> 1055.4 (1440.7) MB, 6.0 / 0 ms [allocation failure].
    5907 ms: Scavenge 1055.6 (1440.7) -> 1055.4 (1440.7) MB, 6.2 / 0 ms [allocation failure].
    5914 ms: Scavenge 1055.4 (1440.7) -> 1055.6 (1440.7) MB, 6.6 / 0 ms [allocation failure].
    6170 ms: Mark-sweep 1055.6 (1440.7) -> 1055.4 (1440.7) MB, 256.0 / 0 ms [last resort gc].
    6419 ms: Mark-sweep 1055.4 (1440.7) -> 1055.4 (1440.7) MB, 248.7 / 0 ms [last resort gc].

<--- JS stacktrace --->

==== JS stack trace =========================================

Security context: 0x30c80b8c9e31 <JS Object>
    2: levenshtein [/Users/rtorruellas/.nvm/versions/node/v6.3.1/lib/node_modules/diff2html-cli/node_modules/diff2html/src/rematch.js:~24] [pc=0x287c08f47567] (this=0x30c80b8e15d9 <JS Global Object>,a=0x391d62360a69 <Very long string[32008]>,b=0x391d62360a91 <Very long string[32021]>)
    3: /* anonymous */ [/Users/rtorruellas/.nvm/versions/node/v6.3.1/lib/node_modules/diff2html-cli/node_module...

FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory
 1: node::Abort() [/Users/rtorruellas/.nvm/versions/node/v6.3.1/bin/node]
 2: node::FatalException(v8::Isolate*, v8::Local<v8::Value>, v8::Local<v8::Message>) [/Users/rtorruellas/.nvm/versions/node/v6.3.1/bin/node]
 3: v8::internal::V8::FatalProcessOutOfMemory(char const*, bool) [/Users/rtorruellas/.nvm/versions/node/v6.3.1/bin/node]
 4: v8::internal::Factory::NewFillerObject(int, bool, v8::internal::AllocationSpace) [/Users/rtorruellas/.nvm/versions/node/v6.3.1/bin/node]
 5: v8::internal::Runtime_AllocateInTargetSpace(int, v8::internal::Object**, v8::internal::Isolate*) [/Users/rtorruellas/.nvm/versions/node/v6.3.1/bin/node]
 6: 0x287c08a06338
 7: 0x287c08e41bce
 8: 0x287c08f47567
 9: 0x287c08e4d139

It probably has to do with you using writefilesync. https://github.com/rtfpessoa/diff2html-cli/blob/master/src/utils.js#L55

I didn't have time to patch your work so I made this one off thing.

rtfpessoa commented 7 years ago

@rtorr no problem with making this tool. That is what OSS is for. I just wanted to understand the change.

Thanks for the guide on how to replicate. I will take a look.

rtfpessoa commented 7 years ago

@rtorr just tested it locally to confirm that is a problem of the matching algorithm.

For large files specially the ones with huge lines it can get lost in the memory. So it is advisable to disable it.

In this case if you try diff2html --lm none -- dae3043897ef581cf399e688739fb58d23b86c7d 1d4c1abd984bef0ba47f57d0f04abab387952a0f it should work as you would expect.

btw I am always open for bug reports in the repo. Let me know if you find anything else.

I maybe need to make the defaults less sensitive in this case :smile:

rtorr commented 7 years ago

@rtfpessoa Or just throw a better error :D

Will update the README to recommend your project