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@xxxxxxxx > > ...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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html