Subject: Re: [PATCH 1/3] rebase.c: state preserve-merges has been removed

From: Junio C Hamano <[email-masked]>
Date: Thu, 26 May 2022 13:33:51 -0700

René Scharfe <[email-masked]> writes:

>>>  		OPT_SET_INT_F('p', "preserve-merges", &preserve_merges_selected,
>>> -			      N_("(DEPRECATED) try to recreate merges instead of "
>>> +			      N_("(REMOVED) try to recreate merges instead of "
>>>  				 "ignoring them"),
>>>  			      1, PARSE_OPT_HIDDEN),
>>>  		OPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),
>
> Hidden options are shown if you use --help-all instead of -h.
>
> OPT_SET_INT_F always sets the struct option member "argh" to NULL.  The
> string changed above is the "help" member, not "argh".

Good points.  I do think it is OK to say REMOVED in case --help-all
asks us to show everything, even though I wonder if we can leave it
there until we remove the "support" of noticing the user asking for
a now-removed feature.

>> So there's no point in changing this string, nor to have translators
>> focus on it, it'll never be used.
>>
>> This series shouldn't fix the general issue (which parse-options.c
>> should really be BUG()-ing about, after fixing the existing
>> occurances. But For this one we could just set this to have a string of
>> "" or something, only the string you're changing in 3/3 will be seen by
>> anyone.
>
> What is the general issue?

I am afraid to ask, after having learned to be worried about those
large rearchitecting projects Ævar talks about X-<.

> Anyway, the new help text explaining what the option once did is a bit
> confusing.  It would be better to focus on what it's doing now (nothing)
> and/or why we still have it (for backward compatibility), I think.

Do you mean that we should say "this option used to do such and such
but it is now a no-op" after "(REMOVED)" label, instead of the above
"this option does such and such"?  I think "(REMOVED)" is a strong
enough hint that lets us get away without saying "used to" and "but
it is now a no-op", so I can accept both.

Or do you mean we should say "(REMOVED) for backward compatibility,
does nothing but errors out"?  I would be less in faviour, then.
Those who are curious enough to ask --help-all would find it more
helpful if we said what it used to do.  Otherwise they wouldn't be
asking --help-all in the first place, no?