tummychow / git-absorb

git commit --fixup, but automatic
https://crates.io/crates/git-absorb
BSD 3-Clause "New" or "Revised" License
3.39k stars 58 forks source link

Fixes up a commit adjacent to the line being changed #15

Open nickolay opened 5 years ago

nickolay commented 5 years ago

After fixing #6, I noticed another issue with the testcase from that issue.

In the testcase the commit to absorb is removing all instances of #[cfg(feature = "rsa_keygen")] and there is an intermediate commit adding a function with that attribute:

83      }
84   
85      #[cfg(feature = "rsa_keygen")]
86   +  pub fn limbs_even_add_one(r: &mut [Limb]) {
     +      // ...
91   +  }
92   +  
93   +  #[cfg(feature = "rsa_keygen")]
94      pub fn limbs_count_low_zero_bits(v: &[Limb]) -> u16 {

git-absorb decides to fix up this intermediate commit with both the hunk removing the cfg attribute at line 85 and at line 93, while arguably line 85 removal doesn't belong to the intermediate commit, as line 85 wasn't added by the commit.

It's clear why git-absorb does it, and if it produced a fixup for an older commit introducing line 85, that fixup would conflict with the intermediate commit. Still, git-absorb could:

tummychow commented 5 years ago

hmm... a very interesting problem. sounds like the desired behavior here would "don't absorb this hunk into this commit, if doing so would widen the range of lines affected by the commit".

i'd probably put this behavior and the current behavior behind flags (strawperson names: --conservative for this, --aggressive for the existing behavior). the default behavior would be the more conservative option, since you can always rerun with more aggressive behavior afterwards. it would leave the hunk in the index while printing an appropriate log line.

i also wonder if this case can be handled better by hg-absorb's linelog-based strategy, but that's a much larger and more complicated consideration