Re: [PATCH] branch: do not fail a no-op --edit-desc

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 29, 2022, 22:26 UTC
Message-ID: <xmqq7d1lom12.fsf@gitster.g>
In-Reply-To: <ba8a503b-76e2-5de9-1082-3b4c6ecd0cc3@gmail.com>

Rubén Justo <rjusto@gmail.com> writes:

Show 9 quoted lines

> Let me try again, I think my review was not good :-)
>
> On 28/9/22 21:15, Junio C Hamano wrote:
>> In a repository on a branch without branch description, try running
>
> It is a bit confusing the construction "a repository on a branch
> without branch description" as "branch" have "repository" inherent.
> So "On a branch without description.." holds the same meaning with less
> distracting words.

Ah, no, I did not mean a repository is on any branch.  I meant

    Go into a repository and be on a branch.  That branch has to be
    without branch description set.  Now, try running these...

I think the easiest clarification is to drop "In a repository" altogether.

> But.. do we want to implement this this way? Maybe we will have to
> implement on purpose this feature in some future refactorization?

Absolutely.  It is simpler and there is not much downside.

> And.. the message does not make it clear the situation: if there is
> a previous description, will clear; if not, will keep.

If there is a previous description, then we use the behaviour we have had for ages (i.e. will remove). If there is not a previous description, then we use the new behaviour (i.e. not attempt to remove and hence we do not show an error). Either way, the end result is "the user indicated they do not want description by giving us an empty edit result. After the command exits, the branch has no description".

> If we want that feature, should we test for it? (do not take
> the snippet as tested...):

Notice the air-quotes around "feature" ;-)

The official stance is "if it hurts, do not do it then". The side discussion about TOCTOU was to see how much hurt we are making the user responsible for, and explaining that it is not that much.