On Mon, Aug 02, 2021 at 09:34:32PM -0500, heapcrash heapcrash wrote: > Thanks all for the feedback! I'll try to address it below: > > > Two other places may be diff hunk headers and --diff-words output, I think. > > I didn't think of those. Does this mean that diffs generated with a > given e.g. diff=cpp configuration might not apply cleanly if run on > some other user's system without that setting? No, because the text in the hunk headers is purely cosmetic. They are just for humans, and applying the patch ignores them. The word-diffs are likewise just meant for human consumption, and can't be applied at all. > > It makes emitting the diffs take more CPU, but the same is true of other > > options like colorMoved etc, so that in itself is not a dealbreaker. > > I didn't think of this scenario, that it would add CPU time even > without -W/--function-context/--show-function. I'd definitely be fine > with preserving the current behavior in these cases (more below). I think the CPU increase is negligible. Either way, the funcname header involves walking backwards through the file applying a pattern to see if a line is a good candidate. The type-specific patterns are a little more complicated, but I doubt the difference is even measurable in practice. If we are going to have defaults for extension-to-diff mapping, I would prefer apply them consistently (so for -W, but also for funcnames, word-diffs, etc). That is both easier to implement and easier to explain to users (and assuming our mapping is accurate, more likely to be what they want). > Another case I didn't think of in the original post is with the "git > log -L:funcname:path/to/file.cpp" option, which tracks changes within > a function over time. Having "better" default function boundary > detection here would also be useful. Yes, I think this uses the same diff funcname patterns, so once we have default mappings, it would start to use them. -Peff
