Re: [PATCH 2/2] add: add a newline at the end of pathless 'add [-u|-A]' warning

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Apr 2, 2013, 14:43 UTC
Message-ID: <vpqehetgfh0.fsf@grenoble-inp.fr>
In-Reply-To: <7v7gldc44p.fsf@alter.siamese.dyndns.org>

Sorry for the late reply,

Junio C Hamano <gitster@pobox.com> writes:

Show 11 quoted lines

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> When the commands give an actual output (e.g. when ran with -v), the
>> output is visually mixed with the warning. The newline makes the actual
>> output more visible.
>>
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>> ---
>
> It would have been easier to immediately understand what is going on
> if you said "blank line" instead of "newline" ;-)

Indeed.

> An obvious issues is what if user does not run with "-v" or if "-v"
> produces no results.  We will be left with an extra blank line at
> the end.

Right, but displaying the blank line only when there's an actual output does not seem easy, and I'd rather avoid too much damage in the code for a warning which is only temporary.

Show 9 quoted lines

> I suspect that the true reason why the warning does not stand out
> and other output looks mixed in it may be because we only prefix the
> first line with the "warning: " label.  In the longer term, I have a
> feeling that we should be showing something like this instead:
>
>     $ cd t && echo >>t0000*.sh && git add -u -v
>     warning: The behavior of 'git add --update (or -u)' with no path ar...
>     warning: subdirectory of the tree will change in Git 2.0 and should...
>     warning: To add content for the whole tree, run:

I personnally do not like this kind of output, the "warning:" on the 2nd and 3rd lines break the flow reading the message. But that's probably a matter of taste.

> using a logic similar to what strbuf_add_commented_lines() and
> strbuf_add_lines() use.

This would mean changing the warning() function, which would change all warnings.

I'm fine with either dropping my patch or applying it as-is (with s/newline/blank line/ in the commit message); a bit reluctant to changing the output of warning(...), but that's an option if other people like it.

Show 18 quoted lines

>>  builtin/add.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/builtin/add.c b/builtin/add.c
>> index ab1c9e8..620bf00 100644
>> --- a/builtin/add.c
>> +++ b/builtin/add.c
>> @@ -344,7 +344,7 @@ static void warn_pathless_add(const char *option_name, const char *short_name) {
>>  		  "  git add %s .\n"
>>  		  "  (or git add %s .)\n"
>>  		  "\n"
>> -		  "With the current Git version, the command is restricted to the current directory."),
>> +		  "With the current Git version, the command is restricted to the current directory.\n"),
>>  		option_name, short_name,
>>  		option_name, short_name,
>>  		option_name, short_name);
>
>

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/