From: Jeff King <peff@peff.net>
Date: Tue, 16 Feb 2016 22:17:39 GMT
Subject: Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value
Message-ID: <20160216221739.GA8370@sigill.intra.peff.net>
In-Reply-To: <56C39F55.1090903@ramsayjones.plus.com>

On Tue, Feb 16, 2016 at 10:14:45PM +0000, Ramsay Jones wrote:

> > I think it's more than that one-liner. This patch shows "type:name"
> > verbatim from what is passed into do_config_from_file, as does the error
> > message. If they are going to have different output formats (e.g.,
> > "<stdin>" versus "stdin"), there needs to be logic transforming them in
> > at least one of the spots.
> 
> Ugh, yes you are right.
> 
> Hmm, I just hacked something up (see below) and, since its a bit
> ugly, I'm now in two minds! (it could be improved, of course). ;-)
> 
> So, I'll leave it to yourself and Lars to decide.
> [...]
> +	if (!strcmp(cftype, "stdin")) {
> +		cftype = "file";
> +		cfname = "<stdin>";
> +	}

I think if we go this route it would be cleaner to just make "type" an
enum and convert it to the appropriate string in the callers. But other
than that, I think your patch is along the correct lines.

I dunno. Personally I am fine with the change in error messages done by
Lars. I could go either way.

-Peff

PS Thanks also for your patch fixing the prototypes. I completely missed
   that.