Re: The git spring cleanup challenge

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Ævar

On 03/06/2021 13:31, Ævar Arnfjörð Bjarmason wrote:
> 
> On Thu, Jun 03 2021, Felipe Contreras wrote:
> 
>> Ævar Arnfjörð Bjarmason wrote:
>>> So I skipped the "disable most config", but for what it's worth I think
>>> I'd miss these the most, I couldn't pick just N favorites, sorry:
>>>
>>>   * diff.colorMoved=true: super useful, but I'd be vary of turning it on
>>>     by default in its current form. E.g. on gcc.git's changelog files it
>>>     has really pathological performance characteristics.

Would you be able say a bit more about this so I can try and reproduce 
it please. I'm working on some patches [1] to improve the performance of 
`diff --color-moved` and `diff --color-moved-ws` and it would be good to 
test them on a problematic repo. At the moment I diffing two releases of 
git to test performance on a large diff. I just cloned the last 18 
months of gcc.git and Changelog seems to just be appended to.

>> Very nice! I didn't know about it. I'll pick it for my third day.
> 
> It makes patch review a lot easier, and also integrates nicely with -w.

I find --color-moved-ws=allow-indentation-change helpful as well (it is 
quite a bit slower at the moment but I have some patches to bring it up 
to the speed of the other --color-moved modes.

[1] https://github.com/phillipwood/git/tree/wip/diff-color-moved-tweaks

> [...]
>>>   * commit.verbose=true: so you know what you're looking at in doing in
>>>     "git commit --amend".
>>
>> Aha! My alias had `commit -v` but I would want this on all commit
>> commands.
>>
>> Moreover, I was thinking on suggesting this by default. Who would it
>> hurt?
> 
> E.g. "git rebase -i" with "reword" now becomes a lot more verbose, I
> think it's a feature, but others might disagree.
> 
> It also exposes various edge cases around our parsing of the diff
> v.s. commit message content in our apply.c etc. code, e.g. say you want
> to blindly search-replace "diff" with "difference" in your commit
> messages. You'll now change the "diff --git" line to "difference --git",
> and now "commit" won't detect that it's the patch part anymore, and
> merge that diff into your commit message itself.
> 
> I can't remember if we pick up on "diff --git" exactly, IIRC, but
> anyway, whatever part of the format you need to screw with, the point
> stands. I've run into mistakes like that in the past, one recently made
> it to this ML.

I think the problem occurs if the scissors line gets messed up when 
editing the commit message

>[...] 
>>> I also have a bunch of aliases that would not be useful to a general
>>> audience, but which I find I can't live without, some of the most
>>> commonly used ones:
>>>
>>>      # Log with "less" n/p already going to the next/prev commit
>>>      log-psfd = "!f() { PAGER=\"less -p'^commit'\" git log -p --stat --full-diff $@; }; f"
>>
>> Very neat.
> 
> I think similar to your "git help xyz" patches having coloring, we
> really should consider things like that by default knowing that we're
> invoking "less". I.e. if we got over the notion that our job is just to
> throw data over the wall to "man" or the pager without any further
> tweaking or integration.

Speaking personally it is not that I think that we should just throw 
data over the all to "man" but that if colors are a good idea we should 
be thinking about the whole ecosystem and working with distributions or 
the man authors to ensure all programs and users benefit from it not 
just git.

Best Wishes

Phillip