| Message ID | xmqqa5dkqjmr.fsf_-_@gitster.g (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] CodingGuidelines: a handful of error message guidelines | expand |
On Wed, Nov 27, 2024 at 7:36 PM Junio C Hamano <gitster@pobox.com> wrote: > It is more efficient to have something in the coding guidelines > document to point at, when we want to review and comment on a new > message in the codebase to make sure it "fits" in the set of > existing messages. > > Let's write down established best practice we are aware of. > > Helped-by: Eric Sunshine <sunshine@sunshineco.com> > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines > @@ -689,16 +689,29 @@ Program Output > Error Messages > > - - Say what the error is first ("cannot open %s", not "%s: cannot open") > + - Say what the error is first ("cannot open '%s'", not "%s: cannot open"). > + > + - Enclose the subject of an error inside a pair of single quotes, > + e.g. `die(_("unable to open '%s'"), path)`. > + > + - Unless there is a compelling reason not to, error messages from the > + Porcelain command should be marked for `_("translation")`. Here you capitalize "Porcelain" but below, "plumbing" is all lowercase. > + - Error messages from the plumbing commands are sometimes meant for > + machine consumption and should not be marked for `_("translation")` > + to keep them 'grep'-able. Using the same example, `_("translation")`, for both the "should be" and "should not be" cases may very well confuse readers. (It certainly confused me.) Perhaps mirroring the example of an item earlier in the list would be clearer: - Unless there is a compelling reason not to, error messages from porcelain commands should be marked for translation, e.g. `die(_("bad revision"))` - Error messages from plumbing commands are sometimes meant for machine consumption, thus should not be marked for translation, e.g. `die("bad revision")` > + - BUG("message") are for communicating the specific error to > + developers, and not to be translated. Okay, although could be slightly more explicit: - BUG("message") is for communicating a specific failure to developers, not end-users, thus should not be translated.
Eric Sunshine <sunshine@sunshineco.com> writes: >> + - Unless there is a compelling reason not to, error messages from the >> + Porcelain command should be marked for `_("translation")`. > > Here you capitalize "Porcelain" but below, "plumbing" is all lowercase. ;-) I think that is how we spell them in our documentation when we contrast them against each other. >> + - Error messages from the plumbing commands are sometimes meant for >> + machine consumption and should not be marked for `_("translation")` >> + to keep them 'grep'-able. > > Using the same example, `_("translation")`, for both the "should be" > and "should not be" cases may very well confuse readers. (It certainly > confused me.) Perhaps mirroring the example of an item earlier in the > list would be clearer: > > - Unless there is a compelling reason not to, error messages from > porcelain commands should be marked for translation, e.g. > `die(_("bad revision"))` > > - Error messages from plumbing commands are sometimes meant for > machine consumption, thus should not be marked for translation, > e.g. `die("bad revision")` Thanks, that is much better. Let me steal it verbatim in the hopefully final reroll. >> + - BUG("message") are for communicating the specific error to >> + developers, and not to be translated. > > Okay, although could be slightly more explicit: > > - BUG("message") is for communicating a specific failure to > developers, not end-users, thus should not be translated. The way I read your rewrite is that the "communitation" mentioned is between the program and the user who saw the message. I wanted to say that the message is seen first by an end-user, and then is communicated to developers. And not translating is one way to make sure the message is not mangled, and stays grep-able, during the game of telephone. Would this work better? - In order to help the user who saw BUG("message") to accurately communicate it to developers, do not mark them for translation. Thanks.
On Thu, Nov 28, 2024 at 4:28 AM Junio C Hamano <gitster@pobox.com> wrote: > Eric Sunshine <sunshine@sunshineco.com> writes: > >> + Porcelain command should be marked for `_("translation")`. > > > > Here you capitalize "Porcelain" but below, "plumbing" is all lowercase. > > ;-) I think that is how we spell them in our documentation when we > contrast them against each other. I must not have been paying close enough attention. > >> + - BUG("message") are for communicating the specific error to > >> + developers, and not to be translated. > > > > Okay, although could be slightly more explicit: > > > > - BUG("message") is for communicating a specific failure to > > developers, not end-users, thus should not be translated. > > The way I read your rewrite is that the "communitation" mentioned is > between the program and the user who saw the message. I wanted to > say that the message is seen first by an end-user, and then is > communicated to developers. And not translating is one way to make > sure the message is not mangled, and stays grep-able, during the > game of telephone. > > Would this work better? > > - In order to help the user who saw BUG("message") to accurately > communicate it to developers, do not mark them for translation. Let's not spend too much time fine-tuning this. I found your original clearer than this rewrite. It was just the "and not to be" bit that made my reading hiccup. Taking your original but substituting in "thus" may help: - BUG("message") are for communicating the specific error to developers, thus should not be translated.
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 3263245b03..2b8f99f333 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -689,16 +689,29 @@ Program Output Error Messages - - Do not end error messages with a full stop. + - Do not end a single-sentence error message 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 + 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. - - Say what the error is first ("cannot open %s", not "%s: cannot open") + - Say what the error is first ("cannot open '%s'", not "%s: cannot open"). + + - Enclose the subject of an error inside a pair of single quotes, + e.g. `die(_("unable to open '%s'"), path)`. + + - Unless there is a compelling reason not to, error messages from the + Porcelain command should be marked for `_("translation")`. + + - Error messages from the plumbing commands are sometimes meant for + machine consumption and should not be marked for `_("translation")` + to keep them 'grep'-able. + + - BUG("message") are for communicating the specific error to + developers, and not to be translated. Externally Visible Names
Taking input from comments by Eric (thanks) on the previous round, this iteration adds a bit more about Porcelain/Plumbing and BUG(). diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 71e4742fd5..2b8f99f333 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -703,8 +703,15 @@ Error Messages - Enclose the subject of an error inside a pair of single quotes, e.g. `die(_("unable to open '%s'"), path)`. - - Unless there is a compelling reason not to, error messages should - be marked for `_("translation")`. + - Unless there is a compelling reason not to, error messages from the + Porcelain command should be marked for `_("translation")`. + + - Error messages from the plumbing commands are sometimes meant for + machine consumption and should not be marked for `_("translation")` + to keep them 'grep'-able. + + - BUG("message") are for communicating the specific error to + developers, and not to be translated. Externally Visible Names --- >8 --- It is more efficient to have something in the coding guidelines document to point at, when we want to review and comment on a new message in the codebase to make sure it "fits" in the set of existing messages. Let's write down established best practice we are aware of. Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/CodingGuidelines | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)