Subject: Re: [PATCH v2 1/2] maintenance: add 'unregister --force'

From: Junio C Hamano <[email-masked]>
Date: Mon, 26 Sep 2022 12:17:00 -0700

Derrick Stolee <[email-masked]> writes:

> On 9/26/2022 11:39 AM, Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Mon, Sep 26 2022, Derrick Stolee wrote:
>> 
>>> On 9/23/2022 9:08 AM, SZEDER Gábor wrote:
>>>> On Thu, Sep 22, 2022 at 01:37:16PM +0000, Derrick Stolee via GitGitGadget wrote:
>>>>>  static int maintenance_unregister(int argc, const char **argv, const char *prefix)
>>>>>  {
>>>>> +	int force = 0;
>>>>>  	struct option options[] = {
>>>>> +		OPT_BOOL(0, "force", &force,
>>>>> +			 N_("return success even if repository was not registered")),
>>>>
>>>> This could be shortened a bit by using OPT__FORCE() instead of
>>>> OPT_BOOL().  OTOH, please make it a bit longer, and declare the option
>>>> with the PARSE_OPT_NOCOMPLETE flag to hide it from completion:
>>>
>>> Looks like I can do both like this:
>>>
>>> 		OPT__FORCE(&force,
>>> 			   N_("return success even if repository was not registered"),
>>> 			   PARSE_OPT_NOCOMPLETE),
>> 
>> I don't think PARSE_OPT_NOCOMPLETE is appropriate here. Yes we use it
>> for most of --force, but in some non-destructive cases (e.g. "add") we
>> don't.
>> 
>> This seems to be such a case, we'll destroy no data or do anything
>> irrecoverable. It's really just a
>> --do-not-be-so-anal-about-your-exit-code option.
>
> I agree, so I wasn't completely sold on PARSE_OPT_NOCOMPLETE. I'll use
> your vote to not add that option.

I am perfectly OK with that.  Given that "git reset --hard" is not
given nocomplete option, I do not see much point in "destructive
ones are not completed" guideline in practice anyway.  After all,
"add --force" would be destructively removing the object name
recorded for the path previously.

Thanks for carefully thinking the UI remifications through.