Re: [PATCH] builtin/add: add a missing newline to an stderr message
Jonathan Niederwrites: > I don't believe the force_mode without an 'x' provides a clear signal > to the end user. Perhaps you meant %cx? Indeed you are right. I think I saw Ramsay's v2 that has the 'x', so let's use that version. Thanks.
Re: [PATCH] builtin/add: add a missing newline to an stderr message
On 08/08/17 22:45, René Scharfe wrote: > Am 08.08.2017 um 23:36 schrieb Ramsay Jones: >> >> Signed-off-by: Ramsay Jones>> --- >> >> Hi Junio, >> >> I noticed this while looking into the t3700 failure on cygwin tonight. >> Also, I couldn't decide whether or not to add the i18n '_()' brackets >> around the message. In the end I didn't, but will happily add them >> if you think I should. >> >> Thanks! >> >> ATB, >> Ramsay Jones >> >> builtin/add.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/builtin/add.c b/builtin/add.c >> index e888fb8c5..385b53ae7 100644 >> --- a/builtin/add.c >> +++ b/builtin/add.c >> @@ -43,7 +43,7 @@ static void chmod_pathspec(struct pathspec *pathspec, int >> force_mode) >> continue; >> >> if (chmod_cache_entry(ce, force_mode) < 0) >> -fprintf(stderr, "cannot chmod '%s'", ce->name); >> +fprintf(stderr, "cannot chmod '%s'\n", ce->name); >> } >> } >> > > FYI: I brought this up yesterday in the original thread, along with a > few other observations: > > https://public-inbox.org/git/3c61d9f6-e0fd-22a4-68e0-89fd9ce9b...@web.de/ Ah, I missed that. Hmm, I just looked at the code in builtin/update-index.c. Yes, it would probably be a good idea to harmonize the messages - but just where did 'flip' come from? ;-) ATB, Ramsay Jones
Re: [PATCH] builtin/add: add a missing newline to an stderr message
Ramsay Jones wrote: > Signed-off-by: Ramsay JonesReviewed-by: Jonathan Nieder Thanks for catching it.
Re: [PATCH] builtin/add: add a missing newline to an stderr message
Junio C Hamano wrote: > René Scharfewrites: >>> diff --git a/builtin/add.c b/builtin/add.c >>> index e888fb8c5..385b53ae7 100644 >>> --- a/builtin/add.c >>> +++ b/builtin/add.c >>> @@ -43,7 +43,7 @@ static void chmod_pathspec(struct pathspec *pathspec, int >>> force_mode) >>> continue; >>> >>> if (chmod_cache_entry(ce, force_mode) < 0) >>> - fprintf(stderr, "cannot chmod '%s'", ce->name); >>> + fprintf(stderr, "cannot chmod '%s'\n", ce->name); >>> } >>> } >> >> FYI: I brought this up yesterday in the original thread, along with a >> few other observations: >> >> https://public-inbox.org/git/3c61d9f6-e0fd-22a4-68e0-89fd9ce9b...@web.de/ >> >> Not sure if the discussion can or should be revived after all this >> time, though; just sending patches like yours might be the way to go. > > Thanks, so it should become > > fprintf(stderr, "cannot chmod %c '%s'\n", force_mode, ce->name); > > in the final version to be queued? I don't believe the force_mode without an 'x' provides a clear signal to the end user. Perhaps you meant %cx? Thanks, Jonathan
Re: [PATCH] builtin/add: add a missing newline to an stderr message
René Scharfewrites: >> diff --git a/builtin/add.c b/builtin/add.c >> index e888fb8c5..385b53ae7 100644 >> --- a/builtin/add.c >> +++ b/builtin/add.c >> @@ -43,7 +43,7 @@ static void chmod_pathspec(struct pathspec *pathspec, int >> force_mode) >> continue; >> >> if (chmod_cache_entry(ce, force_mode) < 0) >> -fprintf(stderr, "cannot chmod '%s'", ce->name); >> +fprintf(stderr, "cannot chmod '%s'\n", ce->name); >> } >> } > > FYI: I brought this up yesterday in the original thread, along with a > few other observations: > > https://public-inbox.org/git/3c61d9f6-e0fd-22a4-68e0-89fd9ce9b...@web.de/ > > Not sure if the discussion can or should be revived after all this > time, though; just sending patches like yours might be the way to go. Thanks, so it should become fprintf(stderr, "cannot chmod %c '%s'\n", force_mode, ce->name); in the final version to be queued?
Re: [PATCH] builtin/add: add a missing newline to an stderr message
Am 08.08.2017 um 23:36 schrieb Ramsay Jones: > > Signed-off-by: Ramsay Jones> --- > > Hi Junio, > > I noticed this while looking into the t3700 failure on cygwin tonight. > Also, I couldn't decide whether or not to add the i18n '_()' brackets > around the message. In the end I didn't, but will happily add them > if you think I should. > > Thanks! > > ATB, > Ramsay Jones > > builtin/add.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/builtin/add.c b/builtin/add.c > index e888fb8c5..385b53ae7 100644 > --- a/builtin/add.c > +++ b/builtin/add.c > @@ -43,7 +43,7 @@ static void chmod_pathspec(struct pathspec *pathspec, int > force_mode) > continue; > > if (chmod_cache_entry(ce, force_mode) < 0) > - fprintf(stderr, "cannot chmod '%s'", ce->name); > + fprintf(stderr, "cannot chmod '%s'\n", ce->name); > } > } > FYI: I brought this up yesterday in the original thread, along with a few other observations: https://public-inbox.org/git/3c61d9f6-e0fd-22a4-68e0-89fd9ce9b...@web.de/ Not sure if the discussion can or should be revived after all this time, though; just sending patches like yours might be the way to go. René