Alexander Miseler [off-list ref] writes:
It's a bit intimidating for a newbie to chime in on a discussion between the
creator and the maintainer, but:
IMHO the biggest problem here isn't the incorrectly, but rather the silently.
Reducing the chance of guessing incorrectly is good, but git-am still has to
guess sometimes and it should warn/inform the user when it does that.
(Please don't cull Cc line).
No need to feel intimidated.
The patch under discussion was merely "first things first--let's fix the
obviously wrong case that can be fixed without regression".
Try implementing that warning logic, and using it in real-life projects.
You don't actually have to _code_ it, but merely imagining how it would
work and perform would be sufficient for you to realize that it would be
quite expensive (you need to find all the possible mismatches, essentially
scanning the whole file), and worse yet, it would be annoyingly noisy with
many false positives, because in many real-life projects, end of function
tends to match the problematic pattern that triggered this discussion
quite often even without patches that introduce more of the pattern.
Unless you can reduce the false hits to manageable levels, such a warning
is not very useful (it would be useful as a lame excuse "we warned, but
you took the suspicious result", but that does not help the users).
In short, Linus and I both know what you are talking about, and we may
revisit that issue later, but the thing is that it would not be very
pleasant, and not something that can be done in one sitting during a
single discussion thread on the list.
'Twas brillig, and Junio C Hamano at 04/03/11 21:33 did gyre and gimble:
In short, Linus and I both know what you are talking about, and we may
revisit that issue later, but the thing is that it would not be very
pleasant, and not something that can be done in one sitting during a
single discussion thread on the list.
As a simple option to avoid that, how about just printing out (by
default) the line offsets if hunks don't apply 100% cleanly? This would
at least alert you to the fact that some fixups were needed.
Just a thought...
--
Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/
Day Job:
Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
Mageia Contributor [http://www.mageia.org/]
PulseAudio Hacker [http://www.pulseaudio.org/]
Trac Hacker [http://trac.edgewall.org/]
On 04.03.2011 22:33, Junio C Hamano wrote:
(Please don't cull Cc line).
Sorry. I used the nice gmane web interface and hoped that it keeps the CC intact, which it apparently doesn't. I guess i
will go old school now and use the mailing list via actual emails :)
Try implementing that warning logic, and using it in real-life projects.
You don't actually have to _code_ it, but merely imagining how it would
work and perform would be sufficient for you to realize that it would be
quite expensive (you need to find all the possible mismatches, essentially
scanning the whole file), and worse yet, it would be annoyingly noisy with
many false positives, because in many real-life projects, end of function
tends to match the problematic pattern that triggered this discussion
quite often even without patches that introduce more of the pattern.
Unless you can reduce the false hits to manageable levels, such a warning
is not very useful (it would be useful as a lame excuse "we warned, but
you took the suspicious result", but that does not help the users).
In short, Linus and I both know what you are talking about, and we may
revisit that issue later, but the thing is that it would not be very
pleasant, and not something that can be done in one sitting during a
single discussion thread on the list.
Understood. On a side note: if this problem is tackled it might be sensible to add a heuristic to git format-patch that
increases the context size for hunks that are likely to be ambiguous. "Likely to be ambiguous" is of course a problem in
itself but even a less than perfect detection might be helpful and it would suffer less from some of the aforementioned
problems, like noisiness/false hits, which would just increase the patch size instead of harassing the user.