[PATCH 0/2] Unify the style of user messages

[PATCH 0/2] Unify the style of user messages

[Vincent van Ravesteijn]

While working on the translations, I came accross these strings. There doesn't seem to be a rule on how these messages should be formatted (or there is, then I didn't look well enough).

To me it feels better to standardize the style, so I looked in the sourcecode for which style is used most and composed some guidelines to unify the messages:
    - messages start with a capital,
    - short messages do not end with a full stop,
    - paths, filenames, and commands are quoted by single quotes (if not separated by the normal text by a ':'),
    - 'could not' is used rather than 'cannot'. 

There could of course be much more changes, but I don't know whether these changes are appreciated. Also, the guidelines might be disputable. Maybe it is not the best timing because we are in rc phase and the people start to translate. On the other hand, we might better change it before all translations are finished.

The second patch makes a few string translatable. 

Vincent

[PATCH 1/2] Unification of user message strings

[Vincent van Ravesteijn]

From: Vincent van Ravesteijn <vfr@lyx.org>

Rewrite user messages to stick to a uniform style for all messages. From the surrounding code, the following guidelines were deduced:
- messages start with a capital,
- short messages do not end with a full stop,
- paths, filenames, and commands are quoted by single quotes (if not separated by the normal text by a ':'),
- 'could not' is used rather than 'cannot'.

Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>
---
 gpg-interface.c |    6 +++---
 grep.c          |    2 +-
 help.c          |    2 +-
 sequencer.c     |   24 ++++++++++++------------
 4 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 09ab64a..5e14a21 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -56,7 +56,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
 	args[3] = NULL;
 
 	if (start_command(&gpg))
-		return error(_("could not run gpg."));
+		return error(_("Could not run 'gpg'"));
 
 	/*
 	 * When the username signingkey is bad, program could be terminated
@@ -68,7 +68,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
 		close(gpg.in);
 		close(gpg.out);
 		finish_command(&gpg);
-		return error(_("gpg did not accept the data"));
+		return error(_("'gpg' did not accept the data"));
 	}
 	close(gpg.in);
 
@@ -79,7 +79,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
 	sigchain_pop(SIGPIPE);
 
 	if (finish_command(&gpg) || !len || len < 0)
-		return error(_("gpg failed to sign the data"));
+		return error(_("'gpg' failed to sign the data"));
 
 	/* Strip CR from the line endings, in case we are on Windows. */
 	for (i = j = bottom; i < signature->len; i++)
diff --git a/grep.c b/grep.c
index 190139c..6e0fef5 100644
--- a/grep.c
+++ b/grep.c
@@ -1277,7 +1277,7 @@ static int grep_source_load_sha1(struct grep_source *gs)
 	grep_read_unlock();
 
 	if (!gs->buf)
-		return error(_("'%s': unable to read %s"),
+		return error(_("%s: unable to read '%s'"),
 			     gs->name,
 			     sha1_to_hex(gs->identifier));
 	return 0;
diff --git a/help.c b/help.c
index 14eefc9..5ab076b 100644
--- a/help.c
+++ b/help.c
@@ -285,7 +285,7 @@ static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old)
 
 static const char bad_interpreter_advice[] =
 	N_("'%s' appears to be a git command, but we were not\n"
-	"able to execute it. Maybe git-%s is broken?");
+	"able to execute it. Maybe 'git-%s' is broken?");
 
 const char *help_unknown_cmd(const char *cmd)
 {
diff --git a/sequencer.c b/sequencer.c
index a37846a..1554173 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -156,10 +156,10 @@ static void write_message(struct strbuf *msgbuf, const char *filename)
 	int msg_fd = hold_lock_file_for_update(&msg_file, filename,
 					       LOCK_DIE_ON_ERROR);
 	if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
-		die_errno(_("Could not write to %s"), filename);
+		die_errno(_("Could not write to '%s'"), filename);
 	strbuf_release(msgbuf);
 	if (commit_lock_file(&msg_file) < 0)
-		die(_("Error wrapping up %s"), filename);
+		die(_("Error wrapping up '%s'"), filename);
 }
 
 static struct tree *empty_tree(void)
@@ -588,11 +588,11 @@ static void read_populate_todo(struct commit_list **todo_list,
 
 	fd = open(todo_file, O_RDONLY);
 	if (fd < 0)
-		die_errno(_("Could not open %s"), todo_file);
+		die_errno(_("Could not open '%s'"), todo_file);
 	if (strbuf_read(&buf, fd, 0) < 0) {
 		close(fd);
 		strbuf_release(&buf);
-		die(_("Could not read %s."), todo_file);
+		die(_("Could not read '%s'"), todo_file);
 	}
 	close(fd);
 
@@ -668,7 +668,7 @@ static int create_seq_dir(void)
 		return -1;
 	}
 	else if (mkdir(seq_dir, 0777) < 0)
-		die_errno(_("Could not create sequencer directory %s"), seq_dir);
+		die_errno(_("Could not create sequencer directory '%s'"), seq_dir);
 	return 0;
 }
 
@@ -682,9 +682,9 @@ static void save_head(const char *head)
 	fd = hold_lock_file_for_update(&head_lock, head_file, LOCK_DIE_ON_ERROR);
 	strbuf_addf(&buf, "%s\n", head);
 	if (write_in_full(fd, buf.buf, buf.len) < 0)
-		die_errno(_("Could not write to %s"), head_file);
+		die_errno(_("Could not write to '%s'"), head_file);
 	if (commit_lock_file(&head_lock) < 0)
-		die(_("Error wrapping up %s."), head_file);
+		die(_("Error wrapping up '%s'"), head_file);
 }
 
 static int reset_for_rollback(const unsigned char *sha1)
@@ -729,10 +729,10 @@ static int sequencer_rollback(struct replay_opts *opts)
 		return rollback_single_pick();
 	}
 	if (!f)
-		return error(_("cannot open %s: %s"), filename,
+		return error(_("Could not open '%s': %s"), filename,
 						strerror(errno));
 	if (strbuf_getline(&buf, f, '\n')) {
-		error(_("cannot read %s: %s"), filename, ferror(f) ?
+		error(_("Could not read '%s': %s"), filename, ferror(f) ?
 			strerror(errno) : _("unexpected end of file"));
 		fclose(f);
 		goto fail;
@@ -762,14 +762,14 @@ static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
 
 	fd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);
 	if (format_todo(&buf, todo_list, opts) < 0)
-		die(_("Could not format %s."), todo_file);
+		die(_("Could not format '%s'"), todo_file);
 	if (write_in_full(fd, buf.buf, buf.len) < 0) {
 		strbuf_release(&buf);
-		die_errno(_("Could not write to %s"), todo_file);
+		die_errno(_("Could not write to '%s'"), todo_file);
 	}
 	if (commit_lock_file(&todo_lock) < 0) {
 		strbuf_release(&buf);
-		die(_("Error wrapping up %s."), todo_file);
+		die(_("Error wrapping up '%s'"), todo_file);
 	}
 	strbuf_release(&buf);
 }
-- 
1.7.5.4

Re: [PATCH 1/2] Unification of user message strings

[Junio C Hamano]

Vincent van Ravesteijn <vfr@lyx.org> writes:

> From: Vincent van Ravesteijn <vfr@lyx.org>
>
> Rewrite user messages to stick to a uniform style for all messages. From the surrounding code, the following guidelines were deduced:
> - messages start with a capital,
> - short messages do not end with a full stop,
> - paths, filenames, and commands are quoted by single quotes (if not separated by the normal text by a ':'),
> - 'could not' is used rather than 'cannot'.
>
> Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>
> ---
>  gpg-interface.c |    6 +++---
>  grep.c          |    2 +-
>  help.c          |    2 +-
>  sequencer.c     |   24 ++++++++++++------------
>  4 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/gpg-interface.c b/gpg-interface.c
> index 09ab64a..5e14a21 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -56,7 +56,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
>  	args[3] = NULL;
>  
>  	if (start_command(&gpg))
> -		return error(_("could not run gpg."));
> +		return error(_("Could not run 'gpg'"));

Ok with s/c/C/, but I am not sure about the 'gpg' bit.  The name of the
program and path to it can be configured so the user may be expecting to
run a program called gnupg, and unquoted gpg feels more like a generic
term to refer to the program.  It might be worth using all-CAPS, though.

Likewise for the other hunks for this file.

> -		return error(_("cannot open %s: %s"), filename,
> +		return error(_("Could not open '%s': %s"), filename,

Honestly speaking, I would personally prefer "Cannot open" over "Could not
open".  Yes, all the error messages report _after_ we attempted to do
something and finding that we _couldn't_ do that thing, so "Could not" may
be technically more correct, but still...

But that is probably just me.

Other than that, the patch looks good; let's hear from others, too.

Re: [PATCH 1/2] Unification of user message strings

[Vincent van Ravesteijn]

>> From: Vincent van Ravesteijn<vfr@lyx.org>
>>
>> Rewrite user messages to stick to a uniform style for all messages. From the surrounding code, the following guidelines were deduced:
>> - messages start with a capital,
>> - short messages do not end with a full stop,
>> - paths, filenames, and commands are quoted by single quotes (if not separated by the normal text by a ':'),
>> - 'could not' is used rather than 'cannot'.
>>
>>
>> @@ -56,7 +56,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
>>   	args[3] = NULL;
>>
>>   	if (start_command(&gpg))
>> -		return error(_("could not run gpg."));
>> +		return error(_("Could not run 'gpg'"));
> Ok with s/c/C/, but I am not sure about the 'gpg' bit.  The name of the
> program and path to it can be configured so the user may be expecting to
> run a program called gnupg, and unquoted gpg feels more like a generic
> term to refer to the program.  It might be worth using all-CAPS, though.

Yes, all-CAPS seems the better alternative.

>> -		return error(_("cannot open %s: %s"), filename,
>> +		return error(_("Could not open '%s': %s"), filename,
> Honestly speaking, I would personally prefer "Cannot open" over "Could not
> open".  Yes, all the error messages report _after_ we attempted to do
> something and finding that we _couldn't_ do that thing, so "Could not" may
> be technically more correct, but still...
>
> But that is probably just me.
>

No it's not you, grep tells me "Cannot" is indeed the most occuring, 
except in sequencer.c (which I stumbled on first).


> Other than that, the patch looks good; let's hear from others, too.

Ok, let's hear other comments, and then I will send a reroll to fix up 
the things above.

Vincent

Re: [PATCH 1/2] Unification of user message strings

[Jeff King]

On Mon, Mar 19, 2012 at 06:51:42PM +0100, Vincent van Ravesteijn wrote:

> Rewrite user messages to stick to a uniform style for all messages.
> From the surrounding code, the following guidelines were deduced:
> - messages start with a capital,

I was surprised by this one, as I think we generally use lower-case
messages. Grepping shows that lower-case has a slight edge, though it is
far from decided:

  $ git grep -E '(error|die)\((_\()?"[A-Z]' | wc -l
  810
  $ git grep -E '(error|die)\((_\()?"[a-z]' | wc -l
  1267

-Peff

PS I was curious if it was simply that some people prefer one way and
   not the other, but the results are quite mixed. Below is the result
   of a small script I wrote that calculates "upper-casedness" per
   author using the above regexes and git-blame.

   The first number is the percentage of an author's messages starting
   with upper-case characters, followed by the total number of messages
   for that author, followed by the author's name. I limited the output
   to the top 20 by total number, as there is a long tail of people
   contributing just a few messages.

      0.14 37 Martin Koegler
      0.14 42 Johannes Sixt
      0.14 81 Jonathan Nieder
      0.16 31 Pierre Habouzit
      0.18 92 Nicolas Pitre
      0.20 66 Jeff King
      0.20 87 Linus Torvalds
      0.26 348 Junio C Hamano
      0.35 20 Christian Couder
      0.37 43 Nguyễn Thái Ngọc Duy
      0.39 18 René Scharfe
      0.41 32 Miklos Vajna
      0.45 270 Ævar Arnfjörð Bjarmason
      0.47 129 Shawn O. Pearce
      0.50 36 David Barr
      0.54 57 Johannes Schindelin
      0.62 39 Ramkumar Ramachandra
      0.67 21 Daniel Barkalow
      0.76 59 Johan Herland

   You can see that some people are usually lowercase and some are usually
   uppercase, but there are many people near 50%, doing both equally.
   There's also some inaccuracy in my simplistic sampling. For example,
   of my 13 upper-case messages, 11 of them are "BUG:".

Re: [PATCH 1/2] Unification of user message strings

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Mar 19, 2012 at 06:51:42PM +0100, Vincent van Ravesteijn wrote:
>
>> Rewrite user messages to stick to a uniform style for all messages.
>> From the surrounding code, the following guidelines were deduced:
>> - messages start with a capital,
>
> I was surprised by this one, as I think we generally use lower-case
> messages. Grepping shows that lower-case has a slight edge, though it is
> far from decided:
> ...
> PS I was curious if it was simply that some people prefer one way and
>    not the other, but the results are quite mixed. Below is the result
>    of a small script I wrote that calculates "upper-casedness" per
>    author using the above regexes and git-blame.
> ...
>    You can see that some people are usually lowercase and some are usually
>    uppercase, but there are many people near 50%, doing both equally.
>    There's also some inaccuracy in my simplistic sampling. For example,
>    of my 13 upper-case messages, 11 of them are "BUG:".

I had a vague impression that plumbing messages tend to be lowercase.  If
it is not be too much trouble, it might be interesting to redo the numbers
divided into the plumbing and Porcelain messages. Perhaps these "50%"
folks updated both plumbing and Porcelain. Another possibility is that
they tried to follow the local convention when they added a new one, or
reworded an existing one.

If we would be rewording, we would only be doing the Porcelain messages,
so I am OK with either way.

Re: [PATCH 1/2] Unification of user message strings

[Vincent van Ravesteijn]

> I had a vague impression that plumbing messages tend to be lowercase.  If
> it is not be too much trouble, it might be interesting to redo the numbers
> divided into the plumbing and Porcelain messages. Perhaps these "50%"
> folks updated both plumbing and Porcelain. Another possibility is that
> they tried to follow the local convention when they added a new one, or
> reworded an existing one.
>
> If we would be rewording, we would only be doing the Porcelain messages,
> so I am OK with either way.

It seems that the terms "Porcelain" and "plumbing" seems to be mixed up 
somewhere.

 From 'git help status': "The porcelain format is similar to the short 
format, but is guaranteed not to change in a backwards-incompatible way 
between git versions or based on user configuration. This makes it ideal 
for parsing by scripts".

 From 'http://progit.org/book/ch9-1.html': "....it has a bunch of verbs 
that do low-level work and were designed to be chained together UNIX 
style or called from scripts. These commands are generally referred to 
as “plumbing” commands, and the more user-friendly commands are called 
“porcelain” commands."

It feels like 'porcelain' means: "be careful, things break easily"; and 
'plumbing' means: "use all the force you want to get it into (a 
user-friendly) shape".

Second, to me it is not totally clear which strings are plumbing and 
which ones are porcelain. Is there a general rule to tell ? The 
command-list file says what the general intention of a command is, but 
often it's both plumbing as porcelain.

Can anyone help me out here ?

Vincent

Re: [PATCH 1/2] Unification of user message strings

[Jakub Narebski]

Vincent van Ravesteijn <vfr@lyx.org> writes:

> > I had a vague impression that plumbing messages tend to be lowercase.  If
> > it is not be too much trouble, it might be interesting to redo the numbers
> > divided into the plumbing and Porcelain messages. Perhaps these "50%"
> > folks updated both plumbing and Porcelain. Another possibility is that
> > they tried to follow the local convention when they added a new one, or
> > reworded an existing one.
> >
> > If we would be rewording, we would only be doing the Porcelain messages,
> > so I am OK with either way.
> 
> It seems that the terms "Porcelain" and "plumbing" seems to be mixed
> up somewhere.
> 
>  From 'git help status': "The porcelain format is similar to the short
> format, but is guaranteed not to change in a backwards-incompatible
> way between git versions or based on user configuration. This makes it
> ideal for parsing by scripts".

The '--porcelain' option means "intended *for* parsing by porcelain",
not that it is 'porcelain' output.
 
>  From 'http://progit.org/book/ch9-1.html': "....it has a bunch of
> verbs that do low-level work and were designed to be chained together
> UNIX style or called from scripts. These commands are generally
> referred to as 'plumbing' commands, and the more user-friendly
> commands are called 'porcelain' commands."

Mnemonics: "plumbing" are hidden 'guts' of git (the engine / backend
part).

> It feels like 'porcelain' means: "be careful, things break easily";
> and 'plumbing' means: "use all the force you want to get it into (a
> user-friendly) shape".
> 
> Second, to me it is not totally clear which strings are plumbing and
> which ones are porcelain. Is there a general rule to tell ? The
> command-list file says what the general intention of a command is, but
> often it's both plumbing as porcelain.

"Porcelain" commands are those that are facing user (as is "porcelain"
in armature), so they might be changed to make it more user friendly.
 
"Plumbing" command output is to be consumed by other commands and
scripts, so it must be 'cast in stone' and cannot be changed.  It is
meant to be easily machine-parseable, and not to be user friendly.

> Can anyone help me out here ?

HTH
-- 
Jakub Narebski

Re: [PATCH 1/2] Unification of user message strings

[Vincent van Ravesteijn]

>>
>> It seems that the terms "Porcelain" and "plumbing" seems to be mixed
>> up somewhere.
>>
>>   From 'git help status': "PORCELAIN FORMAT: The porcelain format is
>> similar to the short format, but is guaranteed not to change in a
>> backwards-incompatible way between git versions or based on user
>> configuration. This makes it ideal for parsing by scripts".
> The '--porcelain' option means "intended *for* parsing by porcelain",
> not that it is 'porcelain' output.

I've a hard time to interpret the quote above in the way you explain. 
Maybe a little improvement to this description would be helpful.

>
> HTH

Thanks.

Vincent

[PATCH 2/2] Make some strings translatable

[Vincent van Ravesteijn]

From: Vincent van Ravesteijn <vfr@lyx.org>

These strings seem not to be part of the plumbing interface and can therefore be translated. Also the style is adjusted to the guidelines mentioned in the previous commit.

Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>
---
 gpg-interface.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 5e14a21..66a7c12 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -109,10 +109,10 @@ int verify_signed_buffer(const char *payload, size_t payload_size,
 	args_gpg[0] = gpg_program;
 	fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX");
 	if (fd < 0)
-		return error("could not create temporary file '%s': %s",
+		return error(_("Could not create temporary file '%s': %s"),
 			     path, strerror(errno));
 	if (write_in_full(fd, signature, signature_size) < 0)
-		return error("failed writing detached signature to '%s': %s",
+		return error(_("Failed writing detached signature to '%s': %s"),
 			     path, strerror(errno));
 	close(fd);
 
@@ -124,7 +124,7 @@ int verify_signed_buffer(const char *payload, size_t payload_size,
 	args_gpg[2] = path;
 	if (start_command(&gpg)) {
 		unlink(path);
-		return error("could not run gpg.");
+		return error(_("Could not run 'gpg'"));
 	}
 
 	write_in_full(gpg.in, payload, payload_size);
-- 
1.7.5.4