Skip to content

Conversation

@pbu88
Copy link
Contributor

@pbu88 pbu88 commented Oct 20, 2015

Small performance improvement.

Also, I'm thinking it would be nice to support unified diffs (not only git diffs). That's what took me to this intermediate step (trying to refactor a little bit). Support for unified diffs is almost there, but some minor headers adaptations needs to be done.

This is very useful for sysadmins for example. A lot of time they are changing .conf files and I've found is better for them to read unified diffs than just telling them which line to add/change.

Hope this helps!

@rtfpessoa
Copy link
Owner

LGTM 👍

Thanks

rtfpessoa pushed a commit that referenced this pull request Oct 20, 2015
Extracts variables out of a loop
@rtfpessoa rtfpessoa merged commit cc858a2 into rtfpessoa:master Oct 20, 2015
@pbu88 pbu88 deleted the small_perf_improvement branch October 20, 2015 14:42
@rtfpessoa
Copy link
Owner

@pbu88 Looking forward for the unified diff support.
If you need any help with the code just tell me.
I am not much experienced with JS and Node, so good practices might be missing in some occasions. Feel free to warn me about any improvements I can do in the lib.

@pbu88
Copy link
Contributor Author

pbu88 commented Oct 21, 2015

Same here about Node :)

It shouldn't be hard. I was thinking about refactoring the logic for parsing git diff into its own method, then write another for unified diffs (which will use a lot of logic from the one already in git diff, only the headers are sort of different). Thanks for the support, I'll give it a try when I have some time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants