Re: [PATCH v1] config: add '--sources' option to print the source of a config value

From: Jeff King <peff@peff.net>
Date: Feb 5, 2016, 13:58 UTC
Message-ID: <20160205135855.GA19154@sigill.intra.peff.net>
In-Reply-To: <56B48803.9080909@gmail.com>

On Fri, Feb 05, 2016 at 12:31:15PM +0100, Sebastian Schuberth wrote:

Show 28 quoted lines

> >I'm not sure returning here is the best idea. We won't have a config
> >filename if we are reading from "-c", but if we return early from this
> >function, it parses differently than every other line. E.g., with your
> >patch, if I do:
> >
> >   git config -c foo.bar=true config --sources --list
> >
> >I'll get:
> >
> >   /home/peff/.gitconfig <tab> user.name=Jeff King
> >   /home/peff/.gitconfig <tab> user.email=peff@peff.net
> >   ...etc...
> >   foo.bar=true
> >
> >If somebody is parsing this as a tab-delimited list, then instead of the
> >source field for that line being empty, it is missing (and it looks like
> >"foo.bar=true" is the source file). I think it would be more friendly to
> >consumers of the output to have a blank (i.e., set "fn" to the empty
> >string and continue in the function).
> 
> Or to come up with a special string to denote config values specified on the
> command line. Maybe somehting like
> 
>     <command line> <tab> foo.bar=true
> 
> I acknowledge that "<command line>" would be a valid filename on some
> filesystems, but I think the risk is rather low that someone would actually
> be using that name for a Git config file.

Yeah, I agree it's unlikely. And the output is already ambiguous, as the first field could be a blob (though I guess the caller knows if they passed "--blob" or not). If we really wanted an unambiguous output, we could have something like "file:...", "blob:...", etc. But that's a bit less readable for humans, and I don't think solves any real-world problems.

So I think it would be OK to use "<command line>" here, as long as the token is documented.

Are there any other reasons why current_config_filename() would return NULL, besides command-line config? I don't think so. It looks like we can read config from stdin, but we use the token "<stdin>" there for the name field (another ambiguity!).

-Peff