| Message ID | 20230323162234.995485-1-oswald.buddenhagen@gmx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] Capitalization and punctuation fixes to some user visible messages | expand |
Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes: > require_clean_work_tree(the_repository, > N_("pull with rebase"), > - _("please commit or stash them."), 1, 0); > + _("Please commit or stash them."), 1, 0); The require_clean_work_tree() function uses the message this patch touches here like so: ... if (err) { if (hint) error("%s", hint); if (!gently) exit(128); } and the message given to error() is shown with "error: " prefix. Our friendly CodingGuidelines is fairly clear on what to do to a single sentence error message like this one: | Error Messages | | - Do not end error messages with a full stop. | | - Do not capitalize the first word, only because it is the first word | in the message ("unable to open %s", not "Unable to open %s"). But | "SHA-3 not supported" is fine, because the reason the first word is | capitalized is not because it is at the beginning of the sentence, | but because the word would be spelled in capital letters even when | it appeared in the middle of the sentence. So, if you want to touch this message, what you want to do is to leave the capitalization alone, but to drop the full stop at the end. > diff --git a/sequencer.c b/sequencer.c > index 3be23d7ca2..fda68cd33d 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -3629,13 +3629,13 @@ static int do_exec(struct repository *r, const char *command_line) > "\n"), > command_line, > dirty ? N_("and made changes to the index and/or the " > - "working tree\n") : ""); > + "working tree.\n") : ""); I think this long and multi-sentence warning message should be split into two and the latter half should become an advice message that the user can squelch. Until it happens, which would require us to rewrite these messages, I wouldn't bother touching this message if I were you. > } else if (dirty) { > warning(_("execution succeeded: %s\nbut " > - "left changes to the index and/or the working tree\n" > + "left changes to the index and/or the working tree.\n" > "Commit or stash your changes, and then run\n" > "\n" > " git rebase --continue\n" Likewise.
On Thu, Mar 23, 2023 at 01:39:28PM -0700, Junio C Hamano wrote: >Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes: > >> require_clean_work_tree(the_repository, >> N_("pull with rebase"), >> - _("please commit or stash them."), 1, 0); >> + _("Please commit or stash them."), 1, 0); > >Our friendly CodingGuidelines is fairly clear on what to do to a >single sentence error message like this one: [...] > i know. that's why the commit message makes a point of stating that it's making an exception. the reasoning is basically that it's not *really* an error message, it's just abusing the machinery. you obviously noticed that yourself, suggesting to turn these strings into advice messages. but that kinda feels like overkill to me.
diff --git a/builtin/pull.c b/builtin/pull.c index 56f679d94a..bb2ddc93ab 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -1044,7 +1044,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (!opt_autostash) require_clean_work_tree(the_repository, N_("pull with rebase"), - _("please commit or stash them."), 1, 0); + _("Please commit or stash them."), 1, 0); if (get_rebase_fork_point(&rebase_fork_point, repo, *refspecs)) oidclr(&rebase_fork_point); diff --git a/sequencer.c b/sequencer.c index 3be23d7ca2..fda68cd33d 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3629,13 +3629,13 @@ static int do_exec(struct repository *r, const char *command_line) "\n"), command_line, dirty ? N_("and made changes to the index and/or the " - "working tree\n") : ""); + "working tree.\n") : ""); if (status == 127) /* command not found */ status = 1; } else if (dirty) { warning(_("execution succeeded: %s\nbut " - "left changes to the index and/or the working tree\n" + "left changes to the index and/or the working tree.\n" "Commit or stash your changes, and then run\n" "\n" " git rebase --continue\n"
These messages are used in multi-line context, where not sticking to proper grammar makes things hard to read and has questionable looks. Therefore, these changes go against the usual rules for error messages. Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de> --- builtin/pull.c | 2 +- sequencer.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)