Subject: Re: [PATCH v3 1/1] diff: --patch-{modifies,grep} arg names for -S and -G

From: Junio C Hamano <[email-masked]>
Date: Wed, 12 Feb 2025 09:08:22 -0800

Illia Bobyr <[email-masked]> writes:

> My thinking is that as long version names improve readability, it also
> applies
> to the test code.  When I see a short option, I often have to check
> the manual
> to remember what exactly does it do.

But by now due to enough exposure, you have committed them in your
memory, no? ;-)

> But, I think, I understand your point of view as well.

Yup, if the options were introduced with long and short forms at the
same time and the tests were written at the same time or shortly
after their introduction, I'd agree that using longer form more may
be beneficial, since there is nobody who is already familier with
either of the forms.  But at this point after 20 years, swapping one
for the other is mostly unnecessary churn, I would have to say (and
I do not particularly want to having to repeat saying the same thing
again).

>> OK.  NOte that this says <regex>.  We may want to have a separate
>   clean-up
>> patch so that Documentation/gitdifcore.txt that used <regular-expression>
>> and the placeholder used here match.
>
> Makes sense.
> I've added this fix as patch 5 in v5.

I'd rather see these "fixes to existing anomalies" done totally
outside of this series.  IOW, I'd prefer to either (1) get the
series done with the minimally necessary changes first and then
after the dust settles from merging that to 'master', see these "oh
we noticed these issues while working on the other series that has
now completed" issues addressed, or (2) do the clean-up of existing
anomalies first as a separate series, and then after the dust
settles for the clean-up, do the proposed addition of longform as a
separate series.  I have slight preference to (1), simply because
nobody complained on these small anomalies for the past 20 years ;-)
but I can also go with "preliminary clean-up first" route.

>> This is an unrelated change that should not be in this patch.  If
>> you want to modify it, please do it in a separate clean-up patch,
>> just like the above <regex> vs <regular-expression> change.
>
> Split it into patch 2 in v5.

Again, when I said "unrelated", I meant that I want them to be
treated as unrelated changes, addressed outside of this series,
either in a preliminary clean-up, or after-the-dust-settles
clean-up.

Thanks.