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

Sorry for the late reply,

Junio C Hamano [off-list ref] writes:

Matthieu Moy [off-list ref] writes:

quoted

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

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.

quoted

 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/