{"thread":{"id":"50682","subject":"[PATCH 1/2] legacy stash: fix \"rudimentary backport of -q\"","startedAt":"2019-03-07T15:29:38Z","lastAt":"2019-03-18T07:02:49Z","messageCount":13,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Johannes Schindelin","Thomas Gummerer"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"370910","messageId":"f76c7cb66c43a6c4bf712516949a867b09710103.1551972571.git.gitgitgadget@gmail.com","threadId":"50682","inReplyTo":"pull.159.git.gitgitgadget@gmail.com","subject":"[PATCH 1/2] legacy stash: fix \"rudimentary backport of -q\"","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-03-07T15:29:32Z","receivedAt":"2019-03-07T15:29:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen this developer backported support for `--quiet` to the scripted\nversion of `git stash` in 80590055ea (stash: optionally use the scripted\nversion again, 2018-12-20), it looked like a sane choice to use `eval`\nto execute the command line passed in via the parameter list of\n`maybe_quiet`.\n\nHowever, that is not what we should have done, as that command-line was\nalready in the correct shape.\n\nThis can be seen very clearly when passing arguments with special\ncharacters, like\n\n\tgit stash -- ':(glob)**/*.txt'\n\nSince this is exactly what we want to test in the next commit (where we\nfix this very incantation with the built-in stash), let's fix the legacy\nscripted version of `git stash` first.\n\nSigned-off-by: Johannes Schindelin \n---\n git-legacy-stash.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/git-legacy-stash.sh b/git-legacy-stash.sh\nindex 8a8c4a9270..f60e9b3e87 100755\n--- a/git-legacy-stash.sh\n+++ b/git-legacy-stash.sh\n@@ -86,17 +86,17 @@ maybe_quiet () {\n \t\tshift\n \t\tif test -n \"$GIT_QUIET\"\n \t\tthen\n-\t\t\teval \"$@\" 2>/dev/null\n+\t\t\t\"$@\" 2>/dev/null\n \t\telse\n-\t\t\teval \"$@\"\n+\t\t\t\"$@\"\n \t\tfi\n \t\t;;\n \t*)\n \t\tif test -n \"$GIT_QUIET\"\n \t\tthen\n-\t\t\teval \"$@\" >/dev/null 2>&1\n+\t\t\t\"$@\" >/dev/null 2>&1\n \t\telse\n-\t\t\teval \"$@\"\n+\t\t\t\"$@\"\n \t\tfi\n \t\t;;\n \tesac\n-- \ngitgitgadget\n\n"},{"id":"370911","messageId":"7b5cca61dea4f01f9bcdcb6d5d2a913d58a341d3.1551972571.git.gitgitgadget@gmail.com","threadId":"50682","inReplyTo":"pull.159.git.gitgitgadget@gmail.com","subject":"[PATCH 2/2] built-in stash: handle :(glob) pathspecs again","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-03-07T15:29:33Z","receivedAt":"2019-03-07T15:29:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen passing a list of pathspecs to, say, `git add`, we need to be\ncareful to use the original form, not the parsed form of the pathspecs.\n\nThis makes a difference e.g. when calling\n\n\tgit stash -- ':(glob)**/*.txt'\n\nwhere the original form includes the `:(glob)` prefix while the parsed\nform does not.\n\nHowever, in the built-in `git stash`, we passed the parsed (i.e.\nincorrect) form, and `git add` would fail with the error message:\n\n\tfatal: pathspec '**/*.txt' did not match any files\n\nat the stage where `git stash` drops the changes from the worktree, even\nif `refs/stash` has been actually updated successfully.\n\nThis fixes https://github.com/git-for-windows/git/issues/2037\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/stash.c | 5 +++--\n t/t3905-stash-include-untracked.sh | 6 ++++++\n 2 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 1bfa24030c..2f29d037c8 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -830,7 +830,7 @@ static void add_pathspecs(struct argv_array *args,\n \tint i;\n \n \tfor (i = 0; i < ps.nr; i++)\n-\t\targv_array_push(args, ps.items[i].match);\n+\t\targv_array_push(args, ps.items[i].original);\n }\n \n /*\n@@ -1466,7 +1466,8 @@ static int push_stash(int argc, const char **argv, const char *prefix)\n \t\t\t\t git_stash_push_usage,\n \t\t\t\t 0);\n \n-\tparse_pathspec(&ps, 0, PATHSPEC_PREFER_FULL, prefix, argv);\n+\tparse_pathspec(&ps, 0, PATHSPEC_PREFER_FULL | PATHSPEC_PREFIX_ORIGIN,\n+\t\t prefix, argv);\n \treturn do_push_stash(ps, stash_msg, quiet, keep_index, patch_mode,\n \t\t\t include_untracked);\n }\ndiff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\nindex cc1c8a7bb2..29ca76f2fb 100755\n--- a/t/t3905-stash-include-untracked.sh\n+++ b/t/t3905-stash-include-untracked.sh\n@@ -283,4 +283,10 @@ test_expect_success 'stash -u -- shows no changes when there are\n \ttest_i18ncmp expect actual\n '\n \n+test_expect_success 'stash -u with globs' '\n+\t>untracked.txt &&\n+\tgit stash -u -- \":(glob)**/*.txt\" &&\n+\ttest_path_is_missing untracked.txt\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"370912","messageId":"pull.159.git.gitgitgadget@gmail.com","threadId":"50682","inReplyTo":null,"subject":"[PATCH 0/2] stash: handle pathspec magic again","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-03-07T15:29:32Z","receivedAt":"2019-03-07T15:29:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"It was reported in https://github.com/git-for-windows/git/issues/2037 that \ngit stash -- ':(glob)**/*.testextension is broken. The problem is not even\nthe stash operation itself, it happens when the git add part of dropping the\nlocal changes is spawned: we simply passed the parsed pathspec instead of\nthe unparsed one.\n\nWhile verifying the fix, I also realized that the escape hatch was seriously\nbroken by my \"backport of the -q option\": It introduced four bogus eval \nstatements, which really need to be dropped.\n\nJohannes Schindelin (2):\n legacy stash: fix \"rudimentary backport of -q\"\n built-in stash: handle :(glob) pathspecs again\n\n builtin/stash.c | 5 +++--\n git-legacy-stash.sh | 8 ++++----\n t/t3905-stash-include-untracked.sh | 6 ++++++\n 3 files changed, 13 insertions(+), 6 deletions(-)\n\n\nbase-commit: 7906af0cb84c8e65656347909abd4e22b04d1c1e\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-159%2Fdscho%2Fstash-with-globs-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-159/dscho/stash-with-globs-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/159\n-- \ngitgitgadget\n"},{"id":"370936","messageId":"xmqq5zsukuyj.fsf@gitster-ct.c.googlers.com","threadId":"50682","inReplyTo":"pull.159.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] stash: handle pathspec magic again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-08T01:37:08Z","receivedAt":"2019-03-08T01:37:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> It was reported in https://github.com/git-for-windows/git/issues/2037 that \n> git stash -- ':(glob)**/*.testextension is broken. The problem is not even\n> the stash operation itself, it happens when the git add part of dropping the\n> local changes is spawned: we simply passed the parsed pathspec instead of\n> the unparsed one.\n>\n> While verifying the fix, I also realized that the escape hatch was seriously\n> broken by my \"backport of the -q option\": It introduced four bogus eval \n> statements, which really need to be dropped.\n\nThanks.\n\nWe seem to be piling many \"oops\" on top, even after the topic hits\n'next'. But fixes are better late than never ;-).\n\nWill queue.\n"},{"id":"370996","messageId":"nycvar.QRO.7.76.6.1903081709220.41@tvgsbejvaqbjf.bet","threadId":"50682","inReplyTo":"xmqq5zsukuyj.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/2] stash: handle pathspec magic again","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-03-08T16:12:02Z","receivedAt":"2019-03-08T16:12:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 8 Mar 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> \n> > It was reported in https://github.com/git-for-windows/git/issues/2037 that \n> > git stash -- ':(glob)**/*.testextension is broken. The problem is not even\n> > the stash operation itself, it happens when the git add part of dropping the\n> > local changes is spawned: we simply passed the parsed pathspec instead of\n> > the unparsed one.\n> >\n> > While verifying the fix, I also realized that the escape hatch was seriously\n> > broken by my \"backport of the -q option\": It introduced four bogus eval \n> > statements, which really need to be dropped.\n> \n> Thanks.\n> \n> We seem to be piling many \"oops\" on top, even after the topic hits\n> 'next'. But fixes are better late than never ;-).\n\nYes, we do. At least now I do not have the impression that I have to\nimpose on Paul to integrate whatever diffs I came up with, I can now just\nsubmit small patch series that are easily reviewed.\n\nIf you care deeply about the commit history, I hereby offer to you to\nclean up the built-in stash patches when you say you're ready to advance\nthem to `master`; I will then squash the fixes into the proper places to\nmake it a nicer read, with the promise that the tree will be the same in\nthe end as with the oops-upon-oops patch history that we're piling up in\n`next`. I would do that for you.\n\n> Will queue.\n\nThanks,\nDscho\n"},{"id":"371053","messageId":"xmqqva0rmtrg.fsf@gitster-ct.c.googlers.com","threadId":"50682","inReplyTo":"nycvar.QRO.7.76.6.1903081709220.41@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 0/2] stash: handle pathspec magic again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-10T00:56:51Z","receivedAt":"2019-03-10T00:56:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> If you care deeply about the commit history, I hereby offer to you to\n> clean up the built-in stash patches when you say you're ready to advance\n> them to `master`.\n\nWhat's the goal of such a rebase? To rebuild the topic as a\nsensible sequence of commits that logically builds on top of\nprevious steps to ease later bisection and understanding?\n\nThanks for an offer out of good intentions,, but let's move on and\npolish the tree shape at the tip of this topic. The history behind\nit may be messier than other segments of our history, and future\ndevelopers may have harder time learning the intention of the topic\nwhen making changes on top, but this one was supposed to create a\nbug-to-bug reimplementation of the scripted version. What matters\nmore would be our future changes on top of this code, which improves\nwhat we used to have as scripted Porcelain. They will genuinely be\nnovel efforts, need to be built in logical order and explainable\nsteps to help future developers. Compared to that, so the history\nof our stumbling along the way to reach today's tip of the topic\nhas much lower value.\n\nBesides I think it is way too late for the current topic. We\nestablished before the topic hit 'next' that reviewers' eyes all\nlost freshness and patience to review another round of this series\nadequately.\n\nWe at least know that the ordering and organization of the iteration\nwe see in 'next' is crappy, because some reviewers did look at them.\nThe rewrite will see no reviews, if any, far fewer and shallower\nreviews than the iteration we have; nobody would be able to say with\nconfidence that the rewritten series achieves its goal of leaving a\nsensible history. Doing so just before it hits 'master' makes it a\nsure thing.\n\nLet's just we all admit that we did a poor job when we decided to\npush this topic to 'next' before it was ready, and learn the lesson\nto avoid haste making waste for the future topics.\n\nThanks.\n"},{"id":"371124","messageId":"xmqqlg1lkh11.fsf@gitster-ct.c.googlers.com","threadId":"50682","inReplyTo":"f76c7cb66c43a6c4bf712516949a867b09710103.1551972571.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] legacy stash: fix \"rudimentary backport of -q\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-11T07:27:06Z","receivedAt":"2019-03-11T07:27:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> This can be seen very clearly when passing arguments with special\n> characters, like\n>\n> \tgit stash -- ':(glob)**/*.txt'\n>\n> Since this is exactly what we want to test in the next commit (where we\n> fix this very incantation with the built-in stash), let's fix the legacy\n> scripted version of `git stash` first.\n\nThanks. I agree that these eval are evaluating one iteration too\nmuch.\n\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> git-legacy-stash.sh | 8 ++++----\n> 1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/git-legacy-stash.sh b/git-legacy-stash.sh\n> index 8a8c4a9270..f60e9b3e87 100755\n> --- a/git-legacy-stash.sh\n> +++ b/git-legacy-stash.sh\n> @@ -86,17 +86,17 @@ maybe_quiet () {\n> \t\tshift\n> \t\tif test -n \"$GIT_QUIET\"\n> \t\tthen\n> -\t\t\teval \"$@\" 2>/dev/null\n> +\t\t\t\"$@\" 2>/dev/null\n> \t\telse\n> -\t\t\teval \"$@\"\n> +\t\t\t\"$@\"\n> \t\tfi\n> \t\t;;\n> \t*)\n> \t\tif test -n \"$GIT_QUIET\"\n> \t\tthen\n> -\t\t\teval \"$@\" >/dev/null 2>&1\n> +\t\t\t\"$@\" >/dev/null 2>&1\n> \t\telse\n> -\t\t\teval \"$@\"\n> +\t\t\t\"$@\"\n> \t\tfi\n> \t\t;;\n> \tesac\n"},{"id":"371125","messageId":"xmqqh8c9kgy6.fsf@gitster-ct.c.googlers.com","threadId":"50682","inReplyTo":"7b5cca61dea4f01f9bcdcb6d5d2a913d58a341d3.1551972571.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] built-in stash: handle :(glob) pathspecs again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-11T07:28:49Z","receivedAt":"2019-03-11T07:28:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> When passing a list of pathspecs to, say, `git add`, we need to be\n> careful to use the original form, not the parsed form of the pathspecs.\n>\n> This makes a difference e.g. when calling\n>\n> \tgit stash -- ':(glob)**/*.txt'\n>\n> where the original form includes the `:(glob)` prefix while the parsed\n> form does not.\n>\n> However, in the built-in `git stash`, we passed the parsed (i.e.\n> incorrect) form, and `git add` would fail with the error message:\n>\n> \tfatal: pathspec '**/*.txt' did not match any files\n>\n> at the stage where `git stash` drops the changes from the worktree, even\n> if `refs/stash` has been actually updated successfully.\n>\n> This fixes https://github.com/git-for-windows/git/issues/2037\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> builtin/stash.c | 5 +++--\n> t/t3905-stash-include-untracked.sh | 6 ++++++\n> 2 files changed, 9 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/stash.c b/builtin/stash.c\n> index 1bfa24030c..2f29d037c8 100644\n> --- a/builtin/stash.c\n> +++ b/builtin/stash.c\n> @@ -830,7 +830,7 @@ static void add_pathspecs(struct argv_array *args,\n> \tint i;\n> \n> \tfor (i = 0; i < ps.nr; i++)\n> -\t\targv_array_push(args, ps.items[i].match);\n> +\t\targv_array_push(args, ps.items[i].original);\n> }\n\nYup. I think Thomas and Peff were also looking at the vicinity of\nthis code wrt the pass-by-value-ness of ps parameter. Their fix\nneed to also come on top of this (or combined together).\n"},{"id":"371160","messageId":"nycvar.QRO.7.76.6.1903111714430.41@tvgsbejvaqbjf.bet","threadId":"50682","inReplyTo":"xmqqva0rmtrg.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/2] stash: handle pathspec magic again","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-03-11T16:25:26Z","receivedAt":"2019-03-11T16:25:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Sun, 10 Mar 2019, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > If you care deeply about the commit history, I hereby offer to you to\n> > clean up the built-in stash patches when you say you're ready to advance\n> > them to `master`.\n> \n> What's the goal of such a rebase?\n\nTo appease you enough that you stop complaining about the current, or\nprevious, state of `ps/stash-in-c`.\n\nI am genuinely interested in making this all more pleasant for you, even\nif my efforts to that end show no fruit.\n\n> To rebuild the topic as a sensible sequence of commits that logically\n> builds on top of previous steps to ease later bisection and\n> understanding?\n> \n> Thanks for an offer out of good intentions,, but let's move on and\n> polish the tree shape at the tip of this topic.\n\nI would be prepared to do that, but I am constantly reminded of the\nunfortunate way we handled `ps/stash-in-c`, where you thought it was way\ntoo early to move to `next`, and I am convinced that we simply were way\ntoo late to start cooking in `next`.\n\nSo I keep offering to do work so that you would be happier, but none of my\nsuggestions seem to work.\n\n> The history behind it may be messier than other segments of our history,\n> and future developers may have harder time learning the intention of the\n> topic when making changes on top, but this one was supposed to create a\n> bug-to-bug reimplementation of the scripted version.\n\nRight. But we moved right past that, and continued enhancing `git stash`,\n(like the `--quiet` thing) and were now stuck with the unfortunate\nsituation that we had to do it in both built-in and scripted version.\n\n> What matters more would be our future changes on top of this code, which\n> improves what we used to have as scripted Porcelain. They will\n> genuinely be novel efforts, need to be built in logical order and\n> explainable steps to help future developers. Compared to that, so the\n> history of our stumbling along the way to reach today's tip of the topic\n> has much lower value.\n> \n> Besides I think it is way too late for the current topic. We\n> established before the topic hit 'next' that reviewers' eyes all\n> lost freshness and patience to review another round of this series\n> adequately.\n> \n> We at least know that the ordering and organization of the iteration\n> we see in 'next' is crappy, because some reviewers did look at them.\n> The rewrite will see no reviews, if any, far fewer and shallower\n> reviews than the iteration we have; nobody would be able to say with\n> confidence that the rewritten series achieves its goal of leaving a\n> sensible history. Doing so just before it hits 'master' makes it a\n> sure thing.\n\nFine. But in that case, I would appreciate not being reminded of the\nmessiness. Not unless you let me do something about it. Don't put me\nbetween a rock and a hard place, please.\n\n> Let's just we all admit that we did a poor job when we decided to\n> push this topic to 'next' before it was ready, and learn the lesson\n> to avoid haste making waste for the future topics.\n\nQuite honestly, I am at a loss what you are suggesting here. The original\ncontributor (Paul) got unexpectedly busy with university, so he was not\nable to take care of any updates.\n\nI would have made those updates (I promised, after all), but at some stage\nit felt more logical to explain in add-on topics what breakages were\nintroduced by the built-in rewrite and fix them: squashing the fixes in\nwould have made it less obvious why certain changes had to be done that\nway (after all, I missed in the original dozens of reviews, pre-submission\nand post-submission, e.g. the ORIG_HEAD problems).\n\nBut you did not complain about me adding on top back then, so I do not\nunderstand why you complain about it now...\n\nI am more than willing to move on, but if we keep repeating how messy the\ncurrent state is and do not even come up with a way how we could handle\nthis better in the future, then I do not really feel that we *are* moving\non after all.\n\nCiao,\nDscho\n\n> Thanks.\n> \n"},{"id":"371161","messageId":"nycvar.QRO.7.76.6.1903111726050.41@tvgsbejvaqbjf.bet","threadId":"50682","inReplyTo":"xmqqh8c9kgy6.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/2] built-in stash: handle :(glob) pathspecs again","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-03-11T16:27:11Z","receivedAt":"2019-03-11T16:27:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 11 Mar 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> \n> > From: Johannes Schindelin \n> >\n> > When passing a list of pathspecs to, say, `git add`, we need to be\n> > careful to use the original form, not the parsed form of the pathspecs.\n> >\n> > This makes a difference e.g. when calling\n> >\n> > \tgit stash -- ':(glob)**/*.txt'\n> >\n> > where the original form includes the `:(glob)` prefix while the parsed\n> > form does not.\n> >\n> > However, in the built-in `git stash`, we passed the parsed (i.e.\n> > incorrect) form, and `git add` would fail with the error message:\n> >\n> > \tfatal: pathspec '**/*.txt' did not match any files\n> >\n> > at the stage where `git stash` drops the changes from the worktree, even\n> > if `refs/stash` has been actually updated successfully.\n> >\n> > This fixes https://github.com/git-for-windows/git/issues/2037\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > builtin/stash.c | 5 +++--\n> > t/t3905-stash-include-untracked.sh | 6 ++++++\n> > 2 files changed, 9 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/builtin/stash.c b/builtin/stash.c\n> > index 1bfa24030c..2f29d037c8 100644\n> > --- a/builtin/stash.c\n> > +++ b/builtin/stash.c\n> > @@ -830,7 +830,7 @@ static void add_pathspecs(struct argv_array *args,\n> > \tint i;\n> > \n> > \tfor (i = 0; i < ps.nr; i++)\n> > -\t\targv_array_push(args, ps.items[i].match);\n> > +\t\targv_array_push(args, ps.items[i].original);\n> > }\n> \n> Yup. I think Thomas and Peff were also looking at the vicinity of\n> this code wrt the pass-by-value-ness of ps parameter. Their fix\n> need to also come on top of this (or combined together).\n\nI agree. How can I help?\n\nCiao,\nDscho\n"},{"id":"371200","messageId":"20190311221929.GI31533@hank.intra.tgummerer.com","threadId":"50682","inReplyTo":"nycvar.QRO.7.76.6.1903111726050.41@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/2] built-in stash: handle :(glob) pathspecs again","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2019-03-11T22:19:29Z","receivedAt":"2019-03-11T22:19:34Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 03/11, Johannes Schindelin wrote:\n> Hi Junio,\n> \n> On Mon, 11 Mar 2019, Junio C Hamano wrote:\n> \n> > Yup. I think Thomas and Peff were also looking at the vicinity of\n> > this code wrt the pass-by-value-ness of ps parameter. Their fix\n> > need to also come on top of this (or combined together).\n> \n> I agree. How can I help?\n\nI just sent an updated version of my patch, based on top of this at [*1*].\nWould be great if you could review that :)\n\n*1*: https://public-inbox.org/git/20190311221624.GC16414@hank.intra.tgummerer.com/\n"},{"id":"371770","messageId":"xmqq8sxc6bjt.fsf@gitster-ct.c.googlers.com","threadId":"50682","inReplyTo":"nycvar.QRO.7.76.6.1903111714430.41@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 0/2] stash: handle pathspec magic again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-18T04:39:34Z","receivedAt":"2019-03-18T04:39:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> To appease you enough that you stop complaining about the current, or\n> previous, state of `ps/stash-in-c`.\n> ...\n\nFirst of all, you do not have to appease me. What happened in the\npast has happened already, and whether I complain or not, the fact\nthat the history we came up with before pushing the topic to 'next'\nwas suboptimal. Nothing short of kicking it out of 'next' and\nredoing as if it were a fresh topic would fix that, but we all\nagreed that it is not the best way to spend our developer and\nreviewer resources.\n\n> Fine. But in that case, I would appreciate not being reminded of the\n> messiness. Not unless you let me do something about it. Don't put me\n> between a rock and a hard place, please.\n\nYou had been given plenty of chance to do something about it after\nyou added \"oh, it was wrong not to have a legacy fallback, and here\nis a patch on the top\". This is not the time to revisit the issue.\n\nGagging me won't change the fact that the history we ended up is\nmessy. Without getting reminded of our past mistake(s) ourselves,\nwhat else encourages us to do better the next time?\n\nThe lesson I personally learned is that yielding to the wish to\nhastily push things that are not ready to 'next' will leave us mess.\nI hope the lesson submitters and mentors have learned is not that by\nbombarding reviewers with too many iterations that do not address an\nissue, a topic can be pushed through with the issue unresolved with\nreviewer fatigue.\n"},{"id":"371787","messageId":"xmqqlg1c1x7v.fsf@gitster-ct.c.googlers.com","threadId":"50682","inReplyTo":"xmqq8sxc6bjt.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/2] stash: handle pathspec magic again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-18T07:02:44Z","receivedAt":"2019-03-18T07:02:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Johannes Schindelin writes:\n>\n>> Fine. But in that case, I would appreciate not being reminded of the\n>> messiness. Not unless you let me do something about it. Don't put me\n>> between a rock and a hard place, please.\n\nPerhaps your attitude is coming from a fundamental misunderstanding\nof the process. Don't be unnecessarily defensive, as nobody is\nbeing hostile or attacking you.\n\nIt's not like this is a competition to see if your taste as a\ndeveloper is better or worse than others (including me). It is a\ncooperative process and everybody involved is \"at fault\" if we made\na mistake. Ending up with a suboptimal history in 'next' is not\ncreditable solely to you. I am as much to blame, so are others who\nadvocated to merge it to 'next' even it were not ready. Or those\nwho did not speak up before it was too late, for that matter.\n\n> Gagging me won't change the fact that the history we ended up is\n> messy. Without getting reminded of our past mistake(s) ourselves,\n> what else encourages us to do better the next time?\n"}]}