Re: [PATCH 2/2] branch, for-each-ref: add option to omit empty lines

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 31, 2023, 15:57 UTC
Message-ID: <xmqq355kj4mf.fsf@gitster.g>
In-Reply-To: <20230331083213.12013-1-oystwa@gmail.com>

Øystein Walle <oystwa@gmail.com> writes:

Show 14 quoted lines

>> > +             if (omit_empty_lines && !format.format) {
>> > +                     error("--omit-empty-lines without --format does not make sense");
>> > +                     usage_with_options(builtin_branch_usage, options);
>> > +             }
>>
>> Does it not make sense?  With the default format, it may happen that
>> there will be no empty line so there is nothing to omit, but I do
>> not see a strong reason to forbid the request like this.
>
> ... it's perfectly fine by me to allow --omit-empty when the user has
> not specified their own format. I added this merely as guidance for the
> user. For example, Git will bail out with a similar message if the user
> tries to unshallow a repository that is already complete, which I assume
> is technically not a problem.

It is not just technically a problem but from the end user's point of view a misguided message. If the end result is in the shape of desired state after the command completes, there shouldn't be an error() to stop the user. Informational "the history is now fully complete---by the way, it was already so before I started working" may be OK. It probably should be fixed, instead of being modelled after to spread the mistake to new features, like this patch does.

Thanks.