config: allow user to know scope of config options #478
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Welcome to GitGitGadgetHi @ROGERSM94, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests. Please make sure that this Pull Request has a good description, as it will be used as cover letter. Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:
It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code. Contributing the patchesBefore you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a comment to your PR of the form Both the person who commented An alternative is the channel Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions. If you want to see what email(s) would be sent for a submit request, add a PR comment If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox ("raw") file corresponding to the mail you want to reply to from the Git mailing list. If you use GMail, you can upload that raw mbox file via: curl -g --user "<EMailAddress>:<Password>" --url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txtNeed help?New contributors who want advice are encouraged to join You may also be able to find help in real time in the developer IRC channel, |
|
/allow |
|
User ROGERSM94 is now allowed to use GitGitGadget. WARNING: ROGERSM94 has no public email address set on GitHub |
dscho
reviewed
Nov 28, 2019
builtin/config.c
Outdated
| @@ -189,11 +191,60 @@ static void show_config_origin(struct strbuf *buf) | |||
| strbuf_addch(buf, term); | |||
| } | |||
|
|
|||
| static const char *scope_to_string(enum config_scope scope) { | |||
| const char *local = "(local)"; | |||
There was a problem hiding this comment.
Should these be translatable? If so, please surround them in N_(...), i.e.
const char *local = N_("local");There was a problem hiding this comment.
the prefixes for --show-origin were not translatable so I didn't consider it, but I don't see any reason why these shouldn't be
builtin/config.c
Outdated
| const char *system = "(system)"; | ||
| const char *command_line = "(command line)"; | ||
| const char *unknown = "(unknown)"; | ||
| /* --local, --global, and --system work the same as --file so there's |
There was a problem hiding this comment.
Please start multi-line comments with a single /* on the first line.
builtin/config.c
Outdated
| * setting the scope, so we use our information about which options | ||
| * were passed | ||
| */ | ||
| if (use_local_config) { |
There was a problem hiding this comment.
Wouldn't it be more succinct to say
if (use_local_config || scope == CONFIG_SCOPE_REPO)
return N_("local");?
builtin/config.c
Outdated
|
|
||
| static void show_config_scope(struct strbuf *buf) | ||
| { | ||
| const char term = end_null ? '\0' : '\t'; |
There was a problem hiding this comment.
Just because the existing code is buggy does not mean you have to retain the bug when copy-editing it. So please use end_nul instead of end_null: NUL is a character, NULL is a pointer. They are totally different things.
There was a problem hiding this comment.
I changed the variable to be end_nul but I didn't change the option --null because that would probably break a lot of things
| EOF | ||
| git config --show-scope user.override >output && | ||
| test_cmp expect output | ||
| ' |
There was a problem hiding this comment.
Correct me if I'm wrong, but don't all these tests exercise the precise same code path? So they would all be broken, or none of them, right?
Adding test cases is not necessarily as cheap a thing as one might think: it costs a lot of time in the Azure Pipelines just because of all the Unix shell overhead (which is particularly expensive on Windows).
I'd much prefer one test case that exercises all of these:
- a system setting
- a setting in a file included from
--system - a setting in
--global - a local setting
- a local setting overriding the
--globalsetting
There was a problem hiding this comment.
I wasn't really sure on what the standard for testing was, so I just covered the same bases as show-origin... As far as your cases go:
- None of the other tests actually touch the system configuration file or option. I believe this has to do with the git sdk not having /usr/etc/gitconfig file, and i'm not sure if the tests should be creating one or not.
- I'd also add an option passed in through
-c,--file, or--blob - I'd also want a test explicitly for the
--local(or global or system) options considering the weird behavior with how the parser considers them not being of their respective scopes when they clearly are - Interaction with
--show-originand--nullprobably don't need to be as tested but i'm not really confident enough to say one way or the other. The other tests can probably be removed though
builtin/config.c
Outdated
| return command_line; | ||
| case CONFIG_SCOPE_UNKNOWN: | ||
| default: | ||
| return unknown; |
There was a problem hiding this comment.
Scope is orthogonal to origin, all options passed through --file and --blob should have command line scoping (but currently don't, which I plan on fixing post haste)
t/t1300-config.sh
Outdated
| (local) file:.git/config user.local=true | ||
| (local) file:.git/config user.override=local | ||
| (local) file:.git/config include.path=../include/relative.include | ||
| (local) file:.git/../include/relative.include user.relative=include |
There was a problem hiding this comment.
So this is what you referenced here regarding the backtrace?
I am afraid that I was not quite clear what I meant: when I include a file in the --system config, and then include another one in a conditional include, and that has the setting that I am looking for, I would like to have that entire trace: the line in the system config that included the intermediate config, and the line in that file that conditionally included the "inner" config, and then the line in that config that set this.
I do agree, though, that this is safely outside the scope of what you have in mind.
There was a problem hiding this comment.
Oh yeah, in that case definitely out of scope
ROGERSM94
force-pushed
the
add-config-flags
branch
3 times, most recently
from
e94c8e8 to
aeeb903
Compare
ROGERSM94
force-pushed
the
add-config-flags
branch
from
aeeb903 to
6f01233
Compare
|
@ROGERSM94 seems that a certain blob cannot be found: I suspect that your patch tries to construct a blob from a file, and that there are CR/LF endings in there for you, but LF-only line endings in the Linux/macOS cases. |
|
So I've actually realized there are two issues at play here. One is that CUSTOM_CONFIG_FILE has a weird name that's illegal for windows... namely The other was that my test was using --file= instead of --blob. I'm not sure if I should redo the other tests or make a new config file here (bit of a silly mistake, but not the real issue here). (fixing this should resolve it, I think) I'm not sure it's necessarily beneficial to have all tests use that file name because it actually makes a large number of the tests unable to run on windows. Changing CUSTOM_CONFIG_FILENAME to something like custom.conf actually allows the test to pass locally on my machine here (and presumably the fixing the second issue should make it pass on linux, etc. without having to change that variable) |
ROGERSM94
force-pushed
the
add-config-flags
branch
from
6f01233 to
ec699bb
Compare
|
So I'm going to leave changing the tests to not depend on that weird file name for later, assuming that the actual issue was that I was passing |
|
Sounds good to me! |
|
/preview |
|
Error: Could not determine full name of ROGERSM94 |
|
/preview |
|
Preview email sent as [email protected] |
|
/submit |
|
Submitted as [email protected] |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| @@ -29,10 +29,11 @@ static int use_worktree_config; | |||
| static struct git_config_source given_config_source; | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Matthew Rogers via GitGitGadget" <[email protected]> writes:
> -static int end_null;
> +static int end_nul;
> - OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
> + OPT_BOOL('z', "null", &end_nul, N_("terminate values with NUL byte")),
> - const char term = end_null ? '\0' : '\t';
> + const char term = end_nul ? '\0' : '\t';
> - if (end_null)
> + if (end_nul)
These are correct changes, but is unrelated noise in the context of
the theme of the patch, no?
> +static const char *scope_to_string(enum config_scope scope) {
> + /*
> + * --local, --global, and --system work the same as --file so there's
> + * no easy way for the parser to tell the difference when it is
> + * setting the scope, so we use our information about which options
> + * were passed
> + */
> + if (use_local_config || scope == CONFIG_SCOPE_REPO) {
> + return "local";
> + } else if (use_global_config || scope == CONFIG_SCOPE_GLOBAL) {
> + return "global";
> + } else if (use_system_config || scope == CONFIG_SCOPE_SYSTEM) {
> + return "system";
The above is slightly tricky; --global/--system/--local are made
mutually exclusive in the higher level, so if any of them is set,
we do not even need to look at the "scope" to tell what kind of
source we are reading from.
> + } else if (given_config_source.use_stdin ||
> + given_config_source.blob ||
> + given_config_source.file ||
> + scope == CONFIG_SCOPE_CMDLINE) {
> + return "command line";
I am not sure what the implication of saying "they came from the
command line" when we read from the standard input or from a blob.
> + } else {
> + return "unknown";
> + }
> +}
In any case, the need for such logic that says "scope might not say
it is REPO, but when use_local_config is true, we are doing a local
config" implies that "scope" parameter the caller of this function
has is not set correctly when these options are used---would that be
the real bug that needs fixing, rather than getting "worked around"
with a code like this?
It almost makes me point fingers at config.c::config_with_options()
where config_source is inspected and git_config_from_*() helpers are
called without setting the current_parsing_scope. Unlike these
cases, when do_git_config_sequence() is called from that function,
the scope is recorded in the variable before each standard config
source file is opened and read. What would we be breaking if we
taught the function to set the current_parsing_scope variable
correctly even when reading from the config_source? That would
certainly simplify this function quite a lot, but if the other parts
of the codebase relies on the current behaviour, we cannot make such
a change lightly.
> +static void show_config_scope(struct strbuf *buf)
> +{
> + const char term = end_nul ? '\0' : '\t';
> + const char *scope = scope_to_string(current_config_scope());
> +
> + strbuf_addch(buf, '(');
> + if (end_nul)
> + strbuf_addstr(buf, N_(scope));
> + else
> + quote_c_style(scope, buf, NULL, 0);
Isn't this overkill? I think this code was copied-and-pasted from
the other function that needs to show an arbitrary end-user supplied
data which is a pathname, so it makes perfect sense to use c-style
quoting, but the token scope_to_string() returns is taken from a
bounded set that doesn't require such quoting, no?
> + strbuf_addch(buf, ')');
> + strbuf_addch(buf, term);
> +}
> +
There was a problem hiding this comment.
On the Git mailing list, Jeff King wrote (reply to this):
On Wed, Dec 18, 2019 at 11:46:06AM -0800, Junio C Hamano wrote:
> It almost makes me point fingers at config.c::config_with_options()
> where config_source is inspected and git_config_from_*() helpers are
> called without setting the current_parsing_scope. Unlike these
> cases, when do_git_config_sequence() is called from that function,
> the scope is recorded in the variable before each standard config
> source file is opened and read. What would we be breaking if we
> taught the function to set the current_parsing_scope variable
> correctly even when reading from the config_source? That would
> certainly simplify this function quite a lot, but if the other parts
> of the codebase relies on the current behaviour, we cannot make such
> a change lightly.
As the author of the SCOPE enum, I think this is the right direction.
The only users of current_config_scope() are in config callbacks (e.g.,
upload_pack_config() checks it for the "packobjectshook"), which you
couldn't trigger via "git config" anyway.
The main reason I didn't set the scope before in config_with_options()
is that it's not given the scope at all; git-config resolves it to the
filename. So if git_config_source grows an enum to select the type, and
all that filename-resolution gets pushed down into config_with_options(),
the whole thing would fall out naturally, I think.
-Peff
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Jeff King <[email protected]> writes:
> ... So if git_config_source grows an enum to select the type, and
> all that filename-resolution gets pushed down into config_with_options(),
> the whole thing would fall out naturally, I think.
Makes sense.
| @@ -29,10 +29,11 @@ static int use_worktree_config; | |||
| static struct git_config_source given_config_source; | |||
There was a problem hiding this comment.
On the Git mailing list, Philip Oakley wrote (reply to this):
Hi Matthew,
On 18/12/2019 01:11, Matthew Rogers via GitGitGadget wrote:
> From: Matthew Rogers <[email protected]>
>
> Add new option --show-scope which allows a user to know what the scope
> of listed config options are (local/global/system/etc.).
>
> Signed-off-by: Matthew Rogers <[email protected]>
> ---
> builtin/config.c | 60 ++++++++++++++++++++++++++++++++++++++++-------
> t/t1300-config.sh | 51 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 103 insertions(+), 8 deletions(-)
Doesn't this also need a man page update as well for adding the option
to the synopsis.
The commit message doesn't fully highlight that the config list will
often be all the users config values, so each value will be
disambiguated/identified as to it's origin.
Philip
There was a problem hiding this comment.
On the Git mailing list, wrote (reply to this):
Junio,
>These are correct changes, but is unrelated noise in the context of
>the theme of the patch, no?
I think that's the case, would the recommended course of action be to
move these changes into its own commit?
>> + if (use_local_config || scope == CONFIG_SCOPE_REPO) {
>> + return "local";
>> + } else if (use_global_config || scope == CONFIG_SCOPE_GLOBAL) {
>> + return "global";
>> + } else if (use_system_config || scope == CONFIG_SCOPE_SYSTEM) {
>> + return "system";
>
>The above is slightly tricky; --global/--system/--local are made
>mutually exclusive in the higher level, so if any of them is set,
>we do not even need to look at the "scope" to tell what kind of
>source we are reading from.
So the way I structured these was to mirror the way other parts
of this file check if we should be doing a --local, etc. and mirrored
that here. This could definitely be cleaned up if we change the behavior
with how --local, etc. set the current_config_scope.
>> + } else if (given_config_source.use_stdin ||
>> + given_config_source.blob ||
>> + given_config_source.file ||
>> + scope == CONFIG_SCOPE_CMDLINE) {
>> + return "command line";
>
>I am not sure what the implication of saying "they came from the
>command line" when we read from the standard input or from a blob.
I agree with you here, I only put this as "command line"
because they came from a place that was ultimately fed in from
command line options (--file/--blob). I wouldn't have a problem with
calling them out as their own scope ("file", "blob", "stdin").
>> + } else {
>> + return "unknown";
>> + }
>> +}
>
>In any case, the need for such logic that says "scope might not say
>it is REPO, but when use_local_config is true, we are doing a local
>config" implies that "scope" parameter the caller of this function
>has is not set correctly when these options are used---would that be
>the real bug that needs fixing, rather than getting "worked around"
>with a code like this?
>
>It almost makes me point fingers at config.c::config_with_options()
>where config_source is inspected and git_config_from_*() helpers are
>called without setting the current_parsing_scope. Unlike these
>cases, when do_git_config_sequence() is called from that function,
>the scope is recorded in the variable before each standard config
>source file is opened and read. What would we be breaking if we
>taught the function to set the current_parsing_scope variable
>correctly even when reading from the config_source? That would
>certainly simplify this function quite a lot, but if the other parts
>of the codebase relies on the current behaviour, we cannot make such
>a change lightly.
From what I can tell from a cursory glance. the only two clients of
this function are remote.c and upload-pack.c. The usecase for remote.c
mostly seems to be to determine the result of `remote_is_configured()`
which (more importantly) seems to be done when that iterates through
the relevant configuration options. Similarly for upload-pack.c.
I don't think it would be harmful for git config --local, etc. to set that
as we would normally intuit.
>> +static void show_config_scope(struct strbuf *buf)
>> +{
>> + const char term = end_nul ? '\0' : '\t';
>> + const char *scope = scope_to_string(current_config_scope());
>> +
>> + strbuf_addch(buf, '(');
>> + if (end_nul)
>> + strbuf_addstr(buf, N_(scope));
>> + else
>> + quote_c_style(scope, buf, NULL, 0);
>
>Isn't this overkill? I think this code was copied-and-pasted from
>the other function that needs to show an arbitrary end-user supplied
>data which is a pathname, so it makes perfect sense to use c-style
>quoting, but the token scope_to_string() returns is taken from a
>bounded set that doesn't require such quoting, no?
Yeah, I guess that is a copy+paste mistake. I don't think its
necessary since we control the input into this function, So I'll fix
that.
Philip,
>Doesn't this also need a man page update as well for adding the option
>to the synopsis.
>
>The commit message doesn't fully highlight that the config list will
>often be all the users config values, so each value will be
>disambiguated/identified as to it's origin.
I'm agreed on these. So I'll look to readjust that.
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
<[email protected]> writes:
>>These are correct changes, but is unrelated noise in the context of
>>the theme of the patch, no?
>
> I think that's the case, would the recommended course of action be to
> move these changes into its own commit?
>
Yup, and it generally is a good idea to make such a clean-up patch
either early in the series, or as a standalone patch (with a note
under three-dash lines that another topic is coming on top of the
cleanup), because it would be much less likely to introduce new bugs
and can be merged quicly to 'next' and then to 'master' to serve as
a base to build your other changes on top.
There was a problem hiding this comment.
On the Git mailing list, Matt Rogers wrote (reply to this):
Philip,
>The commit message doesn't fully highlight that the config list will
>often be all the users config values, so each value will be
>disambiguated/identified as to it's origin.
I don't really understand what you mean by "it's origin" here. When
you say origin, do you mean in the "--show-origin" sense of "file/blob/etc."
or something else? Because scope is kind of an orthogonal concept to origin
in that sense as you can have files with different origins but the same scope.
Thanks,
Matt
On Thu, Dec 19, 2019 at 12:56 PM Junio C Hamano <[email protected]> wrote:
>
> <[email protected]> writes:
>
> >>These are correct changes, but is unrelated noise in the context of
> >>the theme of the patch, no?
> >
> > I think that's the case, would the recommended course of action be to
> > move these changes into its own commit?
> >
>
> Yup, and it generally is a good idea to make such a clean-up patch
> either early in the series, or as a standalone patch (with a note
> under three-dash lines that another topic is coming on top of the
> cleanup), because it would be much less likely to introduce new bugs
> and can be merged quicly to 'next' and then to 'master' to serve as
> a base to build your other changes on top.
>
--
Matthew Rogers
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Matt Rogers <[email protected]> writes:
> Philip,
>
>>The commit message doesn't fully highlight that the config list will
>>often be all the users config values, so each value will be
>>disambiguated/identified as to it's origin.
>
> I don't really understand what you mean by "it's origin" here. When
> you say origin, do you mean in the "--show-origin" sense of "file/blob/etc."
> or something else? Because scope is kind of an orthogonal concept to origin
> in that sense as you can have files with different origins but the same scope.
I do not think origin and scope are orghogonal, though. Can the
same file appear as the source for different configuration var-value
pair in two different scopes?
It is likely that you can _guess_ with high precision that given a
pathname reported by --show-origin what scope it is in. It on the
other hand is not so trivial given a scope to guess which exact file
a var-value pair came from, I would think.\
There was a problem hiding this comment.
On the Git mailing list, Matt Rogers wrote (reply to this):
On Fri, Dec 20, 2019 at 9:37 PM Junio C Hamano <[email protected]> wrote:
>
> I do not think origin and scope are orghogonal, though. Can the
> same file appear as the source for different configuration var-value
> pair in two different scopes?
I meant orthogonal in two senses:
That given the current implementation you don't need to have both
options active at
the same time but can have them active at both times.
And that origin and scope correlate, but aren't necessarily
one-for-one. For example, --show-origin lists in a known order, but it follows
includes and lists the origin as the included file. so if you include a file
globally which has includeif "gitdir:..." directives then it can get hairy when
all your config files are structured like that.
Although, to be fair I doubt that that kind of situation is normal
>
> It is likely that you can _guess_ with high precision that given a
> pathname reported by --show-origin what scope it is in. It on the
> other hand is not so trivial given a scope to guess which exact file
> a var-value pair came from, I would think.\
Normally yes, but things can get complicating depending on your
configuration/include situation.
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Matt Rogers <[email protected]> writes:
> And that origin and scope correlate, but aren't necessarily
> one-for-one.
Yes, exactly. When I see "orthogonal", I expect the word describes
things that do not correlate.
I can see values in the option that gives scope but not path in
order to write scripts that limits var-value pairs to be used
(e.g. "I do not want to be affected by the per-repository values",
"I do not trust settings that comes system-wide"). I also can see
values in the option that gives only path when debugging the
configuration file.
I briefly wondered, for the purpose of such "I do want to see only
those settings coming from these scopes" script, it may make more
sense to have the command _filter_ the var-value pairs, instead of
showing all of them with label, but that feature always exists ;-)
e5a8fe9 to
066d944
Compare
|
That's odd. There are still a lot of failures like this one: |
|
Yeah, this is something I'm trying to figure out since I last pushed it (and forgot to test it locally). Right now I'm trying to teach config to use a scope field that I added to |
ROGERSM94
force-pushed
the
add-config-flags
branch
2 times, most recently
from
9010981 to
92ce9b7
Compare
There are many situations where the scope of a config command is known beforehand, such as passing of '--local', '--file', etc. to an invocation of git config. However, this information is lost when moving from builtin/config.c to /config.c. This historically hasn't been a big deal, but to prepare for the upcoming --show-scope option we teach git_config_source to keep track of the source and the config machinery to use that information to set current_parsing_scope appropriately. Signed-off-by: Matthew Rogers <[email protected]>
Before the changes to teach git_config_source to remember scope information submodule-config.c never needed to consider the question of config scope. Even though zeroing out git_config_source is still correct and preserved the previous behavior of setting the scope to CONFIG_SCOPE_UNKNOWN, it's better to be explicit about such situations by explicitly setting the scope. As none of the current config_scope enumerations make sense we create CONFIG_SCOPE_SUBMODULE to describe the situation. Signed-off-by: Matthew Rogers <[email protected]>
When a user queries config values with --show-origin, often it's difficult to determine what the actual "scope" (local, global, etc.) of a given value is based on just the origin file. Teach 'git config' the '--show-scope' option to print the scope of all displayed config values. Note that we should never see anything of "submodule" scope as that is only ever used by submodule-config.c when parsing the '.gitmodules' file. Signed-off-by: Matthew Rogers <[email protected]>
ROGERSM94
force-pushed
the
add-config-flags
branch
from
fc141e8 to
f76463e
Compare
|
I'm not really sure about the failing tests (specifically "GitGitGadget PR Handler" and "CI for GitGItGadgets Git fork (Documentation)") I don't believe they're caused by these patches since I've seen a few other PR's that have the same failures, but would like to verify before submitting |
|
/submit |
|
Submitted as [email protected] |
The GitGitGadget PR Handler will complain about things like missing sign-offs and merge commits in the PR. But since you were able to submit, those issues seem to have been resolved. In the most recent instance, this Pipeline failed because a comment had been deleted after the Pipeline was triggered but before it was successfully started: https://dev.azure.com/gitgitgadget/git/_build/results?buildId=29680&view=logs&j=fd490c07-0b22-5182-fac9-6d67fe1e939b&t=28e14b79-1d95-5195-ee03-3a68ac48a418 About the "Documentation" job, I fixed this in GitGitGadget's PR builds last night, and also opened a PR to fix this in Git proper: git#707. |
| @@ -3297,6 +3297,22 @@ const char *current_config_origin_type(void) | |||
| } | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Matthew Rogers via GitGitGadget" <[email protected]> writes:
> From: Matthew Rogers <[email protected]>
>
> To prepare for the upcoming --show-scope option, we require the ability
> to convert a config_scope enum to a string. As this was originally
> implemented as a static function 'scope_name()' in
> t/helper/test-config.c, we expose it via config.h and give it a less
> ambiguous name 'config_scope_name()'
> Signed-off-by: Matthew Rogers <[email protected]>
> ---
> config.c | 16 ++++++++++++++++
> config.h | 2 ++
> t/helper/test-config.c | 17 +----------------
> t/t1308-config-set.sh | 2 +-
> 4 files changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/config.c b/config.c
> index d75f88ca0c..83bb98d65e 100644
> --- a/config.c
> +++ b/config.c
> @@ -3297,6 +3297,22 @@ const char *current_config_origin_type(void)
> }
> }
>
> +const char *config_scope_name(enum config_scope scope)
> +{
> + switch (scope) {
> + case CONFIG_SCOPE_SYSTEM:
> + return "system";
> + case CONFIG_SCOPE_GLOBAL:
> + return "global";
> + case CONFIG_SCOPE_REPO:
> + return "repo";
> + case CONFIG_SCOPE_CMDLINE:
> + return "command line";
The change from "cmdline" to "command line" does need to happen
before the end of the series, but I do not think it should happen
here, especialy given that the proposed log message explains that
this step is to expose scope_name() under a better name (which is a
very good split point).
How are you reviewing the patches in your own series before sending
them out? This round is better than the previous rounds where we
didn't have a matching change to the tests so "make test" may not
have passed in the middle of the series, though...
> + default:
> + return "unknown";
> + }
> +}
> +
> const char *current_config_name(void)
> {
> const char *name;
> diff --git a/config.h b/config.h
> index 91fd4c5e96..dcb8c274d4 100644
> --- a/config.h
> +++ b/config.h
> @@ -35,6 +35,7 @@ struct object_id;
>
> #define CONFIG_REGEX_NONE ((void *)1)
>
> +
> struct git_config_source {
> unsigned int use_stdin:1;
> const char *file;
> @@ -301,6 +302,7 @@ enum config_scope {
> CONFIG_SCOPE_REPO,
> CONFIG_SCOPE_CMDLINE,
> };
> +const char *config_scope_name(enum config_scope scope);
>
> enum config_scope current_config_scope(void);
> const char *current_config_origin_type(void);
> diff --git a/t/helper/test-config.c b/t/helper/test-config.c
> index 214003d5b2..1e3bc7c8f4 100644
> --- a/t/helper/test-config.c
> +++ b/t/helper/test-config.c
> @@ -37,21 +37,6 @@
> *
> */
>
> -static const char *scope_name(enum config_scope scope)
> -{
> - switch (scope) {
> - case CONFIG_SCOPE_SYSTEM:
> - return "system";
> - case CONFIG_SCOPE_GLOBAL:
> - return "global";
> - case CONFIG_SCOPE_REPO:
> - return "repo";
> - case CONFIG_SCOPE_CMDLINE:
> - return "cmdline";
> - default:
> - return "unknown";
> - }
> -}
> static int iterate_cb(const char *var, const char *value, void *data)
> {
> static int nr;
> @@ -63,7 +48,7 @@ static int iterate_cb(const char *var, const char *value, void *data)
> printf("value=%s\n", value ? value : "(null)");
> printf("origin=%s\n", current_config_origin_type());
> printf("name=%s\n", current_config_name());
> - printf("scope=%s\n", scope_name(current_config_scope()));
> + printf("scope=%s\n", config_scope_name(current_config_scope()));
>
> return 0;
> }
> diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
> index 7b4e1a63eb..5f3e71a160 100755
> --- a/t/t1308-config-set.sh
> +++ b/t/t1308-config-set.sh
> @@ -265,7 +265,7 @@ test_expect_success 'iteration shows correct origins' '
> value=from-cmdline
> origin=command line
> name=
> - scope=cmdline
> + scope=command line
> EOF
> GIT_CONFIG_PARAMETERS=$cmdline_config test-tool config iterate >actual &&
> test_cmp expect actual
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Junio C Hamano <[email protected]> writes:
> The change from "cmdline" to "command line" does need to happen
> before the end of the series, but I do not think it should happen
> here, especialy given that the proposed log message explains that
> this step is to expose scope_name() under a better name (which is a
> very good split point).
I'll tweak this step with the attached patch, and then adjust
06/10 as needed, while queuing.
config.c | 2 +-
config.h | 1 -
t/t1308-config-set.sh | 2 +-
3 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/config.c b/config.c
index 83bb98d65e..a922b136e5 100644
--- a/config.c
+++ b/config.c
@@ -3307,7 +3307,7 @@ const char *config_scope_name(enum config_scope scope)
case CONFIG_SCOPE_REPO:
return "repo";
case CONFIG_SCOPE_CMDLINE:
- return "command line";
+ return "cmdline";
default:
return "unknown";
}
diff --git a/config.h b/config.h
index dcb8c274d4..c063f33ff6 100644
--- a/config.h
+++ b/config.h
@@ -35,7 +35,6 @@ struct object_id;
#define CONFIG_REGEX_NONE ((void *)1)
-
struct git_config_source {
unsigned int use_stdin:1;
const char *file;
diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
index 5f3e71a160..7b4e1a63eb 100755
--- a/t/t1308-config-set.sh
+++ b/t/t1308-config-set.sh
@@ -265,7 +265,7 @@ test_expect_success 'iteration shows correct origins' '
value=from-cmdline
origin=command line
name=
- scope=command line
+ scope=cmdline
EOF
GIT_CONFIG_PARAMETERS=$cmdline_config test-tool config iterate >actual &&
test_cmp expect actual
There was a problem hiding this comment.
On the Git mailing list, Matt Rogers wrote (reply to this):
>
> How are you reviewing the patches in your own series before sending
> them out? This round is better than the previous rounds where we
> didn't have a matching change to the tests so "make test" may not
> have passed in the middle of the series, though...
>
I went through each patch individually using rebase -i and built/tested it.
Although just to save time I only did t1300 and t1308 since I believe those were
the only ones that should be affected. I can write a script that
would run the whole
test suite overnight for me and make sure the series shakes out okay,
if you'd like.
There was a problem hiding this comment.
On the Git mailing list, Emily Shaffer wrote (reply to this):
On Mon, Feb 10, 2020 at 07:30:22PM -0500, Matt Rogers wrote:
> >
> > How are you reviewing the patches in your own series before sending
> > them out? This round is better than the previous rounds where we
> > didn't have a matching change to the tests so "make test" may not
> > have passed in the middle of the series, though...
> >
>
> I went through each patch individually using rebase -i and built/tested it.
> Although just to save time I only did t1300 and t1308 since I believe those were
> the only ones that should be affected. I can write a script that
> would run the whole
> test suite overnight for me and make sure the series shakes out okay,
> if you'd like.
Not sure whether you intend to do this or not, but to maybe save you
some scripting, I do this like so:
git rebase -x \
"make -j16 && (cd t && prove -j16 -v --shuffle t[0-9]*.sh)" master
If the tests fail, the rebase is paused, which can be understandably
disappointing if you walk away from it overnight only to have it fail
after 10 minutes, though. :)
- Emily
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Matt Rogers <[email protected]> writes:
>> How are you reviewing the patches in your own series before sending
>> them out? This round is better than the previous rounds where we
>> didn't have a matching change to the tests so "make test" may not
>> have passed in the middle of the series, though...
>>
>
> I went through each patch individually using rebase -i and built/tested it.
> Although just to save time I only did t1300 and t1308 since I believe those were
> the only ones that should be affected. I can write a script that
> would run the whole
> test suite overnight for me and make sure the series shakes out okay,
> if you'd like.
What I like does not matter.
What I pointed out for 04/10 wouldn't have been caught by your
testing anyway, as both the code and the test had matching
unnecessry changes. I was wondering if you are relying too heavily
on just tests and without actually proofreading the changes to see
if they still make sense in the context of the updated series, and
if my suspicion was correct, if there are something reviewers can do
to help the authors.
There was a problem hiding this comment.
On the Git mailing list, Matt Rogers wrote (reply to this):
On Tue, Feb 11, 2020 at 1:10 AM Junio C Hamano <[email protected]> wrote:
>
> Matt Rogers <[email protected]> writes:
>
> >> How are you reviewing the patches in your own series before sending
> >> them out? This round is better than the previous rounds where we
> >> didn't have a matching change to the tests so "make test" may not
> >> have passed in the middle of the series, though...
> >>
> >
> > I went through each patch individually using rebase -i and built/tested it.
> > Although just to save time I only did t1300 and t1308 since I believe those were
> > the only ones that should be affected. I can write a script that
> > would run the whole
> > test suite overnight for me and make sure the series shakes out okay,
> > if you'd like.
>
> What I like does not matter.
>
> What I pointed out for 04/10 wouldn't have been caught by your
> testing anyway, as both the code and the test had matching
> unnecessry changes. I was wondering if you are relying too heavily
> on just tests and without actually proofreading the changes to see
> if they still make sense in the context of the updated series, and
> if my suspicion was correct, if there are something reviewers can do
> to help the authors.
>
>
I do try to proofread patches, I'm just not the most careful of reviewers at
times, partially as a personal problem and partially as this is a new workflow
for me. As for the particular issue, I just thought it was a good idea at the
time and I didn't think it all the way through
--
Matthew Rogers
| @@ -1724,15 +1724,12 @@ static int do_git_config_sequence(const struct config_options *opts, | |||
| if (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK)) | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Matthew Rogers via GitGitGadget" <[email protected]> writes:
> From: Matthew Rogers <[email protected]>
>
> Previously when iterating through git config variables, worktree config
> and local config were both considered "CONFIG_SCOPE_REPO". This was
> never a problem before as no one had needed to differentiate between the
> two cases, but future functionality may care whether or not the config
> options come from a worktree or from the repository's actual local
> config file. For example, the planned feature to add a '--show-scope'
> to config to allow a user to see which scope listed config options come
> from would confuse users if it just printed 'repo' rather than 'local'
> or 'worktree' as the documentation would lead them to expect. As well
> as the additional benefit of making the implementation look more like
> how the documentation describes the interface.
>
> To accomplish this we split out what was previously considered repo
> scope to be local and worktree.
>
> The clients of 'current_config_scope()' who cared about
> CONFIG_SCOPE_REPO are also modified to similarly care about
> CONFIG_SCOPE_WORKTREE and CONFIG_SCOPE_LOCAL to preserve previous behavior.
>
> Signed-off-by: Matthew Rogers <[email protected]>
> Signed-off-by: Junio C Hamano <[email protected]>
> ---
> config.c | 13 ++++++-------
> config.h | 3 ++-
> remote.c | 3 ++-
> t/t1308-config-set.sh | 2 +-
> upload-pack.c | 3 ++-
> 5 files changed, 13 insertions(+), 11 deletions(-)
Makes sense. And this step does exactly what it claims to do and
nothing else, which is very good ;-)
> diff --git a/config.c b/config.c
> index 83bb98d65e..7422bdebb1 100644
> --- a/config.c
> +++ b/config.c
> @@ -1724,15 +1724,12 @@ static int do_git_config_sequence(const struct config_options *opts,
> if (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK))
> ret += git_config_from_file(fn, user_config, data);
>
> - current_parsing_scope = CONFIG_SCOPE_REPO;
> + current_parsing_scope = CONFIG_SCOPE_LOCAL;
> if (!opts->ignore_repo && repo_config &&
> !access_or_die(repo_config, R_OK, 0))
> ret += git_config_from_file(fn, repo_config, data);
>
> - /*
> - * Note: this should have a new scope, CONFIG_SCOPE_WORKTREE.
> - * But let's not complicate things before it's actually needed.
> - */
> + current_parsing_scope = CONFIG_SCOPE_WORKTREE;
> if (!opts->ignore_worktree && repository_format_worktree_config) {
> char *path = git_pathdup("config.worktree");
> if (!access_or_die(path, R_OK, 0))
> @@ -3304,8 +3301,10 @@ const char *config_scope_name(enum config_scope scope)
> return "system";
> case CONFIG_SCOPE_GLOBAL:
> return "global";
> - case CONFIG_SCOPE_REPO:
> - return "repo";
> + case CONFIG_SCOPE_LOCAL:
> + return "local";
> + case CONFIG_SCOPE_WORKTREE:
> + return "worktree";
> case CONFIG_SCOPE_CMDLINE:
> return "command line";
> default:
> diff --git a/config.h b/config.h
> index dcb8c274d4..d3ed41ef8e 100644
> --- a/config.h
> +++ b/config.h
> @@ -299,7 +299,8 @@ enum config_scope {
> CONFIG_SCOPE_UNKNOWN = 0,
> CONFIG_SCOPE_SYSTEM,
> CONFIG_SCOPE_GLOBAL,
> - CONFIG_SCOPE_REPO,
> + CONFIG_SCOPE_LOCAL,
> + CONFIG_SCOPE_WORKTREE,
> CONFIG_SCOPE_CMDLINE,
> };
> const char *config_scope_name(enum config_scope scope);
> diff --git a/remote.c b/remote.c
> index 5c4666b53a..593ce297ed 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -369,7 +369,8 @@ static int handle_config(const char *key, const char *value, void *cb)
> }
> remote = make_remote(name, namelen);
> remote->origin = REMOTE_CONFIG;
> - if (current_config_scope() == CONFIG_SCOPE_REPO)
> + if (current_config_scope() == CONFIG_SCOPE_LOCAL ||
> + current_config_scope() == CONFIG_SCOPE_WORKTREE)
> remote->configured_in_repo = 1;
> if (!strcmp(subkey, "mirror"))
> remote->mirror = git_config_bool(key, value);
> diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
> index 5f3e71a160..728a2b87ce 100755
> --- a/t/t1308-config-set.sh
> +++ b/t/t1308-config-set.sh
> @@ -259,7 +259,7 @@ test_expect_success 'iteration shows correct origins' '
> value=from-repo
> origin=file
> name=.git/config
> - scope=repo
> + scope=local
>
> key=foo.bar
> value=from-cmdline
> diff --git a/upload-pack.c b/upload-pack.c
> index a00d7ece6b..c53249cac1 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -1073,7 +1073,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused)
> precomposed_unicode = git_config_bool(var, value);
> }
>
> - if (current_config_scope() != CONFIG_SCOPE_REPO) {
> + if (current_config_scope() != CONFIG_SCOPE_LOCAL &&
> + current_config_scope() != CONFIG_SCOPE_WORKTREE) {
> if (!strcmp("uploadpack.packobjectshook", var))
> return git_config_string(&pack_objects_hook, var, value);
> }
| @@ -1724,23 +1724,20 @@ static int do_git_config_sequence(const struct config_options *opts, | |||
| if (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK)) | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Matthew Rogers via GitGitGadget" <[email protected]> writes:
> From: Matthew Rogers <[email protected]>
>
> CONFIG_SCOPE_CMDLINE is generally used in the code to refer to config
> values passed in via the -c option. Options passed in using this
> mechanism share similar scoping characteristics with the --file and
> --blob options of the 'config' command, namely that they are only in use
> for that single invocation of git, and that they supersede the normal
> system/global/local hierarchy. This patch introduces
> CONFIG_SCOPE_COMMAND to reflect this new idea, which also makes
> CONFIG_SCOPE_CMDLINE redundant.
>
> Signed-off-by: Matthew Rogers <[email protected]>
> Signed-off-by: Junio C Hamano <[email protected]>
> ---
> config.c | 6 +++---
> config.h | 2 +-
> t/t1308-config-set.sh | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
This makes sense, even though it is halfway undoing what 04/10
unnecessarily did ;-)
I think I can just wiggle minor fixes in to 04/10 and then adjust
this one to get the series into a better shape myself before queuing
them to expedite the process. Let me finish reading the remainder
of the topic.
Thanks.
> diff --git a/config.c b/config.c
> index 7422bdebb1..fe1e44a43a 100644
> --- a/config.c
> +++ b/config.c
> @@ -1737,7 +1737,7 @@ static int do_git_config_sequence(const struct config_options *opts,
> free(path);
> }
>
> - current_parsing_scope = CONFIG_SCOPE_CMDLINE;
> + current_parsing_scope = CONFIG_SCOPE_COMMAND;
> if (!opts->ignore_cmdline && git_config_from_parameters(fn, data) < 0)
> die(_("unable to parse command-line config"));
>
> @@ -3305,8 +3305,8 @@ const char *config_scope_name(enum config_scope scope)
> return "local";
> case CONFIG_SCOPE_WORKTREE:
> return "worktree";
> - case CONFIG_SCOPE_CMDLINE:
> - return "command line";
> + case CONFIG_SCOPE_COMMAND:
> + return "command";
> default:
> return "unknown";
> }
> diff --git a/config.h b/config.h
> index d3ed41ef8e..b570f4ce43 100644
> --- a/config.h
> +++ b/config.h
> @@ -301,7 +301,7 @@ enum config_scope {
> CONFIG_SCOPE_GLOBAL,
> CONFIG_SCOPE_LOCAL,
> CONFIG_SCOPE_WORKTREE,
> - CONFIG_SCOPE_CMDLINE,
> + CONFIG_SCOPE_COMMAND,
> };
> const char *config_scope_name(enum config_scope scope);
>
> diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
> index 728a2b87ce..fba0abe429 100755
> --- a/t/t1308-config-set.sh
> +++ b/t/t1308-config-set.sh
> @@ -265,7 +265,7 @@ test_expect_success 'iteration shows correct origins' '
> value=from-cmdline
> origin=command line
> name=
> - scope=command line
> + scope=command
> EOF
> GIT_CONFIG_PARAMETERS=$cmdline_config test-tool config iterate >actual &&
> test_cmp expect actual
| @@ -1702,6 +1702,7 @@ static int do_git_config_sequence(const struct config_options *opts, | |||
| char *xdg_config = xdg_config_home("config"); | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Matthew Rogers via GitGitGadget" <[email protected]> writes:
> From: Matthew Rogers <[email protected]>
>
> do_git_config_sequence operated under the assumption that it was correct
> to set current_parsing_scope to CONFIG_SCOPE_UNKNOWN as part of the
> cleanup it does after it finishes execution. This is incorrect, as it
> blows away the current_parsing_scope if do_git_config_sequence is called
> recursively. As such situations are rare (git config running with the
> '--blob' option is one example) this has yet to cause a problem, but the
> upcoming '--show-scope' option will experience issues in that case, lets
> teach do_git_config_sequence to preserve the current_parsing_scope from
> before it started execution.
>
> Signed-off-by: Matthew Rogers <[email protected]>
> ---
> config.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
This step is new and it does make sense.
Thanks.
> diff --git a/config.c b/config.c
> index fe1e44a43a..0e2c693e78 100644
> --- a/config.c
> +++ b/config.c
> @@ -1702,6 +1702,7 @@ static int do_git_config_sequence(const struct config_options *opts,
> char *xdg_config = xdg_config_home("config");
> char *user_config = expand_user_path("~/.gitconfig", 0);
> char *repo_config;
> + enum config_scope prev_parsing_scope = current_parsing_scope;
>
> if (opts->commondir)
> repo_config = mkpathdup("%s/config", opts->commondir);
> @@ -1741,7 +1742,7 @@ static int do_git_config_sequence(const struct config_options *opts,
> if (!opts->ignore_cmdline && git_config_from_parameters(fn, data) < 0)
> die(_("unable to parse command-line config"));
>
> - current_parsing_scope = CONFIG_SCOPE_UNKNOWN;
> + current_parsing_scope = prev_parsing_scope;
> free(xdg_config);
> free(user_config);
> free(repo_config);
| @@ -29,7 +29,7 @@ static int use_worktree_config; | |||
| static struct git_config_source given_config_source; | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Matthew Rogers via GitGitGadget" <[email protected]> writes:
> From: Matthew Rogers <[email protected]>
>
> There are many situations where the scope of a config command is known
> beforehand, such as passing of '--local', '--file', etc. to an
> invocation of git config. However, this information is lost when moving
> from builtin/config.c to /config.c. This historically hasn't been a big
> deal, but to prepare for the upcoming --show-scope option we teach
> git_config_source to keep track of the source and the config machinery
> to use that information to set current_parsing_scope appropriately.
>
> Signed-off-by: Matthew Rogers <[email protected]>
> ---
> builtin/config.c | 16 +++++++++++++---
> config.c | 3 +++
> config.h | 20 ++++++++++----------
> 3 files changed, 26 insertions(+), 13 deletions(-)
This is split from the last step in the previous round, and the
splitting makes sense.
Thanks.
| @@ -1702,6 +1702,7 @@ static int do_git_config_sequence(const struct config_options *opts, | |||
| char *xdg_config = xdg_config_home("config"); | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Matthew Rogers via GitGitGadget" <[email protected]> writes:
> From: Matthew Rogers <[email protected]>
>
> Before the changes to teach git_config_source to remember scope
> information submodule-config.c never needed to consider the question of
> config scope. Even though zeroing out git_config_source is still
> correct and preserved the previous behavior of setting the scope to
> CONFIG_SCOPE_UNKNOWN, it's better to be explicit about such situations
> by explicitly setting the scope. As none of the current config_scope
> enumerations make sense we create CONFIG_SCOPE_SUBMODULE to describe the
> situation.
>
> Signed-off-by: Matthew Rogers <[email protected]>
> ---
Other than a typo on the title, I think this one is perfect ;-)
Will queue.
> config.c | 2 ++
> config.h | 1 +
> submodule-config.c | 4 +++-
> 3 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/config.c b/config.c
> index 9b6afca210..18a6bdd9ff 100644
> --- a/config.c
> +++ b/config.c
> @@ -3311,6 +3311,8 @@ const char *config_scope_name(enum config_scope scope)
> return "worktree";
> case CONFIG_SCOPE_COMMAND:
> return "command";
> + case CONFIG_SCOPE_SUBMODULE:
> + return "submodule";
> default:
> return "unknown";
> }
> diff --git a/config.h b/config.h
> index 165cacb7da..fe0addb0dc 100644
> --- a/config.h
> +++ b/config.h
> @@ -42,6 +42,7 @@ enum config_scope {
> CONFIG_SCOPE_LOCAL,
> CONFIG_SCOPE_WORKTREE,
> CONFIG_SCOPE_COMMAND,
> + CONFIG_SCOPE_SUBMODULE,
> };
> const char *config_scope_name(enum config_scope scope);
>
> diff --git a/submodule-config.c b/submodule-config.c
> index 85064810b2..b8e97d8ae8 100644
> --- a/submodule-config.c
> +++ b/submodule-config.c
> @@ -635,7 +635,9 @@ static void submodule_cache_check_init(struct repository *repo)
> static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void *data)
> {
> if (repo->worktree) {
> - struct git_config_source config_source = { 0 };
> + struct git_config_source config_source = {
> + 0, .scope = CONFIG_SCOPE_SUBMODULE
> + };
> const struct config_options opts = { 0 };
> struct object_id oid;
> char *file;
|
This patch series was integrated into pu via git@f487ffe. |
|
This patch series was integrated into pu via git@a91d60f. |
|
This patch series was integrated into pu via git@3599796. |
|
This patch series was integrated into next via git@904bca0. |
|
This patch series was integrated into pu via git@85b6131. |
|
This patch series was integrated into pu via git@5c2a670. |
|
This patch series was integrated into pu via git@af72e74. |
|
This patch series was integrated into pu via git@9a92e68. |
|
This patch series was integrated into pu via git@5d55554. |
|
This patch series was integrated into next via git@5d55554. |
|
This patch series was integrated into master via git@5d55554. |
|
Closed via 5d55554. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was originally a pull request to the git-for-windows repository. It adds a new option --show-scope which would allow a user to see what scope a given configuration value has (sytem, local, global, etc.).
changes since v6:
Split patch 6: config: add '--show-scope'... into several parts, including moving the exposure of config_scope_name into its own commit earlier in the series.