git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Tanay Abhra <tanayabh@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, Andreas Krey <a.krey@gmx.de>,
	git@vger.kernel.org
Subject: Re: [PATCH v2] add a flag to supress errors in git_config_parse_key()
Date: Wed, 18 Feb 2015 14:02:15 -0500	[thread overview]
Message-ID: <20150218190215.GD7257@peff.net> (raw)
In-Reply-To: <54E1A30F.5010303@gmail.com>

On Mon, Feb 16, 2015 at 01:28:07PM +0530, Tanay Abhra wrote:

> I went through Junio's config guideline patch series
> and the whole thread of underscore bug report and I also think
> that pager.*.command is the right path to go.
> 
> If you want to relax the syntactic requirement (such as add '_' to
> the current set of allowed chacters), I can work upon it but most of the
> comments point that moving towards pager.*.command would be better.

No, as silly as I find the "_" restriction, it is not worth doing. One,
it would not cover all cases (it is one common case, so it makes the
problem more rare but does not eliminate it). And two, there are other
parsers of git's config format. Technically we do not need to care about
them and they can follow our lead, but we do not need to make things
harder on them than is necessary.

>  	if (last_dot == NULL || last_dot == key) {
> -		error("key does not contain a section: %s", key);
> +		if (!flags)
> +			error("key does not contain a section: %s", key);

The intent of the flag variable is that you would check:

  if (!(flags & CONFIG_ERROR_QUIET))

here. I know that there are no other flags yet, so the two are
equivalent. But when somebody adds a new flag later, you would not want
them to have to tweak each of these sites.

-Peff

  reply	other threads:[~2015-02-18 19:02 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-06 12:45 BUG: 'error: invalid key: pager.show_ref' on 'git show_ref' Andreas Krey
2015-02-06 19:33 ` Jeff King
2015-02-06 19:44   ` Junio C Hamano
2015-02-06 20:39     ` Jeff King
2015-02-10 19:45       ` [PATCH] config: add show_err flag to git_config_parse_key() Tanay Abhra
2015-02-11  0:27         ` Jeff King
2015-02-11 18:47           ` Junio C Hamano
2015-02-16  7:58             ` [PATCH v2] add a flag to supress errors in git_config_parse_key() Tanay Abhra
2015-02-18 19:02               ` Jeff King [this message]
2015-02-07  0:03     ` BUG: 'error: invalid key: pager.show_ref' on 'git show_ref' Mikael Magnusson
2015-02-07  5:01       ` Jeff King
2015-02-06 20:14   ` Junio C Hamano
2015-02-06 20:37     ` Jeff King
2015-02-06 22:17       ` Junio C Hamano
2015-02-06 22:27       ` Junio C Hamano
2015-02-07  4:52         ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150218190215.GD7257@peff.net \
    --to=peff@peff.net \
    --cc=a.krey@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=tanayabh@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).