{"thread":{"id":"49595","subject":"[PATCH v1 0/2] speed up git reset","startedAt":"2018-10-17T16:40:33Z","lastAt":"2018-10-25T17:04:27Z","messageCount":63,"participants":["Ben Peart","Eric Sunshine","Jeff King","Junio C Hamano","Duy Nguyen","Ramsay Jones","Johannes Schindelin","Ævar Arnfjörð Bjarmason","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"360760","messageId":"20181017164021.15204-1-peartben@gmail.com","threadId":"49595","inReplyTo":null,"subject":"[PATCH v1 0/2] speed up git reset","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-17T16:40:19Z","receivedAt":"2018-10-17T16:40:33Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"From: Ben Peart \n\nThe reset (mixed) command unstages the specified file(s) and then shows you\nthe remaining unstaged changes. This can make the command slow on larger\nrepos because at the end it calls refresh_index() which has a single thread\nthat loops through all the entries calling lstat() for every file.\n\nIf the user passes the --quiet switch, reset doesn�t display the remaining\nunstaged changes but it still does all the work to find them, it just\ndoesn�t print them out so passing \"--quiet\" doesn�t help performance.\n\nThis patch series will:\n\n1) change the behavior of \"git reset --quiet\" so that it no longer computes\n the remaining unstaged changes.\n \n2) add a new config setting so that \"--quiet\" can be configured as the default\n so that the default performance of \"git reset\" is improved.\n \nThe performance benefit of this can be significant. In a repo with 200K files\n\"git reset foo\" performance drops from 7.16 seconds to 0.32 seconds for a\nsavings of 96%. Even with the small git repo, reset times drop from 0.191\nseconds to 0.043 seconds for a savings of 77%.\n\nBase Ref: master\nWeb-Diff: https://github.com/benpeart/git/commit/2295a310d0\nCheckout: git fetch https://github.com/benpeart/git reset-refresh-index-v1 && git checkout 2295a310d0\n\nBen Peart (2):\n reset: don't compute unstaged changes after reset when --quiet\n reset: add new reset.quietDefault config setting\n\n Documentation/config.txt | 6 ++++++\n Documentation/git-reset.txt | 4 +++-\n builtin/reset.c | 3 ++-\n 3 files changed, 11 insertions(+), 2 deletions(-)\n\n\nbase-commit: a4b8ab5363a32f283a61ef3a962853556d136c0e\n-- \n2.18.0.windows.1\n\n\n"},{"id":"360761","messageId":"20181017164021.15204-2-peartben@gmail.com","threadId":"49595","inReplyTo":"20181017164021.15204-1-peartben@gmail.com","subject":"[PATCH v1 1/2] reset: don't compute unstaged changes after reset when --quiet","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-17T16:40:20Z","receivedAt":"2018-10-17T16:40:34Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"From: Ben Peart \n\nWhen git reset is run with the --quiet flag, don't bother finding any\nadditional unstaged changes as they won't be output anyway. This speeds up\nthe git reset command by avoiding having to lstat() every file looking for\nchanges that aren't going to be reported anyway.\n\nThe savings can be significant. In a repo with 200K files \"git reset\"\ndrops from 7.16 seconds to 0.32 seconds for a savings of 96%.\n\nSigned-off-by: Ben Peart \n---\n Documentation/git-reset.txt | 4 +++-\n builtin/reset.c | 2 +-\n 2 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex 1d697d9962..8610309b55 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -95,7 +95,9 @@ OPTIONS\n \n -q::\n --quiet::\n-\tBe quiet, only report errors.\n+\tBe quiet, only report errors. Can optimize the performance of reset\n+\tby avoiding scaning all files in the repo looking for additional\n+\tunstaged changes.\n \n \n EXAMPLES\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 11cd0dcb8c..0822798616 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -376,7 +376,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tif (read_from_tree(&pathspec, &oid, intent_to_add))\n \t\t\t\treturn 1;\n \t\t\tif (get_git_work_tree())\n-\t\t\t\trefresh_index(&the_index, flags, NULL, NULL,\n+\t\t\t\trefresh_index(&the_index, flags, quiet ? &pathspec : NULL, NULL,\n \t\t\t\t\t _(\"Unstaged changes after reset:\"));\n \t\t} else {\n \t\t\tint err = reset_index(&oid, reset_type, quiet);\n-- \n2.18.0.windows.1\n\n"},{"id":"360762","messageId":"20181017164021.15204-3-peartben@gmail.com","threadId":"49595","inReplyTo":"20181017164021.15204-1-peartben@gmail.com","subject":"[PATCH v1 2/2] reset: add new reset.quietDefault config setting","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-17T16:40:21Z","receivedAt":"2018-10-17T16:40:36Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"From: Ben Peart \n\nAdd a reset.quietDefault config setting that sets the default value of the\n--quiet flag when running the reset command. This enables users to change\nthe default behavior to take advantage of the performance advantages of\navoiding the scan for unstaged changes after reset. Defaults to false.\n\nSigned-off-by: Ben Peart \n---\n Documentation/config.txt | 6 ++++++\n builtin/reset.c | 1 +\n 2 files changed, 7 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex f6f4c21a54..a5cf4c019b 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2728,6 +2728,12 @@ rerere.enabled::\n \t`$GIT_DIR`, e.g. if \"rerere\" was previously used in the\n \trepository.\n \n+reset.quietDefault::\n+\tSets the default value of the \"quiet\" option for the reset command.\n+\tChoosing \"quiet\" can optimize the performance of the reset command\n+\tby avoiding the scan of all files in the repo looking for additional\n+\tunstaged changes. Defaults to false.\n+\n include::sendemail-config.txt[]\n \n sequence.editor::\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 0822798616..7d151d48a0 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -306,6 +306,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t};\n \n \tgit_config(git_reset_config, NULL);\n+\tgit_config_get_bool(\"reset.quietDefault\", &quiet);\n \n \targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n-- \n2.18.0.windows.1\n\n"},{"id":"360770","messageId":"CAPig+cSiE-M9QMch4WE7y4cib1FBUNiaR2pGGtbDuqiz6juhaw@mail.gmail.com","threadId":"49595","inReplyTo":"20181017164021.15204-2-peartben@gmail.com","subject":"Re: [PATCH v1 1/2] reset: don't compute unstaged changes after reset when --quiet","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-10-17T18:14:32Z","receivedAt":"2018-10-17T18:14:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Oct 17, 2018 at 12:40 PM Ben Peart wrote:\n> When git reset is run with the --quiet flag, don't bother finding any\n> additional unstaged changes as they won't be output anyway. This speeds up\n> the git reset command by avoiding having to lstat() every file looking for\n> changes that aren't going to be reported anyway.\n>\n> Signed-off-by: Ben Peart \n> ---\n> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\n> @@ -95,7 +95,9 @@ OPTIONS\n> --quiet::\n> - Be quiet, only report errors.\n> + Be quiet, only report errors. Can optimize the performance of reset\n> + by avoiding scaning all files in the repo looking for additional\n> + unstaged changes.\n\ns/scaning/scanning/\n\nHowever, I'm not convinced that this should be documented here or at\nleast in this fashion. When I read this new documentation before\nreading the commit message, I was baffled by what it was trying to say\nsince --quiet'ness is a superficial quality, not an optimizer. My\nknee-jerk reaction is that it doesn't belong in end-user documentation\nat all since it's an implementation detail, however, I can see that\nsuch knowledge could be handy for people in situations which would be\nhelped by this. That said, if you do document it, this doesn't feel\nlike the correct place to do so; it should be in a \"Discussion\"\nsection or something. (Who would expect to find --quiet documentation\ntalking about optimizations? Likely, nobody.)\n"},{"id":"360773","messageId":"CAPig+cQ3ia78pLtnHSq8tM3B-XnFgWhwowJxwacYEEzXosJ16g@mail.gmail.com","threadId":"49595","inReplyTo":"20181017164021.15204-3-peartben@gmail.com","subject":"Re: [PATCH v1 2/2] reset: add new reset.quietDefault config setting","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-10-17T18:19:59Z","receivedAt":"2018-10-17T18:20:12Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Oct 17, 2018 at 12:40 PM Ben Peart wrote:\n> Add a reset.quietDefault config setting that sets the default value of the\n> --quiet flag when running the reset command. This enables users to change\n> the default behavior to take advantage of the performance advantages of\n> avoiding the scan for unstaged changes after reset. Defaults to false.\n\nAs with the previous patch, my knee-jerk reaction is that this really\nfeels wrong being tied to --quiet. It's particularly unintuitive.\n\nWhat I _could_ see, and what would feel more natural is if you add a\nnew option (say, --optimize) which is more general, incorporating\nwhatever optimizations become available in the future, not just this\none special-case. A side-effect of --optimize is that it implies\n--quiet, and that is something which can and should be documented.\n\n> Signed-off-by: Ben Peart \n"},{"id":"360774","messageId":"20181017182255.GC28326@sigill.intra.peff.net","threadId":"49595","inReplyTo":"CAPig+cSiE-M9QMch4WE7y4cib1FBUNiaR2pGGtbDuqiz6juhaw@mail.gmail.com","subject":"Re: [PATCH v1 1/2] reset: don't compute unstaged changes after reset when --quiet","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-17T18:22:56Z","receivedAt":"2018-10-17T18:22:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 17, 2018 at 02:14:32PM -0400, Eric Sunshine wrote:\n\n> > diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\n> > @@ -95,7 +95,9 @@ OPTIONS\n> > --quiet::\n> > - Be quiet, only report errors.\n> > + Be quiet, only report errors. Can optimize the performance of reset\n> > + by avoiding scaning all files in the repo looking for additional\n> > + unstaged changes.\n> \n> s/scaning/scanning/\n> \n> However, I'm not convinced that this should be documented here or at\n> least in this fashion. When I read this new documentation before\n> reading the commit message, I was baffled by what it was trying to say\n> since --quiet'ness is a superficial quality, not an optimizer. My\n> knee-jerk reaction is that it doesn't belong in end-user documentation\n> at all since it's an implementation detail, however, I can see that\n> such knowledge could be handy for people in situations which would be\n> helped by this. That said, if you do document it, this doesn't feel\n> like the correct place to do so; it should be in a \"Discussion\"\n> section or something. (Who would expect to find --quiet documentation\n> talking about optimizations? Likely, nobody.)\n\nYeah, I had the same thought. You'd probably choose --quiet because you\nwant it, you know, quiet.\n\nWhereas for the new config variable, you'd probably set it not because\nyou want it quiet all the time, but because you want to get some time\nsavings. So there it does make sense to me to explain.\n\nOther than that, this seems like an obvious and easy win. It does feel a\nlittle hacky (you're really losing something in the output, and ideally\nwe'd just be able to give that answer quickly), but this may be OK as a\nhack in the interim.\n\nThe sad thing is just that it's user-facing, so we have to respect it\nforever. I almost wonder if there should be a global core.optimizeMessages\nor something that tries to tradeoff less information for speed in all\ncommands, but makes no promises about which. Then a user with a big repo\nwho sets it once will get the benefit as more areas are identified (I\nthink \"status\" already has a similar case with ahead/behind)? And vice\nversa, as some messages get faster to produce, they can be dropped from\nthat option.\n\nI dunno. Maybe that is a stupid idea, and people really do want to\ncontrol it on a per-message basis.\n\n-Peff\n"},{"id":"360775","messageId":"20181017182337.GD28326@sigill.intra.peff.net","threadId":"49595","inReplyTo":"CAPig+cQ3ia78pLtnHSq8tM3B-XnFgWhwowJxwacYEEzXosJ16g@mail.gmail.com","subject":"Re: [PATCH v1 2/2] reset: add new reset.quietDefault config setting","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-17T18:23:38Z","receivedAt":"2018-10-17T18:23:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 17, 2018 at 02:19:59PM -0400, Eric Sunshine wrote:\n\n> On Wed, Oct 17, 2018 at 12:40 PM Ben Peart wrote:\n> > Add a reset.quietDefault config setting that sets the default value of the\n> > --quiet flag when running the reset command. This enables users to change\n> > the default behavior to take advantage of the performance advantages of\n> > avoiding the scan for unstaged changes after reset. Defaults to false.\n> \n> As with the previous patch, my knee-jerk reaction is that this really\n> feels wrong being tied to --quiet. It's particularly unintuitive.\n> \n> What I _could_ see, and what would feel more natural is if you add a\n> new option (say, --optimize) which is more general, incorporating\n> whatever optimizations become available in the future, not just this\n> one special-case. A side-effect of --optimize is that it implies\n> --quiet, and that is something which can and should be documented.\n\nHeh, I just wrote something very similar elsewhere in the thread. I'm\nstill not sure if it's a dumb idea, but at least we can be dumb\ntogether.\n\n-Peff\n"},{"id":"360810","messageId":"xmqqpnw7vs5b.fsf@gitster-ct.c.googlers.com","threadId":"49595","inReplyTo":"20181017182255.GC28326@sigill.intra.peff.net","subject":"Re: [PATCH v1 1/2] reset: don't compute unstaged changes after reset when --quiet","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-18T03:40:48Z","receivedAt":"2018-10-18T03:40:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Whereas for the new config variable, you'd probably set it not because\n> you want it quiet all the time, but because you want to get some time\n> savings. So there it does make sense to me to explain.\n>\n> Other than that, this seems like an obvious and easy win. It does feel a\n> little hacky (you're really losing something in the output, and ideally\n> we'd just be able to give that answer quickly), but this may be OK as a\n> hack in the interim.\n\nAfter \"git reset --quiet -- this/area/\" with this change, any\noperation you'd do next that needs to learn if working tree files\nare different from what is recorded in the index outside that area\nwill have to spend more cycles, because the refresh done by \"reset\"\nis now limited to the area. So if your final goal is \"make 'reset'\nas fast as possible\", this is an obvious and easy win. For other\ngoals, i.e. \"make the overall experience of using Git feel faster\",\nit is not so obvious to me, though.\n\nIf we somehow know that it is much less important in your setup that\nthe cached stat bits in the index is kept up to date (e.g. perhaps\nyou are more heavily relying on fsmonitor and are happy with it),\nthen I suspect that we could even skip the refreshing altogether and\ngain more performance, without sacrificing the \"overall experience\nof using Git\" at all, which would be even better.\n\n> The sad thing is just that it's user-facing, so we have to respect it\n> forever. I almost wonder if there should be a global core.optimizeMessages\n> or something that tries to tradeoff less information for speed in all\n> commands, but makes no promises about which. Then a user with a big repo\n> who sets it once will get the benefit as more areas are identified (I\n> think \"status\" already has a similar case with ahead/behind)? And vice\n> versa, as some messages get faster to produce, they can be dropped from\n> that option.\n>\n> I dunno. Maybe that is a stupid idea, and people really do want to\n> control it on a per-message basis.\n>\n> -Peff\n"},{"id":"360818","messageId":"20181018063628.GA23537@sigill.intra.peff.net","threadId":"49595","inReplyTo":"xmqqpnw7vs5b.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v1 1/2] reset: don't compute unstaged changes after reset when --quiet","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-18T06:36:29Z","receivedAt":"2018-10-18T06:36:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 18, 2018 at 12:40:48PM +0900, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Whereas for the new config variable, you'd probably set it not because\n> > you want it quiet all the time, but because you want to get some time\n> > savings. So there it does make sense to me to explain.\n> >\n> > Other than that, this seems like an obvious and easy win. It does feel a\n> > little hacky (you're really losing something in the output, and ideally\n> > we'd just be able to give that answer quickly), but this may be OK as a\n> > hack in the interim.\n> \n> After \"git reset --quiet -- this/area/\" with this change, any\n> operation you'd do next that needs to learn if working tree files\n> are different from what is recorded in the index outside that area\n> will have to spend more cycles, because the refresh done by \"reset\"\n> is now limited to the area. So if your final goal is \"make 'reset'\n> as fast as possible\", this is an obvious and easy win. For other\n> goals, i.e. \"make the overall experience of using Git feel faster\",\n> it is not so obvious to me, though.\n> \n> If we somehow know that it is much less important in your setup that\n> the cached stat bits in the index is kept up to date (e.g. perhaps\n> you are more heavily relying on fsmonitor and are happy with it),\n> then I suspect that we could even skip the refreshing altogether and\n> gain more performance, without sacrificing the \"overall experience\n> of using Git\" at all, which would be even better.\n\nYeah, I assumed that Ben was using fsmonitor. I agree if we can just use\nthat to make this output faster, that would be the ideal. This is the\n\"later the message would get faster to produce\" I hinted at in my\nearlier message.\n\nSo I think we are in agreement. It just isn't clear to me how much work\nit would take to get to the \"ideal\". If it's long enough, then this kind\nof hackery may be useful in the meantime.\n\n-Peff\n"},{"id":"360857","messageId":"5b4d46c2-ac0b-8a44-5e99-b0926ea764d3@gmail.com","threadId":"49595","inReplyTo":"20181018063628.GA23537@sigill.intra.peff.net","subject":"Re: [PATCH v1 1/2] reset: don't compute unstaged changes after reset when --quiet","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-18T18:15:24Z","receivedAt":"2018-10-18T18:15:28Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/18/2018 2:36 AM, Jeff King wrote:\n> On Thu, Oct 18, 2018 at 12:40:48PM +0900, Junio C Hamano wrote:\n> \n>> Jeff King writes:\n>>\n>>> Whereas for the new config variable, you'd probably set it not because\n>>> you want it quiet all the time, but because you want to get some time\n>>> savings. So there it does make sense to me to explain.\n>>>\n>>> Other than that, this seems like an obvious and easy win. It does feel a\n>>> little hacky (you're really losing something in the output, and ideally\n>>> we'd just be able to give that answer quickly), but this may be OK as a\n>>> hack in the interim.\n>>\n>> After \"git reset --quiet -- this/area/\" with this change, any\n>> operation you'd do next that needs to learn if working tree files\n>> are different from what is recorded in the index outside that area\n>> will have to spend more cycles, because the refresh done by \"reset\"\n>> is now limited to the area. So if your final goal is \"make 'reset'\n>> as fast as possible\", this is an obvious and easy win. For other\n>> goals, i.e. \"make the overall experience of using Git feel faster\",\n>> it is not so obvious to me, though.\n\nThe final goal is to make git faster (especially on larger repos) and \nthis proposal accomplishes that. Let's look at why that is.\n\nBy scoping down (or eliminating) what refresh_index() has to lstat() at \nthe end of the reset command, clearly the reset command is faster. Yes, \nthe index isn't as \"fresh\" because not everything was updated but that \ndoesn't typically impact the performance of subsequent commands.\n\nOn the next command, git still has to lstat() every file because it \nisn't sure what changes could have happened in the file system. As a \nresult, the overall impact is that we have had to lstat() every file one \nfewer times between the two commands. A net win overall.\n\nIn addition, the preload_index() code that does the lstat() command is \nhighly optimized across multiple threads (and on Windows takes advantage \nof the fscache). This means that it can lstat() every file _much_ \nfaster than the single threaded loop in refresh_index(). This also \nmakes the overall performance of the pair of git commands faster as we \ngot rid of the slow lstat() loop and kept the fast one.\n\nHere are some numbers to demonstrate that. These are hot cache numbers \nas they are easier to generate. Cold cache numbers make the net perf \nwin significantly better as the cost for the reset jumps from 2.43 \nseconds to 7.16 seconds.\n\n0.32 git add asdf\n0.31 git -c reset.quiet=true reset asdf\n1.34 git status\n1.97 Total\n\n\n0.32 git add asdf\n2.43 git -c reset.quiet=false reset asdf\n1.32 git status\n4.07 Total\n\nNote the status command after the reset doesn't really change as it \nstill must lstat() every file (the 0.02 difference is well within the \nvariability of run to run differences).\n\nFWIW, none of these numbers are using fsmonitor.\n\n\n\nThere was additional discussion about whether this should be tied to the \n\"--quiet\" option and how it should be documented.\n\nOne option would be to change the default behavior of reset so that it \ndoesn't do the refresh_index() call at all. This speeds up reset by \ndefault so there are no user discoverability issues but changes the \ndefault behavior which is an issue.\n\nAnother option that was suggested was to add a separate flag that could \nbe passed to reset so that the \"quiet\" and \"fast\" options don't get \nconflated. I don't care for that option because the two options (at \nthis point and for the foreseeable future) would be identical in \nbehavior from the end users perspective.\n\nIt was also suggested that there be a single \"fast and quiet\" option for \nall of git instead of separate options for each command. I worry about \nthat because now we're forcing users to choose between the \"fast and \nquiet\" version of git and the \"slow and noisy\" version. How do we help \nthem decide which they want? That seems difficult to explain so that \nthey can make a rational choice and also hard to discover. I also have \nto wonder who would say \"give me the slow and noisy version please.\" :)\n\nI'd prefer we systematically move towards a model where the default \nvalues that are chosen for various settings throughout the code are all \nconfigurable via settings. All defaults by necessity make certain \nassumptions about user preference, data shape, machine performance, etc \nand if those assumptions don't match the user's environment then the \nhard coded defaults aren't appropriate. We do our best but its going to \nbe hit or miss.\n\nA consistent way to be able to change those defaults would be very \nuseful in those circumstances. To be clear, I'm not proposing we do a \nwholesale update of our preferences model at this point in time - that \nseems like a significant undertaking and I don't want to tie this \nspecific optimization to a potential change in how default settings work.\n\n\nTo move this forward, here is what I propose:\n\n1) If the '--quiet' flag is passed, we silently take advantage of the \nfact we can avoid having to do an \"extra\" lstat() of every file and \nscope the refresh_index() call to those paths that we know have changed.\n\n2) I can remove the note in the documentation of --quiet which I only \nadded to facilitate discoverability.\n\n3) I can also edit the documentation for reset.quietDefault (maybe I \nshould rename that to \"reset.quiet\"?) so that it does not discuss the \npotential performance impact.\n\n4) To improve the discoverability of the enhanced performance, I could \nadd logic similar to what exists for \"status --uno\" and if \nrefresh_index() takes > x seconds, prompt the user with something like:\n\n\"It took %.2f seconds to enumerate unstaged changes after reset. 'reset \n--quiet' may speed it up. Set the config setting reset.quiet to true to \nmake this the default.\"\n\n\n>>\n>> If we somehow know that it is much less important in your setup that\n>> the cached stat bits in the index is kept up to date (e.g. perhaps\n>> you are more heavily relying on fsmonitor and are happy with it),\n>> then I suspect that we could even skip the refreshing altogether and\n>> gain more performance, without sacrificing the \"overall experience\n>> of using Git\" at all, which would be even better.\n> \n> Yeah, I assumed that Ben was using fsmonitor. I agree if we can just use\n> that to make this output faster, that would be the ideal. This is the\n> \"later the message would get faster to produce\" I hinted at in my\n> earlier message.\n> \n> So I think we are in agreement. It just isn't clear to me how much work\n> it would take to get to the \"ideal\". If it's long enough, then this kind\n> of hackery may be useful in the meantime.\n> \n\nI actually started my effort to speed up reset by attempting to \nmulti-thread refresh_index(). You can see a work in progress at:\n\nhttps://github.com/benpeart/git/pull/new/refresh-index-multithread-gvfs\n\nThe patch doesn't always work as it is still not thread safe. When it \nworks, it's great but I ran into to many difficulties trying to debug \nthe remaining threading issues (even adding print statements would \nchange the timing and the repro would disappear). It will take a lot of \ncode review to discover and fix the remaining non-thread safe code paths.\n\nIn addition, the optimized code path that takes advantage of fsmonitor, \nuses multiple threads, fscache, etc _already exists_ in preload_index(). \n Trying to recreate all those optimizations in refresh_index() is (as I \ndiscovered) a daunting task.\n\nThis patch was tiny/trivial in comparison and provided all the \nperformance benefits so seems like a much better option at this point in \ntime. For now, I suggest we just use that existing path as it provides \nthe benefits without the significant additional work and complexity.\n\n> -Peff\n> \n"},{"id":"360858","messageId":"CACsJy8CvvZcQdxnZbu-FZmVm7wtMDLocjiMVURhvJ=NtuYgi9w@mail.gmail.com","threadId":"49595","inReplyTo":"5b4d46c2-ac0b-8a44-5e99-b0926ea764d3@gmail.com","subject":"Re: [PATCH v1 1/2] reset: don't compute unstaged changes after reset when --quiet","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-18T18:26:45Z","receivedAt":"2018-10-18T18:27:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Oct 18, 2018 at 8:18 PM Ben Peart wrote:\n> I actually started my effort to speed up reset by attempting to\n> multi-thread refresh_index(). You can see a work in progress at:\n>\n> https://github.com/benpeart/git/pull/new/refresh-index-multithread-gvfs\n>\n> The patch doesn't always work as it is still not thread safe. When it\n> works, it's great but I ran into to many difficulties trying to debug\n> the remaining threading issues (even adding print statements would\n> change the timing and the repro would disappear). It will take a lot of\n> code review to discover and fix the remaining non-thread safe code paths.\n>\n> In addition, the optimized code path that takes advantage of fsmonitor,\n> uses multiple threads, fscache, etc _already exists_ in preload_index().\n> Trying to recreate all those optimizations in refresh_index() is (as I\n> discovered) a daunting task.\n\nWhy not make refresh_index() run preload_index() first (or the\nparallel lstat part to be precise), and only do the heavy\ncontent-based refresh in single thread mode?\n-- \nDuy\n"},{"id":"360866","messageId":"ce5077bd-fefc-d415-7237-a2f9a26e3294@gmail.com","threadId":"49595","inReplyTo":"CACsJy8CvvZcQdxnZbu-FZmVm7wtMDLocjiMVURhvJ=NtuYgi9w@mail.gmail.com","subject":"Re: [PATCH v1 1/2] reset: don't compute unstaged changes after reset when --quiet","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-18T19:03:32Z","receivedAt":"2018-10-18T19:03:37Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/18/2018 2:26 PM, Duy Nguyen wrote:\n> On Thu, Oct 18, 2018 at 8:18 PM Ben Peart wrote:\n>> I actually started my effort to speed up reset by attempting to\n>> multi-thread refresh_index(). You can see a work in progress at:\n>>\n>> https://github.com/benpeart/git/pull/new/refresh-index-multithread-gvfs\n>>\n>> The patch doesn't always work as it is still not thread safe. When it\n>> works, it's great but I ran into to many difficulties trying to debug\n>> the remaining threading issues (even adding print statements would\n>> change the timing and the repro would disappear). It will take a lot of\n>> code review to discover and fix the remaining non-thread safe code paths.\n>>\n>> In addition, the optimized code path that takes advantage of fsmonitor,\n>> uses multiple threads, fscache, etc _already exists_ in preload_index().\n>> Trying to recreate all those optimizations in refresh_index() is (as I\n>> discovered) a daunting task.\n> \n> Why not make refresh_index() run preload_index() first (or the\n> parallel lstat part to be precise), and only do the heavy\n> content-based refresh in single thread mode?\n> \n\nHead smack! Why didn't I think of that?\n\nThat is a terrific suggestion. Calling preload_index() right before the \nbig for loop in refresh_index() is a trivial and effective way to do the \nbulk of the updating with the optimized code. After doing that, most of \nthe cache entries can bail out quickly down in refresh_cache_ent() when \nit tests ce_uptodate(ce).\n\nHere are the numbers using that optimization (hot cache, averaged across \n3 runs):\n\n0.32 git add asdf\n1.67 git reset asdf\n1.68 git status\n3.67 Total\n\nvs without it:\n\n0.32 git add asdf\n2.48 git reset asdf\n1.50 git status\n4.30 Total\n\nFor a savings in the reset command of 32% and 15% overall.\n\nClearly doing the refresh_index() faster is not as much savings as not \ndoing it at all. Given how simple this patch is, I think it makes sense \nto do both so that we have optimized each path to is fullest.\n"},{"id":"360887","messageId":"xmqqo9bq4vv7.fsf@gitster-ct.c.googlers.com","threadId":"49595","inReplyTo":"5b4d46c2-ac0b-8a44-5e99-b0926ea764d3@gmail.com","subject":"Re: [PATCH v1 1/2] reset: don't compute unstaged changes after reset when --quiet","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-19T00:34:52Z","receivedAt":"2018-10-19T00:34:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Peart writes:\n\n> Note the status command after the reset doesn't really change as it\n> still must lstat() every file (the 0.02 difference is well within the\n> variability of run to run differences).\n\nOf course, it would not make an iota of difference, whether reset\nrefreshes the cached stat index fully, to the cost of later lstat().\nWhat the refreshing saves is having to scan the contents to find that\nthe file is unchanged at runtime.\n\nIf your lstat() is not significantly faster than opening and\nscanning the file, the optimization based on the cached-stat\ninformation becomes moot. In a working tree full of unmodified\nfiles, stale cached-stat info in the index will cause us to compare\nthe contents and waste a lot of time, and that is what refreshing\navoids. If the \"status\" in your test sequence do not have to do\nthat (e.g. the cached-stat information is already up-to-date and\nthere is no point running refresh in reset), then I'd expect no\ndifference between these two tests.\n\n> To move this forward, here is what I propose:\n>\n> 1) If the '--quiet' flag is passed, we silently take advantage of the\n> fact we can avoid having to do an \"extra\" lstat() of every file and\n> scope the refresh_index() call to those paths that we know have\n> changed.\n\nThat's pretty much what the patch under discussion does.\n\n> 2) I can remove the note in the documentation of --quiet which I only\n> added to facilitate discoverability.\n\nQuite honestly, I am not sure if this (meaning #1 above) alone need\nto be even discoverable. Those who want --quiet output would use\nit, those who want to be told which paths are modified would not,\nand those who want to quickly be told which paths are modified would\nnot be helped by the limited refresh anyway, so \"with --quiet you\ncan make it go faster\" would not help anybody.\n\n> 3) I can also edit the documentation for reset.quietDefault (maybe I\n> should rename that to \"reset.quiet\"?) so that it does not discuss the\n> potential performance impact.\n\nI think reset.quiet (or reset.verbosity) is a good thing to have\nregardless.\n\n"},{"id":"360939","messageId":"20181019161228.17196-1-peartben@gmail.com","threadId":"49595","inReplyTo":"20181017164021.15204-1-peartben@gmail.com","subject":"[PATCH v2 0/3] speed up git reset","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-19T16:12:25Z","receivedAt":"2018-10-19T16:12:41Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"From: Ben Peart \n\nThis itteration avoids the refresh_index() call completely if 'quiet'.\nThe advantage of this is that \"git refresh\" without any pathspec is also\nsignificantly sped up.\n\nAlso added a notification if finding unstaged changes after reset takes\nlonger than 2 seconds to make users aware of the option to speed it up if\nthey don't need the unstaged changes after reset to be output.\n\nIt also renames the new config setting reset.quietDefault to reset.quiet.\n\nBase Ref: \nWeb-Diff: https://github.com/benpeart/git/commit/50d3415ef1\nCheckout: git fetch https://github.com/benpeart/git reset-refresh-index-v2 && git checkout 50d3415ef1\n\n\n### Interdiff (v1..v2):\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex a5cf4c019b..a2d1b8b116 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2728,11 +2728,8 @@ rerere.enabled::\n \t`$GIT_DIR`, e.g. if \"rerere\" was previously used in the\n \trepository.\n \n-reset.quietDefault::\n-\tSets the default value of the \"quiet\" option for the reset command.\n-\tChoosing \"quiet\" can optimize the performance of the reset command\n-\tby avoiding the scan of all files in the repo looking for additional\n-\tunstaged changes. Defaults to false.\n+reset.quiet::\n+\tWhen set to true, 'git reset' will default to the '--quiet' option.\n \n include::sendemail-config.txt[]\n \ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex 8610309b55..1d697d9962 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -95,9 +95,7 @@ OPTIONS\n \n -q::\n --quiet::\n-\tBe quiet, only report errors. Can optimize the performance of reset\n-\tby avoiding scaning all files in the repo looking for additional\n-\tunstaged changes.\n+\tBe quiet, only report errors.\n \n \n EXAMPLES\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 7d151d48a0..d95a27d52e 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -25,6 +25,8 @@\n #include \"submodule.h\"\n #include \"submodule-config.h\"\n \n+#define REFRESH_INDEX_DELAY_WARNING_IN_MS (2 * 1000)\n+\n static const char * const git_reset_usage[] = {\n \tN_(\"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []\"),\n \tN_(\"git reset [-q] [] [--] ...\"),\n@@ -306,7 +308,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t};\n \n \tgit_config(git_reset_config, NULL);\n-\tgit_config_get_bool(\"reset.quietDefault\", &quiet);\n+\tgit_config_get_bool(\"reset.quiet\", &quiet);\n \n \targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n@@ -376,9 +378,19 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tint flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;\n \t\t\tif (read_from_tree(&pathspec, &oid, intent_to_add))\n \t\t\t\treturn 1;\n-\t\t\tif (get_git_work_tree())\n-\t\t\t\trefresh_index(&the_index, flags, quiet ? &pathspec : NULL, NULL,\n+\t\t\tif (!quiet && get_git_work_tree()) {\n+\t\t\t\tuint64_t t_begin, t_delta_in_ms;\n+\n+\t\t\t\tt_begin = getnanotime();\n+\t\t\t\trefresh_index(&the_index, flags, NULL, NULL,\n \t\t\t\t\t _(\"Unstaged changes after reset:\"));\n+\t\t\t\tt_delta_in_ms = (getnanotime() - t_begin) / 1000000;\n+\t\t\t\tif (t_delta_in_ms > REFRESH_INDEX_DELAY_WARNING_IN_MS) {\n+\t\t\t\t\tprintf(_(\"\\nIt took %.2f seconds to enumerate unstaged changes after reset. You can\\n\"\n+\t\t\t\t\t\t\"use '--quiet' to avoid this. Set the config setting reset.quiet to true\\n\"\n+\t\t\t\t\t\t\"to make this the default.\"), t_delta_in_ms / 1000.0);\n+\t\t\t\t}\n+\t\t\t}\n \t\t} else {\n \t\t\tint err = reset_index(&oid, reset_type, quiet);\n \t\t\tif (reset_type == KEEP && !err)\n\n\n### Patches\n\nBen Peart (3):\n reset: don't compute unstaged changes after reset when --quiet\n reset: add new reset.quiet config setting\n reset: warn when refresh_index() takes more than 2 seconds\n\n Documentation/config.txt | 3 +++\n builtin/reset.c | 15 ++++++++++++++-\n 2 files changed, 17 insertions(+), 1 deletion(-)\n\n\nbase-commit: ca63497355222acefcca02b9cbb540a4768f3286\n-- \n2.18.0.windows.1\n\n\n"},{"id":"360940","messageId":"20181019161228.17196-2-peartben@gmail.com","threadId":"49595","inReplyTo":"20181019161228.17196-1-peartben@gmail.com","subject":"[PATCH v2 1/3] reset: don't compute unstaged changes after reset when --quiet","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-19T16:12:26Z","receivedAt":"2018-10-19T16:12:42Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"From: Ben Peart \n\nWhen git reset is run with the --quiet flag, don't bother finding any\nadditional unstaged changes as they won't be output anyway. This speeds up\nthe git reset command by avoiding having to lstat() every file looking for\nchanges that aren't going to be reported anyway.\n\nThe savings can be significant. In a repo with 200K files \"git reset\"\ndrops from 7.16 seconds to 0.32 seconds for a savings of 96%.\n\nSigned-off-by: Ben Peart \n---\n builtin/reset.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 11cd0dcb8c..04f0d9b4f5 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -375,7 +375,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tint flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;\n \t\t\tif (read_from_tree(&pathspec, &oid, intent_to_add))\n \t\t\t\treturn 1;\n-\t\t\tif (get_git_work_tree())\n+\t\t\tif (!quiet && get_git_work_tree())\n \t\t\t\trefresh_index(&the_index, flags, NULL, NULL,\n \t\t\t\t\t _(\"Unstaged changes after reset:\"));\n \t\t} else {\n-- \n2.18.0.windows.1\n\n"},{"id":"360941","messageId":"20181019161228.17196-3-peartben@gmail.com","threadId":"49595","inReplyTo":"20181019161228.17196-1-peartben@gmail.com","subject":"[PATCH v2 2/3] reset: add new reset.quiet config setting","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-19T16:12:27Z","receivedAt":"2018-10-19T16:12:44Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"From: Ben Peart \n\nAdd a reset.quiet config setting that sets the default value of the --quiet\nflag when running the reset command. This enables users to change the\ndefault behavior to take advantage of the performance advantages of\navoiding the scan for unstaged changes after reset. Defaults to false.\n\nSigned-off-by: Ben Peart \n---\n Documentation/config.txt | 3 +++\n builtin/reset.c | 1 +\n 2 files changed, 4 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex f6f4c21a54..a2d1b8b116 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2728,6 +2728,9 @@ rerere.enabled::\n \t`$GIT_DIR`, e.g. if \"rerere\" was previously used in the\n \trepository.\n \n+reset.quiet::\n+\tWhen set to true, 'git reset' will default to the '--quiet' option.\n+\n include::sendemail-config.txt[]\n \n sequence.editor::\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 04f0d9b4f5..3b43aee544 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -306,6 +306,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t};\n \n \tgit_config(git_reset_config, NULL);\n+\tgit_config_get_bool(\"reset.quiet\", &quiet);\n \n \targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n-- \n2.18.0.windows.1\n\n"},{"id":"360942","messageId":"20181019161228.17196-4-peartben@gmail.com","threadId":"49595","inReplyTo":"20181019161228.17196-1-peartben@gmail.com","subject":"[PATCH v2 3/3] reset: warn when refresh_index() takes more than 2 seconds","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-19T16:12:28Z","receivedAt":"2018-10-19T16:12:45Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"From: Ben Peart \n\nrefresh_index() is done after a reset command as an optimization. Because\nit can be an expensive call, warn the user if it takes more than 2 seconds\nand tell them how to avoid it using the --quiet command line option or\nreset.quiet config setting.\n\nSigned-off-by: Ben Peart \n---\n builtin/reset.c | 14 +++++++++++++-\n 1 file changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 3b43aee544..d95a27d52e 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -25,6 +25,8 @@\n #include \"submodule.h\"\n #include \"submodule-config.h\"\n \n+#define REFRESH_INDEX_DELAY_WARNING_IN_MS (2 * 1000)\n+\n static const char * const git_reset_usage[] = {\n \tN_(\"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []\"),\n \tN_(\"git reset [-q] [] [--] ...\"),\n@@ -376,9 +378,19 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tint flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;\n \t\t\tif (read_from_tree(&pathspec, &oid, intent_to_add))\n \t\t\t\treturn 1;\n-\t\t\tif (!quiet && get_git_work_tree())\n+\t\t\tif (!quiet && get_git_work_tree()) {\n+\t\t\t\tuint64_t t_begin, t_delta_in_ms;\n+\n+\t\t\t\tt_begin = getnanotime();\n \t\t\t\trefresh_index(&the_index, flags, NULL, NULL,\n \t\t\t\t\t _(\"Unstaged changes after reset:\"));\n+\t\t\t\tt_delta_in_ms = (getnanotime() - t_begin) / 1000000;\n+\t\t\t\tif (t_delta_in_ms > REFRESH_INDEX_DELAY_WARNING_IN_MS) {\n+\t\t\t\t\tprintf(_(\"\\nIt took %.2f seconds to enumerate unstaged changes after reset. You can\\n\"\n+\t\t\t\t\t\t\"use '--quiet' to avoid this. Set the config setting reset.quiet to true\\n\"\n+\t\t\t\t\t\t\"to make this the default.\"), t_delta_in_ms / 1000.0);\n+\t\t\t\t}\n+\t\t\t}\n \t\t} else {\n \t\t\tint err = reset_index(&oid, reset_type, quiet);\n \t\t\tif (reset_type == KEEP && !err)\n-- \n2.18.0.windows.1\n\n"},{"id":"360946","messageId":"CAPig+cSL9=mmvdq9J9VXF67=010E1eZBjrYYaYQDN1z1OEf0CA@mail.gmail.com","threadId":"49595","inReplyTo":"20181019161228.17196-3-peartben@gmail.com","subject":"Re: [PATCH v2 2/3] reset: add new reset.quiet config setting","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-10-19T16:36:44Z","receivedAt":"2018-10-19T16:36:57Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Oct 19, 2018 at 12:12 PM Ben Peart wrote:\n> Add a reset.quiet config setting that sets the default value of the --quiet\n> flag when running the reset command. This enables users to change the\n> default behavior to take advantage of the performance advantages of\n> avoiding the scan for unstaged changes after reset. Defaults to false.\n>\n> Signed-off-by: Ben Peart \n> ---\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> @@ -2728,6 +2728,9 @@ rerere.enabled::\n> +reset.quiet::\n> + When set to true, 'git reset' will default to the '--quiet' option.\n\nHow does the user reverse this for a particular git-reset invocation?\nThere is no --no-quiet or --verbose option.\n\nPerhaps you want to use OPT__VERBOSITY() instead of OPT__QUIET() in\nbuiltin/reset.c and document that --verbose overrides --quiet and\nreset.quiet (or something like that).\n"},{"id":"360948","messageId":"20181019164631.GB24740@sigill.intra.peff.net","threadId":"49595","inReplyTo":"CAPig+cSL9=mmvdq9J9VXF67=010E1eZBjrYYaYQDN1z1OEf0CA@mail.gmail.com","subject":"Re: [PATCH v2 2/3] reset: add new reset.quiet config setting","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-19T16:46:32Z","receivedAt":"2018-10-19T16:46:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 19, 2018 at 12:36:44PM -0400, Eric Sunshine wrote:\n\n> On Fri, Oct 19, 2018 at 12:12 PM Ben Peart wrote:\n> > Add a reset.quiet config setting that sets the default value of the --quiet\n> > flag when running the reset command. This enables users to change the\n> > default behavior to take advantage of the performance advantages of\n> > avoiding the scan for unstaged changes after reset. Defaults to false.\n> >\n> > Signed-off-by: Ben Peart \n> > ---\n> > diff --git a/Documentation/config.txt b/Documentation/config.txt\n> > @@ -2728,6 +2728,9 @@ rerere.enabled::\n> > +reset.quiet::\n> > + When set to true, 'git reset' will default to the '--quiet' option.\n> \n> How does the user reverse this for a particular git-reset invocation?\n> There is no --no-quiet or --verbose option.\n> \n> Perhaps you want to use OPT__VERBOSITY() instead of OPT__QUIET() in\n> builtin/reset.c and document that --verbose overrides --quiet and\n> reset.quiet (or something like that).\n\nI think OPT__QUIET() provides --no-quiet, since it's really an\nOPT_COUNTUP() under the hood. Saying \"--no-quiet\" should reset it back\nto 0.\n\n-Peff\n"},{"id":"360955","messageId":"CAPig+cR7=OpNsuZu+ppdyDvt5HAHMdDj4cBVg2U34B_j2zZ03g@mail.gmail.com","threadId":"49595","inReplyTo":"20181019164631.GB24740@sigill.intra.peff.net","subject":"Re: [PATCH v2 2/3] reset: add new reset.quiet config setting","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-10-19T17:10:34Z","receivedAt":"2018-10-19T17:10:47Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Oct 19, 2018 at 12:46 PM Jeff King wrote:\n> On Fri, Oct 19, 2018 at 12:36:44PM -0400, Eric Sunshine wrote:\n> > How does the user reverse this for a particular git-reset invocation?\n> > There is no --no-quiet or --verbose option.\n> >\n> > Perhaps you want to use OPT__VERBOSITY() instead of OPT__QUIET() in\n> > builtin/reset.c and document that --verbose overrides --quiet and\n> > reset.quiet (or something like that).\n>\n> I think OPT__QUIET() provides --no-quiet, since it's really an\n> OPT_COUNTUP() under the hood. Saying \"--no-quiet\" should reset it back\n> to 0.\n\nOkay. In any case, --no-quiet probably ought to be mentioned alongside\nthe \"reset.quiet\" option (and perhaps in git-reset.txt to as a way to\nreverse \"reset.quiet\").\n"},{"id":"360956","messageId":"5594e9d0-cfe2-f29f-a788-bebbd8d2b151@gmail.com","threadId":"49595","inReplyTo":"20181019164631.GB24740@sigill.intra.peff.net","subject":"Re: [PATCH v2 2/3] reset: add new reset.quiet config setting","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-19T17:11:24Z","receivedAt":"2018-10-19T17:11:28Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/19/2018 12:46 PM, Jeff King wrote:\n> On Fri, Oct 19, 2018 at 12:36:44PM -0400, Eric Sunshine wrote:\n> \n>> On Fri, Oct 19, 2018 at 12:12 PM Ben Peart wrote:\n>>> Add a reset.quiet config setting that sets the default value of the --quiet\n>>> flag when running the reset command. This enables users to change the\n>>> default behavior to take advantage of the performance advantages of\n>>> avoiding the scan for unstaged changes after reset. Defaults to false.\n>>>\n>>> Signed-off-by: Ben Peart \n>>> ---\n>>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>>> @@ -2728,6 +2728,9 @@ rerere.enabled::\n>>> +reset.quiet::\n>>> + When set to true, 'git reset' will default to the '--quiet' option.\n>>\n>> How does the user reverse this for a particular git-reset invocation?\n>> There is no --no-quiet or --verbose option.\n>>\n>> Perhaps you want to use OPT__VERBOSITY() instead of OPT__QUIET() in\n>> builtin/reset.c and document that --verbose overrides --quiet and\n>> reset.quiet (or something like that).\n> \n> I think OPT__QUIET() provides --no-quiet, since it's really an\n> OPT_COUNTUP() under the hood. Saying \"--no-quiet\" should reset it back\n> to 0.\n> \n\nThanks Peff. That is correct as confirmed by:\n\n\nC:\\Repos\\VSO\\src>git reset --no-quiet\nUnstaged changes after reset:\nM init.ps1\n\nIt took 6.74 seconds to enumerate unstaged changes after reset. You can\nuse '--quiet' to avoid this. Set the config setting reset.quiet to true\nto make this the default.\n\n\n> -Peff\n> \n"},{"id":"360957","messageId":"20181019171130.GA20834@sigill.intra.peff.net","threadId":"49595","inReplyTo":"CAPig+cR7=OpNsuZu+ppdyDvt5HAHMdDj4cBVg2U34B_j2zZ03g@mail.gmail.com","subject":"Re: [PATCH v2 2/3] reset: add new reset.quiet config setting","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-19T17:11:31Z","receivedAt":"2018-10-19T17:11:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 19, 2018 at 01:10:34PM -0400, Eric Sunshine wrote:\n\n> On Fri, Oct 19, 2018 at 12:46 PM Jeff King wrote:\n> > On Fri, Oct 19, 2018 at 12:36:44PM -0400, Eric Sunshine wrote:\n> > > How does the user reverse this for a particular git-reset invocation?\n> > > There is no --no-quiet or --verbose option.\n> > >\n> > > Perhaps you want to use OPT__VERBOSITY() instead of OPT__QUIET() in\n> > > builtin/reset.c and document that --verbose overrides --quiet and\n> > > reset.quiet (or something like that).\n> >\n> > I think OPT__QUIET() provides --no-quiet, since it's really an\n> > OPT_COUNTUP() under the hood. Saying \"--no-quiet\" should reset it back\n> > to 0.\n> \n> Okay. In any case, --no-quiet probably ought to be mentioned alongside\n> the \"reset.quiet\" option (and perhaps in git-reset.txt to as a way to\n> reverse \"reset.quiet\").\n\nYes, I'd agree with that.\n\n-Peff\n"},{"id":"360959","messageId":"38b9f813-0463-3d15-ad9d-86f64c140043@gmail.com","threadId":"49595","inReplyTo":"20181019171130.GA20834@sigill.intra.peff.net","subject":"Re: [PATCH v2 2/3] reset: add new reset.quiet config setting","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-19T17:23:06Z","receivedAt":"2018-10-19T17:23:11Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/19/2018 1:11 PM, Jeff King wrote:\n> On Fri, Oct 19, 2018 at 01:10:34PM -0400, Eric Sunshine wrote:\n> \n>> On Fri, Oct 19, 2018 at 12:46 PM Jeff King wrote:\n>>> On Fri, Oct 19, 2018 at 12:36:44PM -0400, Eric Sunshine wrote:\n>>>> How does the user reverse this for a particular git-reset invocation?\n>>>> There is no --no-quiet or --verbose option.\n>>>>\n>>>> Perhaps you want to use OPT__VERBOSITY() instead of OPT__QUIET() in\n>>>> builtin/reset.c and document that --verbose overrides --quiet and\n>>>> reset.quiet (or something like that).\n>>>\n>>> I think OPT__QUIET() provides --no-quiet, since it's really an\n>>> OPT_COUNTUP() under the hood. Saying \"--no-quiet\" should reset it back\n>>> to 0.\n>>\n>> Okay. In any case, --no-quiet probably ought to be mentioned alongside\n>> the \"reset.quiet\" option (and perhaps in git-reset.txt to as a way to\n>> reverse \"reset.quiet\").\n> \n> Yes, I'd agree with that.\n> \n> -Peff\n> \n\nMakes sense. I'll update the docs to say:\n\n-q::\n--quiet::\n--no-quiet::\n\tBe quiet, only report errors.\n+\nWith --no-quiet report errors and unstaged changes after reset.\n"},{"id":"360968","messageId":"20181019190856.GC24418@sigill.intra.peff.net","threadId":"49595","inReplyTo":"38b9f813-0463-3d15-ad9d-86f64c140043@gmail.com","subject":"Re: [PATCH v2 2/3] reset: add new reset.quiet config setting","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-19T19:08:56Z","receivedAt":"2018-10-19T19:08:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 19, 2018 at 01:23:06PM -0400, Ben Peart wrote:\n\n> > > Okay. In any case, --no-quiet probably ought to be mentioned alongside\n> > > the \"reset.quiet\" option (and perhaps in git-reset.txt to as a way to\n> > > reverse \"reset.quiet\").\n> [...]\n> Makes sense. I'll update the docs to say:\n> \n> -q::\n> --quiet::\n> --no-quiet::\n> \tBe quiet, only report errors.\n> +\n> With --no-quiet report errors and unstaged changes after reset.\n\nI think we should be explicit that \"--no-quiet\" is already the default,\nwhich makes it easy to mention the config option. Something like:\n\n -q::\n --quiet::\n --no-quiet::\n\tBe quiet, only report errors. The default behavior respects the\n\t`reset.quiet` config option, or `--no-quiet` if that is not set.\n\nI don't know if we need to mention the \"unstaged changes\" thing. We may\ngrow other non-error messages (or may even have some now, I didn't\ncheck). But I'm OK including it, too.\n\n-Peff\n"},{"id":"361133","messageId":"xmqqa7n6wp05.fsf@gitster-ct.c.googlers.com","threadId":"49595","inReplyTo":"38b9f813-0463-3d15-ad9d-86f64c140043@gmail.com","subject":"Re: [PATCH v2 2/3] reset: add new reset.quiet config setting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-22T05:04:42Z","receivedAt":"2018-10-22T05:09:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Peart writes:\n\n> On 10/19/2018 1:11 PM, Jeff King wrote:\n>> On Fri, Oct 19, 2018 at 01:10:34PM -0400, Eric Sunshine wrote:\n>>\n>>> On Fri, Oct 19, 2018 at 12:46 PM Jeff King wrote:\n>>>> On Fri, Oct 19, 2018 at 12:36:44PM -0400, Eric Sunshine wrote:\n>>>>> How does the user reverse this for a particular git-reset invocation?\n>>>>> There is no --no-quiet or --verbose option.\n>>>>>\n>>>>> Perhaps you want to use OPT__VERBOSITY() instead of OPT__QUIET() in\n>>>>> builtin/reset.c and document that --verbose overrides --quiet and\n>>>>> reset.quiet (or something like that).\n>>>>\n>>>> I think OPT__QUIET() provides --no-quiet, since it's really an\n>>>> OPT_COUNTUP() under the hood. Saying \"--no-quiet\" should reset it back\n>>>> to 0.\n>>>\n>>> Okay. In any case, --no-quiet probably ought to be mentioned alongside\n>>> the \"reset.quiet\" option (and perhaps in git-reset.txt to as a way to\n>>> reverse \"reset.quiet\").\n>>\n>> Yes, I'd agree with that.\n>>\n>> -Peff\n>>\n>\n> Makes sense. I'll update the docs to say:\n>\n> -q::\n> --quiet::\n> --no-quiet::\n> \tBe quiet, only report errors.\n> +\n> With --no-quiet report errors and unstaged changes after reset.\n\nSounds good. Thanks all.\n"},{"id":"361155","messageId":"20181022131828.21348-1-peartben@gmail.com","threadId":"49595","inReplyTo":"20181017164021.15204-1-peartben@gmail.com","subject":"[PATCH v3 0/3] speed up git reset","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-22T13:18:25Z","receivedAt":"2018-10-22T13:18:41Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"From: Ben Peart \n\nReworded the documentation for git-reset per review feedback.\n\nBase Ref: \nWeb-Diff: https://github.com/benpeart/git/commit/1228898917\nCheckout: git fetch https://github.com/benpeart/git reset-refresh-index-v3 && git checkout 1228898917\n\n\n### Interdiff (v2..v3):\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex 1d697d9962..51a427a34a 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -95,7 +95,9 @@ OPTIONS\n \n -q::\n --quiet::\n-\tBe quiet, only report errors.\n+--no-quiet::\n+\tBe quiet, only report errors. The default behavior respects the\n+\t`reset.quiet` config option, or `--no-quiet` if that is not set.\n \n \n EXAMPLES\n\n\n### Patches\n\nBen Peart (3):\n reset: don't compute unstaged changes after reset when --quiet\n reset: add new reset.quiet config setting\n reset: warn when refresh_index() takes more than 2 seconds\n\n Documentation/config.txt | 3 +++\n Documentation/git-reset.txt | 4 +++-\n builtin/reset.c | 15 ++++++++++++++-\n 3 files changed, 20 insertions(+), 2 deletions(-)\n\n\nbase-commit: ca63497355222acefcca02b9cbb540a4768f3286\n-- \n2.18.0.windows.1\n\n\n"},{"id":"361156","messageId":"20181022131828.21348-2-peartben@gmail.com","threadId":"49595","inReplyTo":"20181022131828.21348-1-peartben@gmail.com","subject":"[PATCH v3 1/3] reset: don't compute unstaged changes after reset when --quiet","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-22T13:18:26Z","receivedAt":"2018-10-22T13:18:44Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"From: Ben Peart \n\nWhen git reset is run with the --quiet flag, don't bother finding any\nadditional unstaged changes as they won't be output anyway. This speeds up\nthe git reset command by avoiding having to lstat() every file looking for\nchanges that aren't going to be reported anyway.\n\nThe savings can be significant. In a repo with 200K files \"git reset\"\ndrops from 7.16 seconds to 0.32 seconds for a savings of 96%.\n\nSigned-off-by: Ben Peart \n---\n builtin/reset.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 11cd0dcb8c..04f0d9b4f5 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -375,7 +375,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tint flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;\n \t\t\tif (read_from_tree(&pathspec, &oid, intent_to_add))\n \t\t\t\treturn 1;\n-\t\t\tif (get_git_work_tree())\n+\t\t\tif (!quiet && get_git_work_tree())\n \t\t\t\trefresh_index(&the_index, flags, NULL, NULL,\n \t\t\t\t\t _(\"Unstaged changes after reset:\"));\n \t\t} else {\n-- \n2.18.0.windows.1\n\n"},{"id":"361157","messageId":"20181022131828.21348-3-peartben@gmail.com","threadId":"49595","inReplyTo":"20181022131828.21348-1-peartben@gmail.com","subject":"[PATCH v3 2/3] reset: add new reset.quiet config setting","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-22T13:18:27Z","receivedAt":"2018-10-22T13:18:44Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"From: Ben Peart \n\nAdd a reset.quiet config setting that sets the default value of the --quiet\nflag when running the reset command. This enables users to change the\ndefault behavior to take advantage of the performance advantages of\navoiding the scan for unstaged changes after reset. Defaults to false.\n\nSigned-off-by: Ben Peart \n---\n Documentation/config.txt | 3 +++\n Documentation/git-reset.txt | 4 +++-\n builtin/reset.c | 1 +\n 3 files changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex f6f4c21a54..a2d1b8b116 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2728,6 +2728,9 @@ rerere.enabled::\n \t`$GIT_DIR`, e.g. if \"rerere\" was previously used in the\n \trepository.\n \n+reset.quiet::\n+\tWhen set to true, 'git reset' will default to the '--quiet' option.\n+\n include::sendemail-config.txt[]\n \n sequence.editor::\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex 1d697d9962..51a427a34a 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -95,7 +95,9 @@ OPTIONS\n \n -q::\n --quiet::\n-\tBe quiet, only report errors.\n+--no-quiet::\n+\tBe quiet, only report errors. The default behavior respects the\n+\t`reset.quiet` config option, or `--no-quiet` if that is not set.\n \n \n EXAMPLES\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 04f0d9b4f5..3b43aee544 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -306,6 +306,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t};\n \n \tgit_config(git_reset_config, NULL);\n+\tgit_config_get_bool(\"reset.quiet\", &quiet);\n \n \targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n-- \n2.18.0.windows.1\n\n"},{"id":"361158","messageId":"20181022131828.21348-4-peartben@gmail.com","threadId":"49595","inReplyTo":"20181022131828.21348-1-peartben@gmail.com","subject":"[PATCH v3 3/3] reset: warn when refresh_index() takes more than 2 seconds","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-22T13:18:28Z","receivedAt":"2018-10-22T13:18:46Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"From: Ben Peart \n\nrefresh_index() is done after a reset command as an optimization. Because\nit can be an expensive call, warn the user if it takes more than 2 seconds\nand tell them how to avoid it using the --quiet command line option or\nreset.quiet config setting.\n\nSigned-off-by: Ben Peart \n---\n builtin/reset.c | 14 +++++++++++++-\n 1 file changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 3b43aee544..d95a27d52e 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -25,6 +25,8 @@\n #include \"submodule.h\"\n #include \"submodule-config.h\"\n \n+#define REFRESH_INDEX_DELAY_WARNING_IN_MS (2 * 1000)\n+\n static const char * const git_reset_usage[] = {\n \tN_(\"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []\"),\n \tN_(\"git reset [-q] [] [--] ...\"),\n@@ -376,9 +378,19 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tint flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;\n \t\t\tif (read_from_tree(&pathspec, &oid, intent_to_add))\n \t\t\t\treturn 1;\n-\t\t\tif (!quiet && get_git_work_tree())\n+\t\t\tif (!quiet && get_git_work_tree()) {\n+\t\t\t\tuint64_t t_begin, t_delta_in_ms;\n+\n+\t\t\t\tt_begin = getnanotime();\n \t\t\t\trefresh_index(&the_index, flags, NULL, NULL,\n \t\t\t\t\t _(\"Unstaged changes after reset:\"));\n+\t\t\t\tt_delta_in_ms = (getnanotime() - t_begin) / 1000000;\n+\t\t\t\tif (t_delta_in_ms > REFRESH_INDEX_DELAY_WARNING_IN_MS) {\n+\t\t\t\t\tprintf(_(\"\\nIt took %.2f seconds to enumerate unstaged changes after reset. You can\\n\"\n+\t\t\t\t\t\t\"use '--quiet' to avoid this. Set the config setting reset.quiet to true\\n\"\n+\t\t\t\t\t\t\"to make this the default.\"), t_delta_in_ms / 1000.0);\n+\t\t\t\t}\n+\t\t\t}\n \t\t} else {\n \t\t\tint err = reset_index(&oid, reset_type, quiet);\n \t\t\tif (reset_type == KEEP && !err)\n-- \n2.18.0.windows.1\n\n"},{"id":"361165","messageId":"CACsJy8Dcf8OknyMaSZxOaib54jLSSt71XXjTZD3UjgnH6J7QFA@mail.gmail.com","threadId":"49595","inReplyTo":"20181022131828.21348-3-peartben@gmail.com","subject":"Re: [PATCH v3 2/3] reset: add new reset.quiet config setting","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-22T14:45:53Z","receivedAt":"2018-10-22T14:46:22Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Oct 22, 2018 at 3:38 PM Ben Peart wrote:\n>\n> From: Ben Peart \n>\n> Add a reset.quiet config setting that sets the default value of the --quiet\n> flag when running the reset command. This enables users to change the\n> default behavior to take advantage of the performance advantages of\n> avoiding the scan for unstaged changes after reset. Defaults to false.\n>\n> Signed-off-by: Ben Peart \n> ---\n> Documentation/config.txt | 3 +++\n> Documentation/git-reset.txt | 4 +++-\n> builtin/reset.c | 1 +\n> 3 files changed, 7 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index f6f4c21a54..a2d1b8b116 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -2728,6 +2728,9 @@ rerere.enabled::\n> `$GIT_DIR`, e.g. if \"rerere\" was previously used in the\n> repository.\n>\n> +reset.quiet::\n> + When set to true, 'git reset' will default to the '--quiet' option.\n> +\n\nWith 'nd/config-split' topic moving pretty much all config keys out of\nconfig.txt, you probably want to do the same for this series: add this\nin a new file called Documentation/reset-config.txt then include the\nfile here like the sendemail one below.\n\n> include::sendemail-config.txt[]\n>\n> sequence.editor::\n-- \nDuy\n"},{"id":"361177","messageId":"045b78ce-230e-86fe-6e5a-684bf9e93fbc@ramsayjones.plus.com","threadId":"49595","inReplyTo":"20181022131828.21348-3-peartben@gmail.com","subject":"Re: [PATCH v3 2/3] reset: add new reset.quiet config setting","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-10-22T19:13:32Z","receivedAt":"2018-10-22T19:13:39Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 22/10/2018 14:18, Ben Peart wrote:\n> From: Ben Peart \n> \n> Add a reset.quiet config setting that sets the default value of the --quiet\n> flag when running the reset command. This enables users to change the\n> default behavior to take advantage of the performance advantages of\n> avoiding the scan for unstaged changes after reset. Defaults to false.\n> \n> Signed-off-by: Ben Peart \n> ---\n> Documentation/config.txt | 3 +++\n> Documentation/git-reset.txt | 4 +++-\n> builtin/reset.c | 1 +\n> 3 files changed, 7 insertions(+), 1 deletion(-)\n> \n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index f6f4c21a54..a2d1b8b116 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -2728,6 +2728,9 @@ rerere.enabled::\n> \t`$GIT_DIR`, e.g. if \"rerere\" was previously used in the\n> \trepository.\n> \n> +reset.quiet::\n> +\tWhen set to true, 'git reset' will default to the '--quiet' option.\n> +\n> include::sendemail-config.txt[]\n> \n> sequence.editor::\n> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\n> index 1d697d9962..51a427a34a 100644\n> --- a/Documentation/git-reset.txt\n> +++ b/Documentation/git-reset.txt\n> @@ -95,7 +95,9 @@ OPTIONS\n> \n> -q::\n> --quiet::\n> -\tBe quiet, only report errors.\n> +--no-quiet::\n> +\tBe quiet, only report errors. The default behavior respects the\n> +\t`reset.quiet` config option, or `--no-quiet` if that is not set.\n\nSorry, I can't quite parse this; -q,--quiet and --no-quiet on the\ncommand line (should) trump whatever rest.quiet is set to in the\nconfiguration. Is that not the case?\n\nATB,\nRamsay Jones\n\n> \n> \n> EXAMPLES\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 04f0d9b4f5..3b43aee544 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -306,6 +306,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \t};\n> \n> \tgit_config(git_reset_config, NULL);\n> +\tgit_config_get_bool(\"reset.quiet\", &quiet);\n> \n> \targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n> \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n> \n"},{"id":"361179","messageId":"20181022200601.GC9917@sigill.intra.peff.net","threadId":"49595","inReplyTo":"045b78ce-230e-86fe-6e5a-684bf9e93fbc@ramsayjones.plus.com","subject":"Re: [PATCH v3 2/3] reset: add new reset.quiet config setting","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-22T20:06:01Z","receivedAt":"2018-10-22T20:06:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 22, 2018 at 08:13:32PM +0100, Ramsay Jones wrote:\n\n> > -q::\n> > --quiet::\n> > -\tBe quiet, only report errors.\n> > +--no-quiet::\n> > +\tBe quiet, only report errors. The default behavior respects the\n> > +\t`reset.quiet` config option, or `--no-quiet` if that is not set.\n> \n> Sorry, I can't quite parse this; -q,--quiet and --no-quiet on the\n> command line (should) trump whatever rest.quiet is set to in the\n> configuration. Is that not the case?\n\nThat is the case, and what was meant by \"the default behavior\" (i.e.,\nthe behavior when none of these is used). Maybe there's a more clear way\nof saying that.\n\n-Peff\n"},{"id":"361191","messageId":"nycvar.QRO.7.76.6.1810222244150.4546@tvgsbejvaqbjf.bet","threadId":"49595","inReplyTo":"20181022131828.21348-2-peartben@gmail.com","subject":"Re: [PATCH v3 1/3] reset: don't compute unstaged changes after reset when --quiet","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-22T20:44:48Z","receivedAt":"2018-10-22T20:44:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ben,\n\nOn Mon, 22 Oct 2018, Ben Peart wrote:\n\n> From: Ben Peart \n> \n> When git reset is run with the --quiet flag, don't bother finding any\n> additional unstaged changes as they won't be output anyway. This speeds up\n> the git reset command by avoiding having to lstat() every file looking for\n> changes that aren't going to be reported anyway.\n> \n> The savings can be significant. In a repo with 200K files \"git reset\"\n> drops from 7.16 seconds to 0.32 seconds for a savings of 96%.\n\nThat's very nice!\n\nThose numbers, just out of curiosity, are they on Windows? Or on Linux?\n\nCiao,\nDscho\n"},{"id":"361216","messageId":"MW2PR2101MB0970EF1065717A38CF581C64F4F40@MW2PR2101MB0970.namprd21.prod.outlook.com","threadId":"49595","inReplyTo":"nycvar.QRO.7.76.6.1810222244150.4546@tvgsbejvaqbjf.bet","subject":"RE: [PATCH v3 1/3] reset: don't compute unstaged changes after reset when --quiet","fromName":"Ben Peart","fromEmail":"ben.peart@microsoft.com","sentAt":"2018-10-22T22:07:45Z","receivedAt":"2018-10-22T22:07:49Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"> -----Original Message-----\n> From: Johannes Schindelin \n> Sent: Monday, October 22, 2018 4:45 PM\n> To: Ben Peart \n> Cc: git@vger.kernel.org; gitster@pobox.com; Ben Peart\n> ; peff@peff.net; sunshine@sunshineco.com\n> Subject: Re: [PATCH v3 1/3] reset: don't compute unstaged changes after\n> reset when --quiet\n> \n> Hi Ben,\n> \n> On Mon, 22 Oct 2018, Ben Peart wrote:\n> \n> > From: Ben Peart \n> >\n> > When git reset is run with the --quiet flag, don't bother finding any\n> > additional unstaged changes as they won't be output anyway. This speeds\n> up\n> > the git reset command by avoiding having to lstat() every file looking for\n> > changes that aren't going to be reported anyway.\n> >\n> > The savings can be significant. In a repo with 200K files \"git reset\"\n> > drops from 7.16 seconds to 0.32 seconds for a savings of 96%.\n> \n> That's very nice!\n> \n> Those numbers, just out of curiosity, are they on Windows? Or on Linux?\n> \n\nIt's safe to assume all my numbers are on Windows. :-)\n\n> Ciao,\n> Dscho\n\n\n"},{"id":"361234","messageId":"xmqqh8hdtsro.fsf@gitster-ct.c.googlers.com","threadId":"49595","inReplyTo":"20181022131828.21348-4-peartben@gmail.com","subject":"Re: [PATCH v3 3/3] reset: warn when refresh_index() takes more than 2 seconds","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-23T00:23:55Z","receivedAt":"2018-10-23T00:24:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Peart writes:\n\n> From: Ben Peart \n>\n> refresh_index() is done after a reset command as an optimization. Because\n> it can be an expensive call, warn the user if it takes more than 2 seconds\n> and tell them how to avoid it using the --quiet command line option or\n> reset.quiet config setting.\n\nI am moderately negative on this step. It will irritate users who\nknow about and still choose not to use the \"--quiet\" option, because\nthey want to gain performance in later real work and/or they want to\nknow what paths are now dirty. A working tree that needs long time\nto refresh will take long time to instead do \"cached stat info says\nit may be modified so let's run 'diff' for real---we may discover\nthat there wasn't any change after all\" when a \"git diff\" is run\nafter a \"reset --quiet\" that does not refresh; i.e. there would be\nvalid reasons to run \"reset\" without \"--quiet\".\n\nIt feels a bit irresponsible to throw an ad without informing\npros-and-cons and to pretend that we are advising on BCP. In\ngeneral, we do *not* advertise new features randomly like this.\n\nThanks. The previous two steps looks quite sensible.\n\n"},{"id":"361261","messageId":"nycvar.QRO.7.76.6.1810231052410.4546@tvgsbejvaqbjf.bet","threadId":"49595","inReplyTo":"MW2PR2101MB0970EF1065717A38CF581C64F4F40@MW2PR2101MB0970.namprd21.prod.outlook.com","subject":"RE: [PATCH v3 1/3] reset: don't compute unstaged changes after reset when --quiet","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-23T08:53:28Z","receivedAt":"2018-10-23T08:53:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ben,\n\nOn Mon, 22 Oct 2018, Ben Peart wrote:\n\n> > From: Johannes Schindelin \n> > \n> > On Mon, 22 Oct 2018, Ben Peart wrote:\n> > \n> > > When git reset is run with the --quiet flag, don't bother finding\n> > > any additional unstaged changes as they won't be output anyway.\n> > > This speeds up the git reset command by avoiding having to lstat()\n> > > every file looking for changes that aren't going to be reported\n> > > anyway.\n> > >\n> > > The savings can be significant. In a repo with 200K files \"git\n> > > reset\" drops from 7.16 seconds to 0.32 seconds for a savings of 96%.\n> > \n> > That's very nice!\n> > \n> > Those numbers, just out of curiosity, are they on Windows? Or on\n> > Linux?\n> > \n> \n> It's safe to assume all my numbers are on Windows. :-)\n\nExcellent. These speed-ups will really help our users.\n\nThanks!\nDscho\n"},{"id":"361262","messageId":"874lddc9fs.fsf@evledraar.gmail.com","threadId":"49595","inReplyTo":"20181017182337.GD28326@sigill.intra.peff.net","subject":"Re: [PATCH v1 2/2] reset: add new reset.quietDefault config setting","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-23T09:13:27Z","receivedAt":"2018-10-23T09:13:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 17 2018, Jeff King wrote:\n\n> On Wed, Oct 17, 2018 at 02:19:59PM -0400, Eric Sunshine wrote:\n>\n>> On Wed, Oct 17, 2018 at 12:40 PM Ben Peart wrote:\n>> > Add a reset.quietDefault config setting that sets the default value of the\n>> > --quiet flag when running the reset command. This enables users to change\n>> > the default behavior to take advantage of the performance advantages of\n>> > avoiding the scan for unstaged changes after reset. Defaults to false.\n>>\n>> As with the previous patch, my knee-jerk reaction is that this really\n>> feels wrong being tied to --quiet. It's particularly unintuitive.\n>>\n>> What I _could_ see, and what would feel more natural is if you add a\n>> new option (say, --optimize) which is more general, incorporating\n>> whatever optimizations become available in the future, not just this\n>> one special-case. A side-effect of --optimize is that it implies\n>> --quiet, and that is something which can and should be documented.\n>\n> Heh, I just wrote something very similar elsewhere in the thread. I'm\n> still not sure if it's a dumb idea, but at least we can be dumb\n> together.\n\nSame here. I'm in general if favor of having the ability to configure\nporcelain command-line options, but in this case it seems like it would\nbe more logical to head for something like:\n\n core.uiMessaging=[default,exhaustive,lossyButFaster,quiet]\n\nWhere default would be our current \"exhaustive\", and this --quiet case\nwould be covered by lossyButFaster, but also things like the\n\"--no-ahead-behind\" flag for git-status.\n\nJust on this implementation: The usual idiom for flags as config is\ncommand.flag=xyz, not command.flagDefault=xyz, so this should be\nreset.quiet.\n"},{"id":"361293","messageId":"CACsJy8C+9f3hFxmrqAN2hi1AeBTa1yZdnwX6iJtsy_OrEfTWpQ@mail.gmail.com","threadId":"49595","inReplyTo":"MW2PR2101MB0970EF1065717A38CF581C64F4F40@MW2PR2101MB0970.namprd21.prod.outlook.com","subject":"Re: [PATCH v3 1/3] reset: don't compute unstaged changes after reset when --quiet","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-23T15:46:35Z","receivedAt":"2018-10-23T15:47:04Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Oct 23, 2018 at 1:01 AM Ben Peart wrote:\n>\n> > -----Original Message-----\n> > From: Johannes Schindelin \n> > Sent: Monday, October 22, 2018 4:45 PM\n> > To: Ben Peart \n> > Cc: git@vger.kernel.org; gitster@pobox.com; Ben Peart\n> > ; peff@peff.net; sunshine@sunshineco.com\n> > Subject: Re: [PATCH v3 1/3] reset: don't compute unstaged changes after\n> > reset when --quiet\n> >\n> > Hi Ben,\n> >\n> > On Mon, 22 Oct 2018, Ben Peart wrote:\n> >\n> > > From: Ben Peart \n> > >\n> > > When git reset is run with the --quiet flag, don't bother finding any\n> > > additional unstaged changes as they won't be output anyway. This speeds\n> > up\n> > > the git reset command by avoiding having to lstat() every file looking for\n> > > changes that aren't going to be reported anyway.\n> > >\n> > > The savings can be significant. In a repo with 200K files \"git reset\"\n> > > drops from 7.16 seconds to 0.32 seconds for a savings of 96%.\n> >\n> > That's very nice!\n> >\n> > Those numbers, just out of curiosity, are they on Windows? Or on Linux?\n> >\n>\n> It's safe to assume all my numbers are on Windows. :-)\n\nIt does bug me about this. Next time please mention the platform you\ntested on in the commit message. Not all platforms behave the same way\nespecially when it comes to performance.\n\n>\n> > Ciao,\n> > Dscho\n>\n>\n\n\n-- \nDuy\n"},{"id":"361302","messageId":"50195f7a-c452-c784-ab23-d05956d48470@gmail.com","threadId":"49595","inReplyTo":"xmqqh8hdtsro.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 3/3] reset: warn when refresh_index() takes more than 2 seconds","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-23T17:12:53Z","receivedAt":"2018-10-23T17:12:57Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/22/2018 8:23 PM, Junio C Hamano wrote:\n> Ben Peart writes:\n> \n>> From: Ben Peart \n>>\n>> refresh_index() is done after a reset command as an optimization. Because\n>> it can be an expensive call, warn the user if it takes more than 2 seconds\n>> and tell them how to avoid it using the --quiet command line option or\n>> reset.quiet config setting.\n> \n> I am moderately negative on this step. It will irritate users who\n> know about and still choose not to use the \"--quiet\" option, because\n> they want to gain performance in later real work and/or they want to\n> know what paths are now dirty. A working tree that needs long time\n> to refresh will take long time to instead do \"cached stat info says\n> it may be modified so let's run 'diff' for real---we may discover\n> that there wasn't any change after all\" when a \"git diff\" is run\n> after a \"reset --quiet\" that does not refresh; i.e. there would be\n> valid reasons to run \"reset\" without \"--quiet\".\n> \n> It feels a bit irresponsible to throw an ad without informing\n> pros-and-cons and to pretend that we are advising on BCP. In\n> general, we do *not* advertise new features randomly like this.\n> \n> Thanks. The previous two steps looks quite sensible.\n> \n\nThe challenge I'm trying to address is the discoverability of this \nsignificant performance win. In earlier review feedback, all mention of \nthis option speeding up reset was removed. I added this patch to enable \nusers to find out it even exists as an option.\n\nWhile I modeled this on the untracked files/--uno and ahead/behind \nlogic, I missed adding this to the 'advice' logic so that it can be \nturned off and avoid irritating users. I'll send an updated patch that \ncorrects that.\n"},{"id":"361304","messageId":"6feb67a1-17fa-4280-9e31-963b619fa051@gmail.com","threadId":"49595","inReplyTo":"20181022200601.GC9917@sigill.intra.peff.net","subject":"Re: [PATCH v3 2/3] reset: add new reset.quiet config setting","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-23T17:31:58Z","receivedAt":"2018-10-23T17:32:03Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/22/2018 4:06 PM, Jeff King wrote:\n> On Mon, Oct 22, 2018 at 08:13:32PM +0100, Ramsay Jones wrote:\n> \n>>> -q::\n>>> --quiet::\n>>> -\tBe quiet, only report errors.\n>>> +--no-quiet::\n>>> +\tBe quiet, only report errors. The default behavior respects the\n>>> +\t`reset.quiet` config option, or `--no-quiet` if that is not set.\n>>\n>> Sorry, I can't quite parse this; -q,--quiet and --no-quiet on the\n>> command line (should) trump whatever rest.quiet is set to in the\n>> configuration. Is that not the case?\n> \n> That is the case, and what was meant by \"the default behavior\" (i.e.,\n> the behavior when none of these is used). Maybe there's a more clear way\n> of saying that.\n> \n> -Peff\n> \n\nIs this more clear?\n\n-q::\n--quiet::\n--no-quiet::\n\tBe quiet, only report errors. The default behavior is set by the\n\t`reset.quiet` config option. `--quiet` and `--no-quiet` will\n\toverwrite the default behavior.\n"},{"id":"361305","messageId":"20181023173504.GA2076@sigill.intra.peff.net","threadId":"49595","inReplyTo":"6feb67a1-17fa-4280-9e31-963b619fa051@gmail.com","subject":"Re: [PATCH v3 2/3] reset: add new reset.quiet config setting","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-23T17:35:04Z","receivedAt":"2018-10-23T17:35:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 23, 2018 at 01:31:58PM -0400, Ben Peart wrote:\n\n> On 10/22/2018 4:06 PM, Jeff King wrote:\n> > On Mon, Oct 22, 2018 at 08:13:32PM +0100, Ramsay Jones wrote:\n> > \n> > > > -q::\n> > > > --quiet::\n> > > > -\tBe quiet, only report errors.\n> > > > +--no-quiet::\n> > > > +\tBe quiet, only report errors. The default behavior respects the\n> > > > +\t`reset.quiet` config option, or `--no-quiet` if that is not set.\n> > > \n> > > Sorry, I can't quite parse this; -q,--quiet and --no-quiet on the\n> > > command line (should) trump whatever rest.quiet is set to in the\n> > > configuration. Is that not the case?\n> > \n> > That is the case, and what was meant by \"the default behavior\" (i.e.,\n> > the behavior when none of these is used). Maybe there's a more clear way\n> > of saying that.\n> > \n> \n> Is this more clear?\n> \n> -q::\n> --quiet::\n> --no-quiet::\n> \tBe quiet, only report errors. The default behavior is set by the\n> \t`reset.quiet` config option. `--quiet` and `--no-quiet` will\n> \toverwrite the default behavior.\n\nThat looks OK to me (but then so did the earlier one ;) ).\n\nI'd probably s/overwrite/override/.\n\n-Peff\n"},{"id":"361307","messageId":"1ba81f12-7040-1ba5-2009-fa681caf9874@gmail.com","threadId":"49595","inReplyTo":"874lddc9fs.fsf@evledraar.gmail.com","subject":"Re: [PATCH v1 2/2] reset: add new reset.quietDefault config setting","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-23T18:11:01Z","receivedAt":"2018-10-23T18:11:06Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/23/2018 5:13 AM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Wed, Oct 17 2018, Jeff King wrote:\n> \n>> On Wed, Oct 17, 2018 at 02:19:59PM -0400, Eric Sunshine wrote:\n>>\n>>> On Wed, Oct 17, 2018 at 12:40 PM Ben Peart wrote:\n>>>> Add a reset.quietDefault config setting that sets the default value of the\n>>>> --quiet flag when running the reset command. This enables users to change\n>>>> the default behavior to take advantage of the performance advantages of\n>>>> avoiding the scan for unstaged changes after reset. Defaults to false.\n>>>\n>>> As with the previous patch, my knee-jerk reaction is that this really\n>>> feels wrong being tied to --quiet. It's particularly unintuitive.\n>>>\n>>> What I _could_ see, and what would feel more natural is if you add a\n>>> new option (say, --optimize) which is more general, incorporating\n>>> whatever optimizations become available in the future, not just this\n>>> one special-case. A side-effect of --optimize is that it implies\n>>> --quiet, and that is something which can and should be documented.\n>>\n>> Heh, I just wrote something very similar elsewhere in the thread. I'm\n>> still not sure if it's a dumb idea, but at least we can be dumb\n>> together.\n> \n> Same here. I'm in general if favor of having the ability to configure\n> porcelain command-line options, but in this case it seems like it would\n> be more logical to head for something like:\n> \n> core.uiMessaging=[default,exhaustive,lossyButFaster,quiet]\n> \n> Where default would be our current \"exhaustive\", and this --quiet case\n> would be covered by lossyButFaster, but also things like the\n> \"--no-ahead-behind\" flag for git-status.\n> \n\nThis sounds like an easy way to choose a set of default values that we \nthink make sense to get bundled together. That could be a way for users \nto quickly choose a set of good defaults but I still think you would \nwant find grained control over the individual settings.\n\nComing up with the set of values to bundle together, figuring out the \nhierarchy of precedence for this new global config->individual \nconfig->individual command line, updating the code to make it all work \nis outside the scope of this particular patch series.\n\n> Just on this implementation: The usual idiom for flags as config is\n> command.flag=xyz, not command.flagDefault=xyz, so this should be\n> reset.quiet.\n> \n\nThanks, I agree and fixed that in later iterations.\n"},{"id":"361309","messageId":"e1f50b07-b3bf-0805-fcc9-692331dd170a@gmail.com","threadId":"49595","inReplyTo":"CACsJy8Dcf8OknyMaSZxOaib54jLSSt71XXjTZD3UjgnH6J7QFA@mail.gmail.com","subject":"Re: [PATCH v3 2/3] reset: add new reset.quiet config setting","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-23T18:47:43Z","receivedAt":"2018-10-23T18:47:48Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/22/2018 10:45 AM, Duy Nguyen wrote:\n> On Mon, Oct 22, 2018 at 3:38 PM Ben Peart wrote:\n>>\n>> From: Ben Peart \n>>\n>> Add a reset.quiet config setting that sets the default value of the --quiet\n>> flag when running the reset command. This enables users to change the\n>> default behavior to take advantage of the performance advantages of\n>> avoiding the scan for unstaged changes after reset. Defaults to false.\n>>\n>> Signed-off-by: Ben Peart \n>> ---\n>> Documentation/config.txt | 3 +++\n>> Documentation/git-reset.txt | 4 +++-\n>> builtin/reset.c | 1 +\n>> 3 files changed, 7 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>> index f6f4c21a54..a2d1b8b116 100644\n>> --- a/Documentation/config.txt\n>> +++ b/Documentation/config.txt\n>> @@ -2728,6 +2728,9 @@ rerere.enabled::\n>> `$GIT_DIR`, e.g. if \"rerere\" was previously used in the\n>> repository.\n>>\n>> +reset.quiet::\n>> + When set to true, 'git reset' will default to the '--quiet' option.\n>> +\n> \n> With 'nd/config-split' topic moving pretty much all config keys out of\n> config.txt, you probably want to do the same for this series: add this\n> in a new file called Documentation/reset-config.txt then include the\n> file here like the sendemail one below.\n> \n\nSeems a bit overkill to pull a line of documentation into a separate \nfile and replace it with a line of 'import' logic. Perhaps if/when \nthere is more documentation to pull out that would make more sense.\n\n>> include::sendemail-config.txt[]\n>>\n>> sequence.editor::\n"},{"id":"361311","messageId":"20181023190423.5772-1-peartben@gmail.com","threadId":"49595","inReplyTo":"20181017164021.15204-1-peartben@gmail.com","subject":"[PATCH v4 0/3] speed up git reset","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-23T19:04:20Z","receivedAt":"2018-10-23T19:04:35Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"From: Ben Peart \n\nUpdated the wording in the documentation and commit messages to (hopefully)\nmake it clearer. Added the warning about 'reset --quiet' to the advice\nsystem so that it can be turned off.\n\nBase Ref: \nWeb-Diff: https://github.com/benpeart/git/commit/8a2fef45d4\nCheckout: git fetch https://github.com/benpeart/git reset-refresh-index-v4 && git checkout 8a2fef45d4\n\n\n### Patches\n\nBen Peart (3):\n reset: don't compute unstaged changes after reset when --quiet\n reset: add new reset.quiet config setting\n reset: warn when refresh_index() takes more than 2 seconds\n\n Documentation/config.txt | 7 +++++++\n Documentation/git-reset.txt | 5 ++++-\n advice.c | 2 ++\n advice.h | 1 +\n builtin/reset.c | 15 ++++++++++++++-\n 5 files changed, 28 insertions(+), 2 deletions(-)\n\n\nbase-commit: ca63497355222acefcca02b9cbb540a4768f3286\n-- \n2.18.0.windows.1\n\n\n"},{"id":"361312","messageId":"20181023190423.5772-2-peartben@gmail.com","threadId":"49595","inReplyTo":"20181023190423.5772-1-peartben@gmail.com","subject":"[PATCH v4 1/3] reset: don't compute unstaged changes after reset when --quiet","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-23T19:04:21Z","receivedAt":"2018-10-23T19:04:37Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"From: Ben Peart \n\nWhen git reset is run with the --quiet flag, don't bother finding any\nadditional unstaged changes as they won't be output anyway. This speeds up\nthe git reset command by avoiding having to lstat() every file looking for\nchanges that aren't going to be reported anyway.\n\nThe savings can be significant. In a repo on Windows with 200K files\n\"git reset\" drops from 7.16 seconds to 0.32 seconds for a savings of 96%.\n\nSigned-off-by: Ben Peart \n---\n builtin/reset.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 11cd0dcb8c..04f0d9b4f5 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -375,7 +375,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tint flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;\n \t\t\tif (read_from_tree(&pathspec, &oid, intent_to_add))\n \t\t\t\treturn 1;\n-\t\t\tif (get_git_work_tree())\n+\t\t\tif (!quiet && get_git_work_tree())\n \t\t\t\trefresh_index(&the_index, flags, NULL, NULL,\n \t\t\t\t\t _(\"Unstaged changes after reset:\"));\n \t\t} else {\n-- \n2.18.0.windows.1\n\n"},{"id":"361314","messageId":"20181023190423.5772-3-peartben@gmail.com","threadId":"49595","inReplyTo":"20181023190423.5772-1-peartben@gmail.com","subject":"[PATCH v4 2/3] reset: add new reset.quiet config setting","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-23T19:04:22Z","receivedAt":"2018-10-23T19:04:39Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"From: Ben Peart \n\nAdd a reset.quiet config setting that sets the default value of the --quiet\nflag when running the reset command. This enables users to change the\ndefault behavior to take advantage of the performance advantages of\navoiding the scan for unstaged changes after reset. Defaults to false.\n\nSigned-off-by: Ben Peart \n---\n Documentation/config.txt | 3 +++\n Documentation/git-reset.txt | 5 ++++-\n builtin/reset.c | 1 +\n 3 files changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex f6f4c21a54..a2d1b8b116 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2728,6 +2728,9 @@ rerere.enabled::\n \t`$GIT_DIR`, e.g. if \"rerere\" was previously used in the\n \trepository.\n \n+reset.quiet::\n+\tWhen set to true, 'git reset' will default to the '--quiet' option.\n+\n include::sendemail-config.txt[]\n \n sequence.editor::\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex 1d697d9962..2dac95c71a 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -95,7 +95,10 @@ OPTIONS\n \n -q::\n --quiet::\n-\tBe quiet, only report errors.\n+--no-quiet::\n+\tBe quiet, only report errors. The default behavior is set by the\n+\t`reset.quiet` config option. `--quiet` and `--no-quiet` will\n+\toverride the default behavior.\n \n \n EXAMPLES\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 04f0d9b4f5..3b43aee544 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -306,6 +306,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t};\n \n \tgit_config(git_reset_config, NULL);\n+\tgit_config_get_bool(\"reset.quiet\", &quiet);\n \n \targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n-- \n2.18.0.windows.1\n\n"},{"id":"361313","messageId":"20181023190423.5772-4-peartben@gmail.com","threadId":"49595","inReplyTo":"20181023190423.5772-1-peartben@gmail.com","subject":"[PATCH v4 3/3] reset: warn when refresh_index() takes more than 2 seconds","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-23T19:04:23Z","receivedAt":"2018-10-23T19:04:40Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"From: Ben Peart \n\nrefresh_index() is done after a reset command as an optimization. Because\nit can be an expensive call, warn the user if it takes more than 2 seconds\nand tell them how to avoid it using the --quiet command line option or\nreset.quiet config setting.\n\nSigned-off-by: Ben Peart \n---\n Documentation/config.txt | 4 ++++\n advice.c | 2 ++\n advice.h | 1 +\n builtin/reset.c | 14 +++++++++++++-\n 4 files changed, 20 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex a2d1b8b116..415db31def 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -333,6 +333,10 @@ advice.*::\n \tcommitBeforeMerge::\n \t\tAdvice shown when linkgit:git-merge[1] refuses to\n \t\tmerge to avoid overwriting local changes.\n+\tresetQuiet::\n+\t\tAdvice to consider using the `--quiet` option to linkgit:git-reset[1]\n+\t\twhen the command takes more than 2 seconds to enumerate unstaged\n+\t\tchanges after reset.\n \tresolveConflict::\n \t\tAdvice shown by various commands when conflicts\n \t\tprevent the operation from being performed.\ndiff --git a/advice.c b/advice.c\nindex 3561cd64e9..5f35656409 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -12,6 +12,7 @@ int advice_push_needs_force = 1;\n int advice_status_hints = 1;\n int advice_status_u_option = 1;\n int advice_commit_before_merge = 1;\n+int advice_reset_quiet_warning = 1;\n int advice_resolve_conflict = 1;\n int advice_implicit_identity = 1;\n int advice_detached_head = 1;\n@@ -65,6 +66,7 @@ static struct {\n \t{ \"statusHints\", &advice_status_hints },\n \t{ \"statusUoption\", &advice_status_u_option },\n \t{ \"commitBeforeMerge\", &advice_commit_before_merge },\n+\t{ \"resetQuiet\", &advice_reset_quiet_warning },\n \t{ \"resolveConflict\", &advice_resolve_conflict },\n \t{ \"implicitIdentity\", &advice_implicit_identity },\n \t{ \"detachedHead\", &advice_detached_head },\ndiff --git a/advice.h b/advice.h\nindex ab24df0fd0..696bf0e7d2 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -12,6 +12,7 @@ extern int advice_push_needs_force;\n extern int advice_status_hints;\n extern int advice_status_u_option;\n extern int advice_commit_before_merge;\n+extern int advice_reset_quiet_warning;\n extern int advice_resolve_conflict;\n extern int advice_implicit_identity;\n extern int advice_detached_head;\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 3b43aee544..b31a0eae8a 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -25,6 +25,8 @@\n #include \"submodule.h\"\n #include \"submodule-config.h\"\n \n+#define REFRESH_INDEX_DELAY_WARNING_IN_MS (2 * 1000)\n+\n static const char * const git_reset_usage[] = {\n \tN_(\"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []\"),\n \tN_(\"git reset [-q] [] [--] ...\"),\n@@ -376,9 +378,19 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tint flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;\n \t\t\tif (read_from_tree(&pathspec, &oid, intent_to_add))\n \t\t\t\treturn 1;\n-\t\t\tif (!quiet && get_git_work_tree())\n+\t\t\tif (!quiet && get_git_work_tree()) {\n+\t\t\t\tuint64_t t_begin, t_delta_in_ms;\n+\n+\t\t\t\tt_begin = getnanotime();\n \t\t\t\trefresh_index(&the_index, flags, NULL, NULL,\n \t\t\t\t\t _(\"Unstaged changes after reset:\"));\n+\t\t\t\tt_delta_in_ms = (getnanotime() - t_begin) / 1000000;\n+\t\t\t\tif (advice_reset_quiet_warning && t_delta_in_ms > REFRESH_INDEX_DELAY_WARNING_IN_MS) {\n+\t\t\t\t\tprintf(_(\"\\nIt took %.2f seconds to enumerate unstaged changes after reset. You can\\n\"\n+\t\t\t\t\t\t\"use '--quiet' to avoid this. Set the config setting reset.quiet to true\\n\"\n+\t\t\t\t\t\t\"to make this the default.\\n\"), t_delta_in_ms / 1000.0);\n+\t\t\t\t}\n+\t\t\t}\n \t\t} else {\n \t\t\tint err = reset_index(&oid, reset_type, quiet);\n \t\t\tif (reset_type == KEEP && !err)\n-- \n2.18.0.windows.1\n\n"},{"id":"361318","messageId":"nycvar.QRO.7.76.6.1810232153120.4546@tvgsbejvaqbjf.bet","threadId":"49595","inReplyTo":"CACsJy8C+9f3hFxmrqAN2hi1AeBTa1yZdnwX6iJtsy_OrEfTWpQ@mail.gmail.com","subject":"Re: [PATCH v3 1/3] reset: don't compute unstaged changes after reset when --quiet","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-23T19:55:34Z","receivedAt":"2018-10-23T19:55:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Duy,\n\nOn Tue, 23 Oct 2018, Duy Nguyen wrote:\n\n> On Tue, Oct 23, 2018 at 1:01 AM Ben Peart wrote:\n> >\n> > > -----Original Message-----\n> > > From: Johannes Schindelin \n> > > Sent: Monday, October 22, 2018 4:45 PM\n> > > To: Ben Peart \n> > > Cc: git@vger.kernel.org; gitster@pobox.com; Ben Peart\n> > > ; peff@peff.net; sunshine@sunshineco.com\n> > > Subject: Re: [PATCH v3 1/3] reset: don't compute unstaged changes after\n> > > reset when --quiet\n> > >\n> > > Hi Ben,\n> > >\n> > > On Mon, 22 Oct 2018, Ben Peart wrote:\n> > >\n> > > > From: Ben Peart \n> > > >\n> > > > When git reset is run with the --quiet flag, don't bother finding any\n> > > > additional unstaged changes as they won't be output anyway. This speeds\n> > > up\n> > > > the git reset command by avoiding having to lstat() every file looking for\n> > > > changes that aren't going to be reported anyway.\n> > > >\n> > > > The savings can be significant. In a repo with 200K files \"git reset\"\n> > > > drops from 7.16 seconds to 0.32 seconds for a savings of 96%.\n> > >\n> > > That's very nice!\n> > >\n> > > Those numbers, just out of curiosity, are they on Windows? Or on Linux?\n> > >\n> >\n> > It's safe to assume all my numbers are on Windows. :-)\n> \n> It does bug me about this. Next time please mention the platform you\n> tested on in the commit message. Not all platforms behave the same way\n> especially when it comes to performance.\n\nAnd pretty much all different testing scenarios behave differently, too.\nAnd at some stage, we're asking for too many fries.\n\nIn other words: we always accepted performance improvements when it could\nbe demonstrated that they improved a certain not too uncommon scenario,\nand I do not think it would make sense to change this stance now. Not\nunless you can demonstrate a good reason why we should.\n\nCiao,\nJohannes\n"},{"id":"361322","messageId":"20181023200245.GA15214@sigill.intra.peff.net","threadId":"49595","inReplyTo":"1ba81f12-7040-1ba5-2009-fa681caf9874@gmail.com","subject":"Re: [PATCH v1 2/2] reset: add new reset.quietDefault config setting","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-23T20:02:46Z","receivedAt":"2018-10-23T20:02:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 23, 2018 at 02:11:01PM -0400, Ben Peart wrote:\n\n> This sounds like an easy way to choose a set of default values that we think\n> make sense to get bundled together. That could be a way for users to quickly\n> choose a set of good defaults but I still think you would want find grained\n> control over the individual settings.\n> \n> Coming up with the set of values to bundle together, figuring out the\n> hierarchy of precedence for this new global config->individual\n> config->individual command line, updating the code to make it all work is\n> outside the scope of this particular patch series.\n\nTrue, it probably does make sense to give individual defaults. Having a\nunifying option may help with the discoverability issue you were\nthinking of elsewhere, though.\n\n-Peff\n"},{"id":"361323","messageId":"87zhv4bfck.fsf@evledraar.gmail.com","threadId":"49595","inReplyTo":"1ba81f12-7040-1ba5-2009-fa681caf9874@gmail.com","subject":"Re: [PATCH v1 2/2] reset: add new reset.quietDefault config setting","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-23T20:03:23Z","receivedAt":"2018-10-23T20:03:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Oct 23 2018, Ben Peart wrote:\n\n> On 10/23/2018 5:13 AM, Ævar Arnfjörð Bjarmason wrote:\n>>\n>> On Wed, Oct 17 2018, Jeff King wrote:\n>>\n>>> On Wed, Oct 17, 2018 at 02:19:59PM -0400, Eric Sunshine wrote:\n>>>\n>>>> On Wed, Oct 17, 2018 at 12:40 PM Ben Peart wrote:\n>>>>> Add a reset.quietDefault config setting that sets the default value of the\n>>>>> --quiet flag when running the reset command. This enables users to change\n>>>>> the default behavior to take advantage of the performance advantages of\n>>>>> avoiding the scan for unstaged changes after reset. Defaults to false.\n>>>>\n>>>> As with the previous patch, my knee-jerk reaction is that this really\n>>>> feels wrong being tied to --quiet. It's particularly unintuitive.\n>>>>\n>>>> What I _could_ see, and what would feel more natural is if you add a\n>>>> new option (say, --optimize) which is more general, incorporating\n>>>> whatever optimizations become available in the future, not just this\n>>>> one special-case. A side-effect of --optimize is that it implies\n>>>> --quiet, and that is something which can and should be documented.\n>>>\n>>> Heh, I just wrote something very similar elsewhere in the thread. I'm\n>>> still not sure if it's a dumb idea, but at least we can be dumb\n>>> together.\n>>\n>> Same here. I'm in general if favor of having the ability to configure\n>> porcelain command-line options, but in this case it seems like it would\n>> be more logical to head for something like:\n>>\n>> core.uiMessaging=[default,exhaustive,lossyButFaster,quiet]\n>>\n>> Where default would be our current \"exhaustive\", and this --quiet case\n>> would be covered by lossyButFaster, but also things like the\n>> \"--no-ahead-behind\" flag for git-status.\n>>\n>\n> This sounds like an easy way to choose a set of default values that we\n> think make sense to get bundled together. That could be a way for\n> users to quickly choose a set of good defaults but I still think you\n> would want find grained control over the individual settings.\n\nWould you? It seems wanting to configure reset's --quiet in particular\nis purely a proxy goal for wanting to toggle off slow things in the\nUI. Otherwise why focus on it, and not the plethora of other --quiet\noptions we have?\n\n # Including (but probably not limited to):\n $ git grep -e OPT__QUIET -e '(OPT|option).*\"quiet\"' -- '*.[ch]' | wc -l\n 34\n\n> Coming up with the set of values to bundle together, figuring out the\n> hierarchy of precedence for this new global config->individual\n> config->individual command line[...]\n\nIf we'd still want reset.quiet & whatever the global \"turn off slow\nstuff\" UI option is then this part is easy and\ne.g. {transfer,fetch,receive}.fsckObjects can be used as a template for\nhow to do it.\n\n https://github.com/git/git/blob/v2.19.0/fetch-pack.c#L1432-L1443\n https://github.com/git/git/blob/v2.19.0/fetch-pack.c#L859-L863\n\nI.e. the more specific option always overrides the less specific one.\n\n> [...]updating the code to make it all work is outside the scope of\n> this particular patch series.\n\nIs that a Jedi mind trick to get out of patch review? :)\n\nI understand that it's not the patch you wrote, but sometimes feedback\nis \"maybe we shouldn't do this, but this other thing\".\n\nThe --ahead-behind config setting stalled on-list before:\nhttps://public-inbox.org/git/36e3a9c3-f7e2-4100-1bfc-647b809a09d0@jeffhostetler.com/\n\nNow we have this similarly themed thing.\n\nI think we need to be mindful of how changes like this can add up to\nvery confusing UI. I.e. in this case I can see a \"how take make git fast\non large repos\" post on stackoverflow in our future where the answer is\nsetting a bunch of seemingly irrelevant config options like reset.quiet\nand status.aheadbehind=false etc.\n\nSo maybe we should take a step back and consider if the real thing we\nwant is just some way for the user to tell git \"don't work so hard at\ncoming up with these values\".\n\nThat can also be smart, e.g. some \"auto\" setting that tweaks it based on\nestimated repo size so even with the same config your tiny dotfiles.git\nwill get \"ahead/behind\" reporting, but not when you cd into windows.git.\n\n>> Just on this implementation: The usual idiom for flags as config is\n>> command.flag=xyz, not command.flagDefault=xyz, so this should be\n>> reset.quiet.\n>>\n>\n> Thanks, I agree and fixed that in later iterations.\n"},{"id":"361345","messageId":"xmqq8t2oqchi.fsf@gitster-ct.c.googlers.com","threadId":"49595","inReplyTo":"e1f50b07-b3bf-0805-fcc9-692331dd170a@gmail.com","subject":"Re: [PATCH v3 2/3] reset: add new reset.quiet config setting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-24T02:56:09Z","receivedAt":"2018-10-24T02:56:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Peart writes:\n\n>>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>>> index f6f4c21a54..a2d1b8b116 100644\n>>> --- a/Documentation/config.txt\n>>> +++ b/Documentation/config.txt\n>>> @@ -2728,6 +2728,9 @@ rerere.enabled::\n>>> `$GIT_DIR`, e.g. if \"rerere\" was previously used in the\n>>> repository.\n>>>\n>>> +reset.quiet::\n>>> + When set to true, 'git reset' will default to the '--quiet' option.\n>>> +\n>>\n>> With 'nd/config-split' topic moving pretty much all config keys out of\n>> config.txt, you probably want to do the same for this series: add this\n>> in a new file called Documentation/reset-config.txt then include the\n>> file here like the sendemail one below.\n>>\n>\n> Seems a bit overkill to pull a line of documentation into a separate\n> file and replace it with a line of 'import' logic. Perhaps if/when\n> there is more documentation to pull out that would make more sense.\n\nThis change (ehh, rather, perhaps nd/config-split topic) came at an\nunfortunate moment. Until I actually did one integration cycle to\nrebuild 'pu' and merge this patch and the other topic, I had exactly\nthe same reaction as yours above to Duy's comment. But seeing the\ntree at the tip of 'pu' today, I do think the end result with a\nsingle liner file that has configuration for the \"reset\" command\nthat is included in config.txt would make sense, and I also think\nyou would agree with it if you see the same tree.\n\nHow we should get there is a different story. I think Duy's series\nneeds at least another update to move the split pieces into its own\nsubdirectory of Documentation/, and it is not all that urgent, while\nthis three-patch series (with the advice.* bit added) for \"reset\" is\npretty much ready to go 'next', so my gut feeling is that it is best\nto keep the description here, and to ask Duy to base the updated\nversion of config-split topic on top.\n\n\n"},{"id":"361348","messageId":"3c31d5c3-df46-69e3-c138-30a93d9b3ce4@ramsayjones.plus.com","threadId":"49595","inReplyTo":"20181023190423.5772-3-peartben@gmail.com","subject":"Re: [PATCH v4 2/3] reset: add new reset.quiet config setting","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-10-24T00:39:32Z","receivedAt":"2018-10-24T03:07:13Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 23/10/2018 20:04, Ben Peart wrote:\n> From: Ben Peart \n\nSorry for the late reply, ... I've been away from email - I am\nstill trying to catch up.\n\n> \n> Add a reset.quiet config setting that sets the default value of the --quiet\n> flag when running the reset command. This enables users to change the\n> default behavior to take advantage of the performance advantages of\n> avoiding the scan for unstaged changes after reset. Defaults to false.\n> \n> Signed-off-by: Ben Peart \n> ---\n> Documentation/config.txt | 3 +++\n> Documentation/git-reset.txt | 5 ++++-\n> builtin/reset.c | 1 +\n> 3 files changed, 8 insertions(+), 1 deletion(-)\n> \n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index f6f4c21a54..a2d1b8b116 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -2728,6 +2728,9 @@ rerere.enabled::\n> \t`$GIT_DIR`, e.g. if \"rerere\" was previously used in the\n> \trepository.\n> \n> +reset.quiet::\n> +\tWhen set to true, 'git reset' will default to the '--quiet' option.\n\nMention that this 'Defaults to false'?\n\n> +\n> include::sendemail-config.txt[]\n> \n> sequence.editor::\n> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\n> index 1d697d9962..2dac95c71a 100644\n> --- a/Documentation/git-reset.txt\n> +++ b/Documentation/git-reset.txt\n> @@ -95,7 +95,10 @@ OPTIONS\n> \n> -q::\n> --quiet::\n> -\tBe quiet, only report errors.\n> +--no-quiet::\n> +\tBe quiet, only report errors. The default behavior is set by the\n> +\t`reset.quiet` config option. `--quiet` and `--no-quiet` will\n> +\toverride the default behavior.\n\nBetter than last time, but how about something like:\n\n -q::\n --quiet::\n --no-quiet::\n Be quiet, only report errors. The default behaviour of the\n command, which is to not be quiet, can be specified by the\n `reset.quiet` configuration variable. The `--quiet` and\n `--no-quiet` options can be used to override any configured\n default.\n\nHmm, I am not sure that is any better! :-D\n\nAlso, note that the --no-option is often described separately to\nthe --option (in a separate paragraph). I don't know if that would\nhelp here.\n\n[The default behaviour is _not_ set by the configuration, if no\nconfiguration is specified. :-P ]\n\nNot sure if that helps!\n\nATB,\nRamsay Jones\n\n> \n> \n> EXAMPLES\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 04f0d9b4f5..3b43aee544 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -306,6 +306,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \t};\n> \n> \tgit_config(git_reset_config, NULL);\n> +\tgit_config_get_bool(\"reset.quiet\", &quiet);\n> \n> \targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n> \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n> \n"},{"id":"361374","messageId":"xmqqva5rn72r.fsf@gitster-ct.c.googlers.com","threadId":"49595","inReplyTo":"xmqq8t2oqchi.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 2/3] reset: add new reset.quiet config setting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-24T07:21:16Z","receivedAt":"2018-10-24T07:21:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> Seems a bit overkill to pull a line of documentation into a separate\n>> file and replace it with a line of 'import' logic. Perhaps if/when\n>> there is more documentation to pull out that would make more sense.\n>\n> This change (ehh, rather, perhaps nd/config-split topic) came at an\n> unfortunate moment. Until I actually did one integration cycle to\n> rebuild 'pu' and merge this patch and the other topic, I had exactly\n> the same reaction as yours above to Duy's comment. But seeing the\n> tree at the tip of 'pu' today, I do think the end result with a\n> single liner file that has configuration for the \"reset\" command\n> that is included in config.txt would make sense, and I also think\n> you would agree with it if you see the same tree.\n>\n> How we should get there is a different story. I think Duy's series\n> needs at least another update to move the split pieces into its own\n> subdirectory of Documentation/, and it is not all that urgent, while\n> this three-patch series (with the advice.* bit added) for \"reset\" is\n> pretty much ready to go 'next', so my gut feeling is that it is best\n> to keep the description here, and to ask Duy to base the updated\n> version of config-split topic on top.\n\nI'll take the \"it is not all that urgent\" bit (and only that bit)\nback, even though the conclusion would be the same. It is quite\npainful having to keep this topic while a few topics that touch the\nhuge Documentation/config.txt is in flight. The monolithic file is\nlarge enough that it does not cause much pain while many topics are\nin flight, but the single step of spliting it into million pieces\ndone by nd/config-split topic is a pain to merge.\n\nAnybody interested may fetch, and try\n\n $ git checkout 5c2d198e8e\n $ git merge b2358ceaca\n\nand imagine that you'd have to redo that every time somebody adds or\ntouches up a byte in the description of an individual entry in the\nDocumentation/config.txt file. It is not pretty until we finish the\nsplit.\n\n"},{"id":"361413","messageId":"CACsJy8AKWp859cGMwh0_tRwODPCAQ+Rmkaz6HQcy8UQOgMH-og@mail.gmail.com","threadId":"49595","inReplyTo":"e1f50b07-b3bf-0805-fcc9-692331dd170a@gmail.com","subject":"Re: [PATCH v3 2/3] reset: add new reset.quiet config setting","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-24T14:49:14Z","receivedAt":"2018-10-24T14:49:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Oct 23, 2018 at 8:47 PM Ben Peart wrote:\n>\n>\n>\n> On 10/22/2018 10:45 AM, Duy Nguyen wrote:\n> > On Mon, Oct 22, 2018 at 3:38 PM Ben Peart wrote:\n> >>\n> >> From: Ben Peart \n> >>\n> >> Add a reset.quiet config setting that sets the default value of the --quiet\n> >> flag when running the reset command. This enables users to change the\n> >> default behavior to take advantage of the performance advantages of\n> >> avoiding the scan for unstaged changes after reset. Defaults to false.\n> >>\n> >> Signed-off-by: Ben Peart \n> >> ---\n> >> Documentation/config.txt | 3 +++\n> >> Documentation/git-reset.txt | 4 +++-\n> >> builtin/reset.c | 1 +\n> >> 3 files changed, 7 insertions(+), 1 deletion(-)\n> >>\n> >> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> >> index f6f4c21a54..a2d1b8b116 100644\n> >> --- a/Documentation/config.txt\n> >> +++ b/Documentation/config.txt\n> >> @@ -2728,6 +2728,9 @@ rerere.enabled::\n> >> `$GIT_DIR`, e.g. if \"rerere\" was previously used in the\n> >> repository.\n> >>\n> >> +reset.quiet::\n> >> + When set to true, 'git reset' will default to the '--quiet' option.\n> >> +\n> >\n> > With 'nd/config-split' topic moving pretty much all config keys out of\n> > config.txt, you probably want to do the same for this series: add this\n> > in a new file called Documentation/reset-config.txt then include the\n> > file here like the sendemail one below.\n> >\n>\n> Seems a bit overkill to pull a line of documentation into a separate\n> file and replace it with a line of 'import' logic. Perhaps if/when\n> there is more documentation to pull out that would make more sense.\n\nThere are a couple benefits of having all config keys stored in the\nsame way (i.e. in separate files). Searching will be easier, you\n_know_ reset.stuff will be in reset-config.txt. If you mix both ways,\nyou may need to look in config.txt as well as searching\nreset-config.txt. This single config key also stands out when you look\nat the end result of nd/config-split. The config split also opens up\nan opportunity to include command-specific config in individual\ncommand man page if done consistently.\n-- \nDuy\n"},{"id":"361415","messageId":"CACsJy8Bkx5QS_4QV13FHpbZmhO=0oc3_BsBPQKdtjq6aouALFA@mail.gmail.com","threadId":"49595","inReplyTo":"xmqq8t2oqchi.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 2/3] reset: add new reset.quiet config setting","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-24T14:54:06Z","receivedAt":"2018-10-24T14:54:36Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Oct 24, 2018 at 4:56 AM Junio C Hamano wrote:\n> How we should get there is a different story. I think Duy's series\n> needs at least another update to move the split pieces into its own\n> subdirectory of Documentation/, and it is not all that urgent, while\n> this three-patch series (with the advice.* bit added) for \"reset\" is\n> pretty much ready to go 'next', so my gut feeling is that it is best\n> to keep the description here, and to ask Duy to base the updated\n> version of config-split topic on top.\n\nOK. Just to be sure we're on the same page. Am I waiting for all\nconfig changes to land in 'master', or do I rebase my series on\n'next'? I usually base on 'master' but the mention of 'next' here\nconfuses me a bit.\n-- \nDuy\n"},{"id":"361421","messageId":"29db5fed-4556-277e-7aad-7ff3233550a9@gmail.com","threadId":"49595","inReplyTo":"87zhv4bfck.fsf@evledraar.gmail.com","subject":"Recommended configurations (was Re: [PATCH v1 2/2] reset: add new reset.quietDefault config setting)","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-10-24T15:48:20Z","receivedAt":"2018-10-24T15:48:23Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/23/2018 4:03 PM, Ævar Arnfjörð Bjarmason wrote:\n> [snip]\n> The --ahead-behind config setting stalled on-list before:\n> https://public-inbox.org/git/36e3a9c3-f7e2-4100-1bfc-647b809a09d0@jeffhostetler.com/\n>\n> Now we have this similarly themed thing.\n>\n> I think we need to be mindful of how changes like this can add up to\n> very confusing UI. I.e. in this case I can see a \"how take make git fast\n> on large repos\" post on stackoverflow in our future where the answer is\n> setting a bunch of seemingly irrelevant config options like reset.quiet\n> and status.aheadbehind=false etc.\n>\n> So maybe we should take a step back and consider if the real thing we\n> want is just some way for the user to tell git \"don't work so hard at\n> coming up with these values\".\n>\n> That can also be smart, e.g. some \"auto\" setting that tweaks it based on\n> estimated repo size so even with the same config your tiny dotfiles.git\n> will get \"ahead/behind\" reporting, but not when you cd into windows.git.\n\nGenerally, there are a lot of config settings that are likely in the \"if \nyou have a big repo, then you should use this\" category. However, there \nis rarely a one-size-fits-all solution to these problems, just like \nthere are different ways a repo can be \"big\" (working directory? number \nof commits? submodules?).\n\nI would typically expect that users with _really_ big repos have the \nresources to have an expert tweak the settings that are best for that \ndata shape and share those settings with all the users. In VFS for Git, \nwe turn certain config settings on by default when mounting the repo \n[1], but others are either signaled through warning messages (like the \nstatus.aheadBehind config setting [2]).\n\nWe never upstreamed the status.aheadBehind config setting [2], but we \n_did_ upstream the command-line option as fd9b544 \"status: add \n--[no-]ahead-behind to status and commit for V2 format\". We didn't want \nto change the expected output permanently, so we didn't add the config \nsetting to our list of \"required\" settings, but instead created a list \nof optional settings [3]; these settings don't override the existing \nsettings so users can opt-out. (Now that we have the commit-graph \nenabled and kept up-to-date, it may be time to revisit the importance of \nthis setting.)\n\nAll of this is to say: it is probably a good idea to have some \n\"recommended configuration\" for big repos, but there will always be \npower users who want to tweak each and every one of these settings. I'm \nopen to design ideas of how to store a list of recommended \nconfigurations and how to set a group of config settings with one \ncommand (say, a \"git recommended-config [small|large|submodules]\" \nbuiltin that fills the local config with the important settings).\n\nThanks,\n-Stolee\n\n[1] \nhttps://github.com/Microsoft/VFSForGit/blob/7daa9f1133764a4e4bd87014833fc2091e6702c1/GVFS/GVFS/CommandLine/GVFSVerb.cs#L79-L104\n     Code in VFS for Git that enables \"required\" config settings.\n\n[2] \nhttps://github.com/Microsoft/git/commit/0cbe9e6b23e4d9008d4a1676e1dd6a87bdcd6ed5\n     status: add status.aheadbehind setting\n\n[3] \nhttps://github.com/Microsoft/VFSForGit/blob/7daa9f1133764a4e4bd87014833fc2091e6702c1/GVFS/GVFS/CommandLine/GVFSVerb.cs#L120-L123\n     Code in VFS for Git that enables \"optional\" config settings.\n"},{"id":"361438","messageId":"20181024235813.GA1399@sigill.intra.peff.net","threadId":"49595","inReplyTo":"29db5fed-4556-277e-7aad-7ff3233550a9@gmail.com","subject":"Re: Recommended configurations (was Re: [PATCH v1 2/2] reset: add new reset.quietDefault config setting)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-24T23:58:14Z","receivedAt":"2018-10-24T23:58:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 24, 2018 at 11:48:20AM -0400, Derrick Stolee wrote:\n\n> Generally, there are a lot of config settings that are likely in the \"if you\n> have a big repo, then you should use this\" category. However, there is\n> rarely a one-size-fits-all solution to these problems, just like there are\n> different ways a repo can be \"big\" (working directory? number of commits?\n> submodules?).\n> [...]\n> All of this is to say: it is probably a good idea to have some \"recommended\n> configuration\" for big repos, but there will always be power users who want\n> to tweak each and every one of these settings. I'm open to design ideas of\n> how to store a list of recommended configurations and how to set a group of\n> config settings with one command (say, a \"git recommended-config\n> [small|large|submodules]\" builtin that fills the local config with the\n> important settings).\n\nMaybe it would be useful to teach git-sizer[1] to recommend particular\nsettings based on the actual definitions of \"big\" that it measures.\n\nI do hope that some options will just be no-brainers to enable always,\nthough (e.g., I think in the long run commit-graph should just default\nto \"on\"; it's cheap to keep up to date and helps proportionally to the\nrepo size).\n\n[1] https://github.com/github/git-sizer\n"},{"id":"361443","messageId":"xmqqa7n2n81a.fsf@gitster-ct.c.googlers.com","threadId":"49595","inReplyTo":"CACsJy8Bkx5QS_4QV13FHpbZmhO=0oc3_BsBPQKdtjq6aouALFA@mail.gmail.com","subject":"Re: [PATCH v3 2/3] reset: add new reset.quiet config setting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-25T01:12:49Z","receivedAt":"2018-10-25T01:12:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> OK. Just to be sure we're on the same page. Am I waiting for all\n> config changes to land in 'master', or do I rebase my series on\n> 'next'? I usually base on 'master' but the mention of 'next' here\n> confuses me a bit.\n\nI was hoping that you can do something like:\n\n $ git fetch https://github.com/gitster/git \\\n --refmap=refs/heads/*:refs/remotes/broken-out/* \\\n\t bp/reset-quiet master\n $ git checkout broken-out/master^0\n $ git merge broekn-out/bp/reset-quiet\n $ git rebase HEAD np/config-split\n\nonce it is clear to everybody that Ben's reset series is ready to be\nmerged to 'next' (or it actually hits 'next').\n"},{"id":"361465","messageId":"xmqqd0rylla8.fsf@gitster-ct.c.googlers.com","threadId":"49595","inReplyTo":"20181024235813.GA1399@sigill.intra.peff.net","subject":"Re: Recommended configurations (was Re: [PATCH v1 2/2] reset: add new reset.quietDefault config setting)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-25T04:09:35Z","receivedAt":"2018-10-25T04:10:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I do hope that some options will just be no-brainers to enable always,\n> though (e.g., I think in the long run commit-graph should just default\n> to \"on\"; it's cheap to keep up to date and helps proportionally to the\n> repo size).\n\nSame here.\n\nWe should strive to make any feature to optimize for repositories\nwith a particular trait not to hurt too much to repositories without\nthat trait, so that we can start such a feature as opt-in but later\ncan make it the default for everybody. Sometimes it may not be\npossible, but my gut feeling is that features aiming for optimizing\nbig repositories should fundamentally need only very small overhead\nwhen enabled in a small repository.\n\nSo I view them not as a set of million \"if your repository matches\nthis criterion, turn it on\" knobs. Rather, they are \"we haven't\ntested it fully, but you can opt into the experiment a new way to do\nthe same operation, which is designed to optimize for repositories\nwith this trait. Enabling it even when your repository does not have\nthat trait and reporting regression is also very welcome, as it is a\ngood indication that the new way has rough edges at its corners\".\n\nThanks.\n"},{"id":"361470","messageId":"xmqqpnvyk4jc.fsf@gitster-ct.c.googlers.com","threadId":"49595","inReplyTo":"3c31d5c3-df46-69e3-c138-30a93d9b3ce4@ramsayjones.plus.com","subject":"Re: [PATCH v4 2/3] reset: add new reset.quiet config setting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-25T04:56:39Z","receivedAt":"2018-10-25T04:56:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>> index f6f4c21a54..a2d1b8b116 100644\n>> --- a/Documentation/config.txt\n>> +++ b/Documentation/config.txt\n>> @@ -2728,6 +2728,9 @@ rerere.enabled::\n>> \t`$GIT_DIR`, e.g. if \"rerere\" was previously used in the\n>> \trepository.\n>> \n>> +reset.quiet::\n>> +\tWhen set to true, 'git reset' will default to the '--quiet' option.\n>\n> Mention that this 'Defaults to false'?\n\nPerhaps.\n\n>> -q::\n>> --quiet::\n>> -\tBe quiet, only report errors.\n>> +--no-quiet::\n>> +\tBe quiet, only report errors. The default behavior is set by the\n>> +\t`reset.quiet` config option. `--quiet` and `--no-quiet` will\n>> +\toverride the default behavior.\n>\n> Better than last time, but how about something like:\n>\n> -q::\n> --quiet::\n> --no-quiet::\n> Be quiet, only report errors. The default behaviour of the\n> command, which is to not be quiet, can be specified by the\n> `reset.quiet` configuration variable. The `--quiet` and\n> `--no-quiet` options can be used to override any configured\n> default.\n>\n> Hmm, I am not sure that is any better! :-D\n\nTo be honest, I find the second sentence in your rewrite even more\nconfusing. It reads as if `reset.quiet` configuration variable \ncan be used to restore the \"show what is yet to be added\"\nbehaviour, due to the parenthetical mention of the default behaviour\nwithout any configuration.\n\n\tThe command reports what is yet to be added to the index\n\tafter `reset` by default. It can be made to only report\n\terrors with the `--quiet` option, or setting `reset.quiet`\n\tconfiguration variable to `true` (the latter can be\n\toverriden with `--no-quiet`).\n\nThat may not be much better, though X-<.\n"},{"id":"361497","messageId":"xmqqbm7igyw6.fsf@gitster-ct.c.googlers.com","threadId":"49595","inReplyTo":"xmqqpnvyk4jc.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4 2/3] reset: add new reset.quiet config setting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-25T09:26:49Z","receivedAt":"2018-10-25T09:26:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> To be honest, I find the second sentence in your rewrite even more\n> confusing. It reads as if `reset.quiet` configuration variable \n> can be used to restore the \"show what is yet to be added\"\n> behaviour, due to the parenthetical mention of the default behaviour\n> without any configuration.\n>\n> \tThe command reports what is yet to be added to the index\n> \tafter `reset` by default. It can be made to only report\n> \terrors with the `--quiet` option, or setting `reset.quiet`\n> \tconfiguration variable to `true` (the latter can be\n> \toverriden with `--no-quiet`).\n>\n> That may not be much better, though X-<.\n\nIn any case, the comments are getting closer to the bikeshedding\nterritory, that can be easily addressed incrementally. I am getting\nthe impression that everbody agrees that the change is desirable,\nsufficiently documented and properly implemented. \n\nShall we mark it for \"Will merge to 'next'\" in the what's cooking\nreport and leave further refinements to incremental updates as\nneeded?\n"},{"id":"361514","messageId":"7daf3329-15c4-d3de-227d-5c729b9cb824@gmail.com","threadId":"49595","inReplyTo":"xmqqbm7igyw6.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4 2/3] reset: add new reset.quiet config setting","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-10-25T13:26:13Z","receivedAt":"2018-10-25T13:26:20Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/25/2018 5:26 AM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n>> To be honest, I find the second sentence in your rewrite even more\n>> confusing. It reads as if `reset.quiet` configuration variable\n>> can be used to restore the \"show what is yet to be added\"\n>> behaviour, due to the parenthetical mention of the default behaviour\n>> without any configuration.\n>>\n>> \tThe command reports what is yet to be added to the index\n>> \tafter `reset` by default. It can be made to only report\n>> \terrors with the `--quiet` option, or setting `reset.quiet`\n>> \tconfiguration variable to `true` (the latter can be\n>> \toverriden with `--no-quiet`).\n>>\n>> That may not be much better, though X-<.\n> \n> In any case, the comments are getting closer to the bikeshedding\n> territory, that can be easily addressed incrementally. I am getting\n> the impression that everbody agrees that the change is desirable,\n> sufficiently documented and properly implemented.\n> \n> Shall we mark it for \"Will merge to 'next'\" in the what's cooking\n> report and leave further refinements to incremental updates as\n> needed?\n> \n\nWhile not great, I think it is good enough. I don't think either of the \nlast couple of rewrite attempts were clearly better than what is in the \nlatest patch. I'd agree we should merge to 'next' and if someone comes \nup with something great, we can update it then.\n"},{"id":"361534","messageId":"47378681-3d7d-97fd-cf2a-4a0fe344a9e1@ramsayjones.plus.com","threadId":"49595","inReplyTo":"xmqqbm7igyw6.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4 2/3] reset: add new reset.quiet config setting","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-10-25T17:04:22Z","receivedAt":"2018-10-25T17:04:27Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 25/10/2018 10:26, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n>> To be honest, I find the second sentence in your rewrite even more\n>> confusing. It reads as if `reset.quiet` configuration variable \n>> can be used to restore the \"show what is yet to be added\"\n>> behaviour, due to the parenthetical mention of the default behaviour\n>> without any configuration.\n>>\n>> \tThe command reports what is yet to be added to the index\n>> \tafter `reset` by default. It can be made to only report\n>> \terrors with the `--quiet` option, or setting `reset.quiet`\n>> \tconfiguration variable to `true` (the latter can be\n>> \toverriden with `--no-quiet`).\n>>\n>> That may not be much better, though X-<.\n> \n> In any case, the comments are getting closer to the bikeshedding\n> territory, that can be easily addressed incrementally. I am getting\n> the impression that everbody agrees that the change is desirable,\n> sufficiently documented and properly implemented. \n> \n> Shall we mark it for \"Will merge to 'next'\" in the what's cooking\n> report and leave further refinements to incremental updates as\n> needed?\n\nYeah, the first version gave me a 'huh?' moment (hence the\ncomment), the last version was better and, as you can see,\nI am no great shakes at wordsmith-ing documentation! ;-)\n\nThanks!\n\nATB,\nRamsay Jones\n\n"}]}