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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 12, 2025, 17:08 UTC
Message-ID: <xmqq8qqbum49.fsf@gitster.g>
In-Reply-To: <081272b5-b035-47a6-a951-eb923a9a5833@gmail.com>

Illia Bobyr <illia.bobyr@gmail.com> writes:

Show 5 quoted lines

> 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).

Show 7 quoted lines

>> 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.

Show 5 quoted lines

>> 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.