git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Lars Schneider <larsxschneider@gmail.com>
Date
Feb 7, 2016, 19:44 UTC
Message-ID
<278405D6-873C-4E73-965B-543D66A893D7@gmail.com>
In-Reply-To
<20160205135855.GA19154@sigill.intra.peff.net>
On 05 Feb 2016, at 14:58, Jeff King <peff@peff.net> wrote:
Show 40 quoted lines
> On Fri, Feb 05, 2016 at 12:31:15PM +0100, Sebastian Schuberth wrote:
> 
>>> 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.
Sounds good to me. I'll add it!
> 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!).

During my testing I haven't found any other case either. To be honest I didn't know the "stdin" way to set the config! I added a test case for that, too!

Thanks, Lars

Previous: Jeff KingNext: Jeff King
Message 10 of 14 in “config: add '--sources' option to print the source of a config value”
  1. config: add '--sources' option to print the source of a config valuelarsxschneider@gmail.com, Feb 5, 2016
  2. Sebastian SchuberthFeb 5, 2016
  3. Jeff KingFeb 5, 2016
  4. Lars SchneiderFeb 7, 2016
  5. Sebastian SchuberthFeb 8, 2016
  6. Jeff KingFeb 8, 2016
  7. Jeff KingFeb 5, 2016
  8. Sebastian SchuberthFeb 5, 2016
  9. Jeff KingFeb 5, 2016
  10. Lars SchneiderFeb 7, 2016
  11. Jeff KingFeb 8, 2016
  12. Sebastian SchuberthFeb 8, 2016
  13. Jeff KingFeb 8, 2016
  14. Lars SchneiderFeb 7, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.