Thread (2 messages) 2 messages, 2 authors, 2024-03-28

Re: [PATCH 17/16] config: add core.commentString

From: Jeff King <hidden>
Date: 2024-03-28 09:47:47

Possibly related (same subject, not in this thread)

On Wed, Mar 27, 2024 at 09:13:31AM -0700, Junio C Hamano wrote:
quoted
An alternative to using "$var cannot ..." in the error messages (if we
don't like the all-lowercase variable name) is to just say "comment
strings cannot ...". That vaguely covers both cases, and the message
printed by the config code itself does mention the actual variable name
that triggered the error.
OK, because the error() return from this function will trigger
another die() in the caller, e.g.

    error: core.commentchar must have at least one character
    fatal: bad config variable 'core.commentchar' in file '.git/config' at line 6

so we can afford to make the "error" side vague, except that the
"fatal" one is also downcased already, so we are not really solving
anything by making the message vague, I would think.  The posted
patch as-is is prefectly fine.
Oh, right.  For some reason I thought the die() message would have the
variable as written by the user, but that obviously is not true. So I
agree it would not even be an improvement (and the normalizing in my new
error() message is something we've been living with all along anyway for
other messages).
Side note:
    I wonder if we would later want to somehow _merge_ these two
    error messages, i.e. the lower-level will notice and record the
    nature of the problem instead of calling error(), and the caller
    will use the recorded information while composing the "fatal"
    message to die with.  I actually do not know if it is a good
    idea to begin with.  If we want to do it right, the "record"
    part probably cannot be a simple "stringify into strbuf" that
    will result in lego message that is harder for i18n folks.
Yeah, this is a general problem of accumulating errors. I had always
assumed in cases like this that we could have some language-independent
syntax like:

  die("%s:%d: error parsing '%s': %s",
      file, line_nr, var, err_from_callback);

It's certainly lego-like, but it avoids the worst lego cases where
we're literally composing sentences. But as somebody who does not do
translations, it's possible I'm just being optimistic. ;)

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help