Sorry for the late reply,

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

> Matthieu Moy <matthieu....@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....@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.

> 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.

>>  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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to