Sorry for the late reply,
Junio C Hamano <gitster@xxxxxxxxx> writes:
> Matthieu Moy <Matthieu.Moy@xxxxxxx> 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@xxxxxxx>
>> ---
>
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html