| Message ID | 20181023190423.5772-3-peartben@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | speed up git reset | expand |
On 23/10/2018 20:04, Ben Peart wrote: > From: Ben Peart <benpeart@microsoft.com> Sorry for the late reply, ... I've been away from email - I am still trying to catch up. > > Add a reset.quiet config setting that sets the default value of the --quiet > flag when running the reset command. This enables users to change the > default behavior to take advantage of the performance advantages of > avoiding the scan for unstaged changes after reset. Defaults to false. > > Signed-off-by: Ben Peart <benpeart@microsoft.com> > --- > Documentation/config.txt | 3 +++ > Documentation/git-reset.txt | 5 ++++- > builtin/reset.c | 1 + > 3 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/Documentation/config.txt b/Documentation/config.txt > index f6f4c21a54..a2d1b8b116 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -2728,6 +2728,9 @@ rerere.enabled:: > `$GIT_DIR`, e.g. if "rerere" was previously used in the > repository. > > +reset.quiet:: > + When set to true, 'git reset' will default to the '--quiet' option. Mention that this 'Defaults to false'? > + > include::sendemail-config.txt[] > > sequence.editor:: > diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt > index 1d697d9962..2dac95c71a 100644 > --- a/Documentation/git-reset.txt > +++ b/Documentation/git-reset.txt > @@ -95,7 +95,10 @@ OPTIONS > > -q:: > --quiet:: > - Be quiet, only report errors. > +--no-quiet:: > + Be quiet, only report errors. The default behavior is set by the > + `reset.quiet` config option. `--quiet` and `--no-quiet` will > + override the default behavior. Better than last time, but how about something like: -q:: --quiet:: --no-quiet:: Be quiet, only report errors. The default behaviour of the command, which is to not be quiet, can be specified by the `reset.quiet` configuration variable. The `--quiet` and `--no-quiet` options can be used to override any configured default. Hmm, I am not sure that is any better! :-D Also, note that the --no-option is often described separately to the --option (in a separate paragraph). I don't know if that would help here. [The default behaviour is _not_ set by the configuration, if no configuration is specified. :-P ] Not sure if that helps! ATB, Ramsay Jones > > > EXAMPLES > diff --git a/builtin/reset.c b/builtin/reset.c > index 04f0d9b4f5..3b43aee544 100644 > --- a/builtin/reset.c > +++ b/builtin/reset.c > @@ -306,6 +306,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) > }; > > git_config(git_reset_config, NULL); > + git_config_get_bool("reset.quiet", &quiet); > > argc = parse_options(argc, argv, prefix, options, git_reset_usage, > PARSE_OPT_KEEP_DASHDASH); >
Ramsay Jones <ramsay@ramsayjones.plus.com> writes: >> diff --git a/Documentation/config.txt b/Documentation/config.txt >> index f6f4c21a54..a2d1b8b116 100644 >> --- a/Documentation/config.txt >> +++ b/Documentation/config.txt >> @@ -2728,6 +2728,9 @@ rerere.enabled:: >> `$GIT_DIR`, e.g. if "rerere" was previously used in the >> repository. >> >> +reset.quiet:: >> + When set to true, 'git reset' will default to the '--quiet' option. > > Mention that this 'Defaults to false'? Perhaps. >> -q:: >> --quiet:: >> - Be quiet, only report errors. >> +--no-quiet:: >> + Be quiet, only report errors. The default behavior is set by the >> + `reset.quiet` config option. `--quiet` and `--no-quiet` will >> + override the default behavior. > > Better than last time, but how about something like: > > -q:: > --quiet:: > --no-quiet:: > Be quiet, only report errors. The default behaviour of the > command, which is to not be quiet, can be specified by the > `reset.quiet` configuration variable. The `--quiet` and > `--no-quiet` options can be used to override any configured > default. > > Hmm, I am not sure that is any better! :-D To be honest, I find the second sentence in your rewrite even more confusing. It reads as if `reset.quiet` configuration variable can be used to restore the "show what is yet to be added" behaviour, due to the parenthetical mention of the default behaviour without any configuration. The command reports what is yet to be added to the index after `reset` by default. It can be made to only report errors with the `--quiet` option, or setting `reset.quiet` configuration variable to `true` (the latter can be overriden with `--no-quiet`). That may not be much better, though X-<.
Junio C Hamano <gitster@pobox.com> writes: > To be honest, I find the second sentence in your rewrite even more > confusing. It reads as if `reset.quiet` configuration variable > can be used to restore the "show what is yet to be added" > behaviour, due to the parenthetical mention of the default behaviour > without any configuration. > > The command reports what is yet to be added to the index > after `reset` by default. It can be made to only report > errors with the `--quiet` option, or setting `reset.quiet` > configuration variable to `true` (the latter can be > overriden with `--no-quiet`). > > That may not be much better, though X-<. In any case, the comments are getting closer to the bikeshedding territory, that can be easily addressed incrementally. I am getting the impression that everbody agrees that the change is desirable, sufficiently documented and properly implemented. Shall we mark it for "Will merge to 'next'" in the what's cooking report and leave further refinements to incremental updates as needed?
On 10/25/2018 5:26 AM, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > >> To be honest, I find the second sentence in your rewrite even more >> confusing. It reads as if `reset.quiet` configuration variable >> can be used to restore the "show what is yet to be added" >> behaviour, due to the parenthetical mention of the default behaviour >> without any configuration. >> >> The command reports what is yet to be added to the index >> after `reset` by default. It can be made to only report >> errors with the `--quiet` option, or setting `reset.quiet` >> configuration variable to `true` (the latter can be >> overriden with `--no-quiet`). >> >> That may not be much better, though X-<. > > In any case, the comments are getting closer to the bikeshedding > territory, that can be easily addressed incrementally. I am getting > the impression that everbody agrees that the change is desirable, > sufficiently documented and properly implemented. > > Shall we mark it for "Will merge to 'next'" in the what's cooking > report and leave further refinements to incremental updates as > needed? > While not great, I think it is good enough. I don't think either of the last couple of rewrite attempts were clearly better than what is in the latest patch. I'd agree we should merge to 'next' and if someone comes up with something great, we can update it then.
On 25/10/2018 10:26, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > >> To be honest, I find the second sentence in your rewrite even more >> confusing. It reads as if `reset.quiet` configuration variable >> can be used to restore the "show what is yet to be added" >> behaviour, due to the parenthetical mention of the default behaviour >> without any configuration. >> >> The command reports what is yet to be added to the index >> after `reset` by default. It can be made to only report >> errors with the `--quiet` option, or setting `reset.quiet` >> configuration variable to `true` (the latter can be >> overriden with `--no-quiet`). >> >> That may not be much better, though X-<. > > In any case, the comments are getting closer to the bikeshedding > territory, that can be easily addressed incrementally. I am getting > the impression that everbody agrees that the change is desirable, > sufficiently documented and properly implemented. > > Shall we mark it for "Will merge to 'next'" in the what's cooking > report and leave further refinements to incremental updates as > needed? Yeah, the first version gave me a 'huh?' moment (hence the comment), the last version was better and, as you can see, I am no great shakes at wordsmith-ing documentation! ;-) Thanks! ATB, Ramsay Jones
diff --git a/Documentation/config.txt b/Documentation/config.txt index f6f4c21a54..a2d1b8b116 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2728,6 +2728,9 @@ rerere.enabled:: `$GIT_DIR`, e.g. if "rerere" was previously used in the repository. +reset.quiet:: + When set to true, 'git reset' will default to the '--quiet' option. + include::sendemail-config.txt[] sequence.editor:: diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt index 1d697d9962..2dac95c71a 100644 --- a/Documentation/git-reset.txt +++ b/Documentation/git-reset.txt @@ -95,7 +95,10 @@ OPTIONS -q:: --quiet:: - Be quiet, only report errors. +--no-quiet:: + Be quiet, only report errors. The default behavior is set by the + `reset.quiet` config option. `--quiet` and `--no-quiet` will + override the default behavior. EXAMPLES diff --git a/builtin/reset.c b/builtin/reset.c index 04f0d9b4f5..3b43aee544 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -306,6 +306,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) }; git_config(git_reset_config, NULL); + git_config_get_bool("reset.quiet", &quiet); argc = parse_options(argc, argv, prefix, options, git_reset_usage, PARSE_OPT_KEEP_DASHDASH);