On Mon, Oct 25 2021, Abhradeep Chakraborty via GitGitGadget wrote:
> From: Abhradeep Chakraborty <chakrabortyabhradeep79@xxxxxxxxx>
>
> Issue no #635 in the gitgitgadget/git repo has a brief description
> about the violation of error message style convention. These
> violations are - 1) Do not end error messages with a full stop. 2) Do
> not capitalize ("unable to open %s", not "Unable to open %s") 3) Say
> what the error is first ("cannot open %s", not "%s: cannot open")
Let's summarize the guidelines instead of asking readers to visit
https://github.com/gitgitgadget/git/issues/635 (which isn't even
directly linked here) only to find a quote from CodingGuidelines.
I think it's good to split these up, e.g. the rewording of the BUG()
message for the 3rd quoted item in CodingGuidelines from cases where
you're only changing the capitalization of these messages.
> One thing to note that I didn't change any .po file as there is a
> doubt whether those are good to change or not.
Those should be left alone, and will be updated by translators as part
of the whole "git.pot" process.
> - res = error(_("Could not parse HEAD^{tree}"));
> + res = error(_("could not parse HEAD^{tree}"));
If we're going to mass-edit these I wouldn't mind an addition to the
guidelines that says we ''-quote things, and add that when appropriate....
> regerror(ret, ®ex, errbuf, sizeof(errbuf));
> - err(s, _("Malformed search regexp %s: %s"),
> + err(s, _("malformed search regexp %s: %s"),
> s->answer.buf, errbuf);
> continue;
..e.g. here.
> - error(_("You have not concluded your merge (MERGE_HEAD exists)."));
> + error(_("you have not concluded your merge (MERGE_HEAD exists)"));
> if (advice_enabled(ADVICE_RESOLVE_CONFLICT))
> advise(_("Please, commit your changes before merging."));
> - die(_("Exiting because of unfinished merge."));
> + die(_("exiting because of unfinished merge"));
> }
Surely any convention of not using full-stop should extend to advise()
too?
But in this case aren't we constructing sentences in parts? Maybe having
the full-stops here is better than not, I'm not sure.
Also, have you tried using coccinelle for this? Doing this sort of
replacement should be a small matter of some inline Python, the
linux.git repo has some good examples of that.
It would make discussing these changes easier, and per the "pending"
docs in contrib/coccinelle/README any patches could follow such rules
incrementally...
