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 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 t*.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
Re: [PATCH 2/2] add: add a newline at the end of pathless 'add [-u|-A]' warning
Matthieu Moy matthieu@grenoble-inp.fr writes: I'm fine with either dropping my patch or applying it as-is (with s/newline/blank line/ in the commit message). OK; let's insert it immediately after e24afab09137 (add: make pathless 'add [-u|-A]' warning a file-global function, 2013-03-19), like the attached. I'd prefer to spell this die(...\n ); over die(...\n); as the latter _looks_ as if the author didn't know die() gives us line termination. The former hopefully is more explicit that we do want an empty line at the end. commit a8ed21a59219e8fe81b76ed0961641555f25cdad Author: Matthieu Moy matthieu@imag.fr Date: Mon Mar 11 09:01:33 2013 +0100 add: add a blank line at the end of pathless 'add [-u|-A]' warning When the commands give an actual output (e.g. when ran with -v), the output is visually mixed with the warning. An additional blank line makes the actual output more visible. Signed-off-by: Matthieu Moy matthieu@imag.fr Signed-off-by: Junio C Hamano gits...@pobox.com diff --git a/builtin/add.c b/builtin/add.c index a4028ee..db02233 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -57,7 +57,9 @@ static void warn_pathless_add(void) 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_with_implicit_dot, short_option_with_implicit_dot, option_with_implicit_dot, short_option_with_implicit_dot, option_with_implicit_dot, short_option_with_implicit_dot); -- 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
Re: [PATCH 2/2] add: add a newline at the end of pathless 'add [-u|-A]' warning
Junio C Hamano gits...@pobox.com writes: diff --git a/builtin/add.c b/builtin/add.c index a4028ee..db02233 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -57,7 +57,9 @@ static void warn_pathless_add(void) 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, + ), Looks good. Thanks, -- 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
[PATCH 2/2] add: add a newline at the end of pathless 'add [-u|-A]' warning
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
---
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);
--
1.8.2.rc3.16.g0a33571.dirty
--
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
Re: [PATCH 2/2] add: add a newline at the end of pathless 'add [-u|-A]' warning
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 ;-)
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.
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 t*.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:
warning:
warning: git add --update :/
warning: (or git add -u :/)
warning:
warning: To restrict the command to the current directory, run:
warning:
warning: git add --update .
warning: (or git add -u .)
warning:
warning: With the current Git version, the command is restricted to...
add 't/t-basic.sh'
using a logic similar to what strbuf_add_commented_lines() and
strbuf_add_lines() use.
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);
--
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
