search

packsaddle / ruby-git_diff_parser

Parse `git diff` into patches and lines.
http://packsaddle.org
MIT License
33 stars 14 forks source link

Fix how line numbers are counted in Patch#removed_lines #183

Open joroshiba opened 6 years ago

joroshiba commented 6 years ago

Non removed lines should be counted towards the line number exclusively, test was simply incorrect previously. Manually checked against the diff to confirm this.

joroshiba commented 6 years ago

I have a branch ready which adds upon this to add the data I originally wanted (a memoized function which gives us the lines which those deleted lines would reside out in new file), plus solving my need (a function which given a line from the current file, tells you which line it resided at in the old file, if it existed).

This work is dependent on this branch, I can either include it in this PR or wait for this one to go through and then open up the branch or fold it into this PR. I think they are seperate issues, but would like to see both go in, so waiting till this gets in for now.

joroshiba commented 6 years ago

@sanemat

sanemat commented 6 years ago

Thanks, I'm in a vacation so after that I'll check this.

On Sat, Mar 3, 2018, 03:57 Jordan Oroshiba notifications@github.com wrote:

@sanemat https://github.com/sanemat

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/packsaddle/ruby-git_diff_parser/pull/183#issuecomment-370035440, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEmuJeqy4JlsalW_SBh0eJi_D_jqcvwks5taaQXgaJpZM4SC2N9 .

sandstrom commented 2 years ago

What's the status of this PR?

Anything missing from the PR author? Or are we waiting for review?

joroshiba commented 1 year ago

@sandstrom in all honesty, this is 4 years old and I should have done something 4 years ago. Other priorities, I had forked and moved on.

I think everyone can agree the behavior is better but I don't have the context really anymore on @cjoudrey to say whether or not those changes should be made, and I have no ability to merge. As far as I know the project I was working on which was pointing to my fork is still doing so, but I've moved on since then.

sandstrom commented 1 year ago

@joroshiba Alright, just curious since we ran into this a few weeks ago 😄

Thanks for the update!