{"thread":{"id":"47968","subject":"git stash push -u always warns \"pathspec '...' did not match any files\"","startedAt":"2018-03-03T10:01:13Z","lastAt":"2018-03-21T22:07:43Z","messageCount":33,"participants":["Marc Strapetz","Thomas Gummerer","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"340887","messageId":"349f9369-b799-4f7b-bda1-33bcbd7ea067@syntevo.com","threadId":"47968","inReplyTo":null,"subject":"git stash push -u always warns \"pathspec '...' did not match any files\"","fromName":"Marc Strapetz","fromEmail":"marc.strapetz@syntevo.com","sentAt":"2018-03-03T09:44:14Z","receivedAt":"2018-03-03T10:01:13Z","isPatch":false,"sender":{"key":"marc.strapetz@syntevo.com","avatar":"https://avatars.githubusercontent.com/u/3380730?v=4"},"body":"Reproducible in a test repository with following steps:\n\n$ touch untracked\n$ git stash push -u -- untracked\nSaved working directory and index state WIP on master: 0096475 init\nfatal: pathspec 'untracked' did not match any files\nerror: unrecognized input\n\nThe file is stashed correctly, though.\n\nTested with Git 2.16.2 on Linux and Windows.\n\n-Marc\n\n\t\n"},{"id":"340956","messageId":"20180303154654.GH2130@hank","threadId":"47968","inReplyTo":"349f9369-b799-4f7b-bda1-33bcbd7ea067@syntevo.com","subject":"Re: git stash push -u always warns \"pathspec '...' did not match any files\"","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-03-03T15:46:54Z","receivedAt":"2018-03-03T15:46:26Z","isPatch":false,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 03/03, Marc Strapetz wrote:\n> Reproducible in a test repository with following steps:\n> \n> $ touch untracked\n> $ git stash push -u -- untracked\n> Saved working directory and index state WIP on master: 0096475 init\n> fatal: pathspec 'untracked' did not match any files\n> error: unrecognized input\n> \n> The file is stashed correctly, though.\n> \n> Tested with Git 2.16.2 on Linux and Windows.\n\nThanks for the bug report and the reproduction recipe. The following\npatch should fix it:\n\n--- >8 ---\nSubject: [PATCH] stash push: avoid printing errors\n\nCurrently 'git stash push -u -- ' prints the following errors\nif only matches untracked files:\n\n fatal: pathspec 'untracked' did not match any files\n error: unrecognized input\n\nThis is because we first clean up the untracked files using 'git clean\n', and then use a command chain involving 'git add -u\n' and 'git apply' to clear the changes to files that are in\nthe index and were stashed.\n\nAs the only includes untracked files that were already\nremoved by 'git clean', the 'git add' call will barf, and so will 'git\napply', as there are no changes that need to be applied.\n\nFix this by making sure to only call this command chain if there are\nstill files that match after the call to 'git clean'.\n\nReported-by: Marc Strapetz \nSigned-off-by: Thomas Gummerer \n---\n git-stash.sh | 2 +-\n t/t3903-stash.sh | 7 +++++++\n 2 files changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex fc8f8ae640..058ad0bed8 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -320,7 +320,7 @@ push_stash () {\n \t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION -- \"$@\"\n \t\tfi\n \n-\t\tif test $# != 0\n+\t\tif test $# != 0 && git ls-files --error-unmatch -- \"$@\" >/dev/null 2>/dev/null\n \t\tthen\n \t\t\tgit add -u -- \"$@\" |\n \t\t\tgit checkout-index -z --force --stdin\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex aefde7b172..506004aece 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1096,4 +1096,11 @@ test_expect_success 'stash -- works with binary files' '\n \ttest_path_is_file subdir/untracked\n '\n \n+test_expect_success 'stash -- doesnt print error' '\n+\t>untracked &&\n+\tgit stash push -u -- untracked 2>actual&&\n+\ttest_path_is_missing untracked &&\n+\ttest_line_count = 0 actual\n+'\n+\n test_done\n-- \n2.16.2.395.g2e18187dfd\n\n"},{"id":"340978","messageId":"08d765ec-b2d7-7087-fca5-2eecefcef9cf@syntevo.com","threadId":"47968","inReplyTo":"20180303154654.GH2130@hank","subject":"Re: git stash push -u always warns \"pathspec '...' did not match any files\"","fromName":"Marc Strapetz","fromEmail":"marc.strapetz@syntevo.com","sentAt":"2018-03-04T10:44:30Z","receivedAt":"2018-03-04T10:44:37Z","isPatch":false,"sender":{"key":"marc.strapetz@syntevo.com","avatar":"https://avatars.githubusercontent.com/u/3380730?v=4"},"body":"On 03.03.2018 16:46, Thomas Gummerer wrote:\n> On 03/03, Marc Strapetz wrote:\n>> Reproducible in a test repository with following steps:\n>>\n>> $ touch untracked\n>> $ git stash push -u -- untracked\n>> Saved working directory and index state WIP on master: 0096475 init\n>> fatal: pathspec 'untracked' did not match any files\n>> error: unrecognized input\n>>\n>> The file is stashed correctly, though.\n>>\n>> Tested with Git 2.16.2 on Linux and Windows.\n> \n> Thanks for the bug report and the reproduction recipe. The following\n> patch should fix it:\n\nThanks, I can confirm that the misleading warning message is fixed.\n\nWhat I've noticed now is that when using -u option, Git won't warn if \nthe pathspec is actually not matching a file. Also, an empty stash may \nbe created. For example:\n\n$ git stash push -u -- nonexisting\nSaved working directory and index state WIP on master: 171081d initial \nimport\n\nI would probably expect to see an error message as for:\n\n$ git stash push -- nonexisting\nerror: pathspec 'nonexisting' did not match any file(s) known to git.\nDid you forget to 'git add'?\n\nThat said, this is no problem for us, because I know that the paths I'm \nproviding to \"git stash push\" do exist. I just wanted to point out.\n\n-Marc\n\n"},{"id":"341374","messageId":"xmqqwoyk51w4.fsf@gitster-ct.c.googlers.com","threadId":"47968","inReplyTo":"08d765ec-b2d7-7087-fca5-2eecefcef9cf@syntevo.com","subject":"Re: git stash push -u always warns \"pathspec '...' did not match any files\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-09T22:18:19Z","receivedAt":"2018-03-09T22:18:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Marc Strapetz writes:\n\n> Thanks, I can confirm that the misleading warning message is fixed.\n>\n> What I've noticed now is that when using -u option, Git won't warn if\n> the pathspec is actually not matching a file. Also, an empty stash may\n> be created.\n\nSoooo..., does it mean that the patch Thomas posted and you\nconfirmed trades one issue with another issue with a similar\ngraveness? Is this something we want to \"fix\" without adding\nanother breakage?\n\n"},{"id":"341382","messageId":"d49b1b73-3d16-f29d-4363-a5dba4853842@syntevo.com","threadId":"47968","inReplyTo":"xmqqwoyk51w4.fsf@gitster-ct.c.googlers.com","subject":"Re: git stash push -u always warns \"pathspec '...' did not match any files\"","fromName":"Marc Strapetz","fromEmail":"marc.strapetz@syntevo.com","sentAt":"2018-03-10T09:18:13Z","receivedAt":"2018-03-10T09:37:57Z","isPatch":false,"sender":{"key":"marc.strapetz@syntevo.com","avatar":"https://avatars.githubusercontent.com/u/3380730?v=4"},"body":"On 09.03.2018 23:18, Junio C Hamano wrote:\n> Marc Strapetz writes:\n> \n>> Thanks, I can confirm that the misleading warning message is fixed.\n>>\n>> What I've noticed now is that when using -u option, Git won't warn if\n>> the pathspec is actually not matching a file. Also, an empty stash may\n>> be created.\n> \n> Soooo..., does it mean that the patch Thomas posted and you\n> confirmed trades one issue with another issue with a similar\n> graveness?\n\n From my understanding these are two separate problems for which the new \none was somewhat hidden by the one Thomas has fixed: Thomas has fixed \npost-processing code after the stash has already been saved away. The \nproblem I'm referring to is a missing check for invalid paths before the \nstash is saved away.\n\n-Marc\n"},{"id":"341386","messageId":"20180310111215.GA14732@hank","threadId":"47968","inReplyTo":"d49b1b73-3d16-f29d-4363-a5dba4853842@syntevo.com","subject":"Re: git stash push -u always warns \"pathspec '...' did not match any files\"","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-03-10T11:12:15Z","receivedAt":"2018-03-10T11:09:10Z","isPatch":false,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 03/10, Marc Strapetz wrote:\n> On 09.03.2018 23:18, Junio C Hamano wrote:\n> >Marc Strapetz writes:\n> >\n> >>Thanks, I can confirm that the misleading warning message is fixed.\n> >>\n> >>What I've noticed now is that when using -u option, Git won't warn if\n> >>the pathspec is actually not matching a file. Also, an empty stash may\n> >>be created.\n> >\n> >Soooo..., does it mean that the patch Thomas posted and you\n> >confirmed trades one issue with another issue with a similar\n> >graveness?\n\nI've been meaning to follow up on this, but haven't found the time to\ndo so yet, sorry.\n\n> From my understanding these are two separate problems for which the new one\n> was somewhat hidden by the one Thomas has fixed: Thomas has fixed\n> post-processing code after the stash has already been saved away. The\n> problem I'm referring to is a missing check for invalid paths before the\n> stash is saved away.\n\nYeah, just to demonstrate what the new problem Marc describes is,\ncurrently 'git stash push -u ' would produce the following\noutput, and create a new stash:\n\n $ git stash push -u unknown\n Saved working directory and index state WIP on master: 7e31236f65 Sixth batch for 2.17\n fatal: pathspec 'unknown' did not match any files\n error: unrecognized input\n $\n\nWith the patch I posted it would just print \n\n $ git stash push -u unknown\n Saved working directory and index state WIP on master: 7e31236f65 Sixth batch for 2.17\n $\n\nand produce a new stash as before. Both of those end up confusing to\nthe user, dunno which one is better. What really should happen is\n\n $ git stash push -u unknown\n No local changes to save\n $\n\nand not creating a stash. So these were many words to basically say\nthat I think my patch is still the right thing to do, but it may or\nmay not confuse the user more if they are hitting the other bug Marc\nnoted. Either way I'll try to address this as soon as I can get some\ntime to look at it.\n\n> -Marc\n"},{"id":"341716","messageId":"20180314214642.22185-1-t.gummerer@gmail.com","threadId":"47968","inReplyTo":"20180310111215.GA14732@hank","subject":"[PATCH v2 1/2] stash push: avoid printing errors","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-03-14T21:46:41Z","receivedAt":"2018-03-14T21:43:40Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Currently 'git stash push -u -- ' prints the following errors\nif only matches untracked files:\n\n fatal: pathspec 'untracked' did not match any files\n error: unrecognized input\n\nThis is because we first clean up the untracked files using 'git clean\n', and then use a command chain involving 'git add -u\n' and 'git apply' to clear the changes to files that are in\nthe index and were stashed.\n\nAs the only includes untracked files that were already\nremoved by 'git clean', the 'git add' call will barf, and so will 'git\napply', as there are no changes that need to be applied.\n\nFix this by making sure to only call this command chain if there are\nstill files that match after the call to 'git clean'.\n\nReported-by: Marc Strapetz \nSigned-off-by: Thomas Gummerer \n---\n\n> Either way I'll try to address this as soon as I can get some\n> time to look at it.\n\nI finally got around to do this. The fix (in the second patch) turns\nout to be fairly simple, I just forgot to pass the pathspec along to\none function whene originally introducing the pathspec feature in git\nstash push (more explanation in the commit message for the patch\nitself). Thanks Marc for reporting the two breakages!\n\nv2 also fixes a couple of typos in the first patch which I failed to\nnotice when I sent it out last time.\n\n git-stash.sh | 2 +-\n t/t3903-stash.sh | 7 +++++++\n 2 files changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex fc8f8ae640..058ad0bed8 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -320,7 +320,7 @@ push_stash () {\n \t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION -- \"$@\"\n \t\tfi\n \n-\t\tif test $# != 0\n+\t\tif test $# != 0 && git ls-files --error-unmatch -- \"$@\" >/dev/null 2>/dev/null\n \t\tthen\n \t\t\tgit add -u -- \"$@\" |\n \t\t\tgit checkout-index -z --force --stdin\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex aefde7b172..fbfda4b243 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1096,4 +1096,11 @@ test_expect_success 'stash -- works with binary files' '\n \ttest_path_is_file subdir/untracked\n '\n \n+test_expect_success 'stash -u -- doesnt print error' '\n+\t>untracked &&\n+\tgit stash push -u -- untracked 2>actual &&\n+\ttest_path_is_missing untracked &&\n+\ttest_line_count = 0 actual\n+'\n+\n test_done\n-- \n2.16.2.804.g6dcf76e11\n\n"},{"id":"341717","messageId":"20180314214642.22185-2-t.gummerer@gmail.com","threadId":"47968","inReplyTo":"20180314214642.22185-1-t.gummerer@gmail.com","subject":"[PATCH v2 2/2] stash push -u: don't create empty stash","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-03-14T21:46:42Z","receivedAt":"2018-03-14T21:43:45Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"When introducing the stash push feature, and thus allowing users to pass\nin a pathspec to limit the files that would get stashed in\ndf6bba0937 (\"stash: teach 'push' (and 'create_stash') to honor\npathspec\", 2017-02-28), this developer missed one place where the\npathspec should be passed in.\n\nNamely in the call to the 'untracked_files()' function in the\n'no_changes()' function. This resulted in 'git stash push -u --\n' creating an empty stash when there are untracked files\nin the repository other that don't match the pathspec.\n\nAs 'git stash' never creates empty stashes, this behaviour is wrong and\nconfusing for users. Instead it should just show a message \"No local\nchanges to save\", and not create a stash.\n\nLuckily the 'untracked_files()' function already correctly respects\npathspecs that are passed to it, so the fix is simply to pass the\npathspec along to the function.\n\nReported-by: Marc Strapetz \nSigned-off-by: Thomas Gummerer \n---\n git-stash.sh | 2 +-\n t/t3903-stash.sh | 6 ++++++\n 2 files changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 058ad0bed8..7a4ec98f6b 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -39,7 +39,7 @@ fi\n no_changes () {\n \tgit diff-index --quiet --cached HEAD --ignore-submodules -- \"$@\" &&\n \tgit diff-files --quiet --ignore-submodules -- \"$@\" &&\n-\t(test -z \"$untracked\" || test -z \"$(untracked_files)\")\n+\t(test -z \"$untracked\" || test -z \"$(untracked_files $@)\")\n }\n \n untracked_files () {\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex fbfda4b243..5e7078c083 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1103,4 +1103,10 @@ test_expect_success 'stash -u -- doesnt print error' '\n \ttest_line_count = 0 actual\n '\n \n+test_expect_success 'stash -u -- shows no changes when there are none' '\n+\tgit stash push -u -- non-existant >actual &&\n+\techo \"No local changes to save\" >expect &&\n+\ttest_i18ncmp expect actual\n+'\n+\n test_done\n-- \n2.16.2.804.g6dcf76e11\n\n"},{"id":"341738","messageId":"d9588594-10fd-1080-abda-aeeb12948b2c@syntevo.com","threadId":"47968","inReplyTo":"20180314214642.22185-1-t.gummerer@gmail.com","subject":"Re: [PATCH v2 1/2] stash push: avoid printing errors","fromName":"Marc Strapetz","fromEmail":"marc.strapetz@syntevo.com","sentAt":"2018-03-15T08:51:48Z","receivedAt":"2018-03-15T08:52:08Z","isPatch":true,"sender":{"key":"marc.strapetz@syntevo.com","avatar":"https://avatars.githubusercontent.com/u/3380730?v=4"},"body":"On 14.03.2018 22:46, Thomas Gummerer wrote:\n> Currently 'git stash push -u -- ' prints the following errors\n> if only matches untracked files:\n> \n> fatal: pathspec 'untracked' did not match any files\n> error: unrecognized input\n> \n> This is because we first clean up the untracked files using 'git clean\n> ', and then use a command chain involving 'git add -u\n> ' and 'git apply' to clear the changes to files that are in\n> the index and were stashed.\n> \n> As the only includes untracked files that were already\n> removed by 'git clean', the 'git add' call will barf, and so will 'git\n> apply', as there are no changes that need to be applied.\n> \n> Fix this by making sure to only call this command chain if there are\n> still files that match after the call to 'git clean'.\n> \n> Reported-by: Marc Strapetz \n> Signed-off-by: Thomas Gummerer \n> ---\n> \n>> Either way I'll try to address this as soon as I can get some\n>> time to look at it.\n> \n> I finally got around to do this. The fix (in the second patch) turns\n> out to be fairly simple, I just forgot to pass the pathspec along to\n> one function whene originally introducing the pathspec feature in git\n> stash push (more explanation in the commit message for the patch\n> itself). Thanks Marc for reporting the two breakages!\n\nThanks, I confirm that both issues are resolved. There is another issue \nnow which seems to be a regression. When *successfully* stashing an \nuntracked file, local modifications of other files are cleared, too.\n\n$ git init\n$ touch file1\n$ git add file1\n$ git commit -m \"initial import\"\n$ echo \"a\" > file1\n$ touch file2\n$ git status --porcelain\n M file1\n?? file2\n$ git stash push -u -- file2\nSaved working directory and index state WIP on master: 25352d7 initial \nimport\n$ git status\nOn branch master\nnothing to commit, working tree clean\n\nHence, by stashing just \"file2\" the local modification of \"file1\" became \nreset.\n\n-Marc\n\n\n\n\n"},{"id":"341826","messageId":"xmqqtvth3xt5.fsf@gitster-ct.c.googlers.com","threadId":"47968","inReplyTo":"20180314214642.22185-2-t.gummerer@gmail.com","subject":"Re: [PATCH v2 2/2] stash push -u: don't create empty stash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-15T20:09:58Z","receivedAt":"2018-03-15T20:10:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer writes:\n\n> no_changes () {\n> \tgit diff-index --quiet --cached HEAD --ignore-submodules -- \"$@\" &&\n> \tgit diff-files --quiet --ignore-submodules -- \"$@\" &&\n> -\t(test -z \"$untracked\" || test -z \"$(untracked_files)\")\n> +\t(test -z \"$untracked\" || test -z \"$(untracked_files $@)\")\n\nDon't you need a pair of double-quotes around that $@?\n"},{"id":"341921","messageId":"20180316201034.GA2224@hank","threadId":"47968","inReplyTo":"xmqqtvth3xt5.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 2/2] stash push -u: don't create empty stash","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-03-16T20:10:34Z","receivedAt":"2018-03-16T20:08:40Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 03/15, Junio C Hamano wrote:\n> Thomas Gummerer writes:\n> \n> > no_changes () {\n> > \tgit diff-index --quiet --cached HEAD --ignore-submodules -- \"$@\" &&\n> > \tgit diff-files --quiet --ignore-submodules -- \"$@\" &&\n> > -\t(test -z \"$untracked\" || test -z \"$(untracked_files)\")\n> > +\t(test -z \"$untracked\" || test -z \"$(untracked_files $@)\")\n> \n> Don't you need a pair of double-quotes around that $@?\n\nAh yes indeed. Will fix, thanks!\n"},{"id":"341922","messageId":"20180316201254.GB2224@hank","threadId":"47968","inReplyTo":"d9588594-10fd-1080-abda-aeeb12948b2c@syntevo.com","subject":"Re: [PATCH v2 1/2] stash push: avoid printing errors","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-03-16T20:12:54Z","receivedAt":"2018-03-16T20:09:46Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 03/15, Marc Strapetz wrote:\n> On 14.03.2018 22:46, Thomas Gummerer wrote:\n> >Currently 'git stash push -u -- ' prints the following errors\n> >if only matches untracked files:\n> >\n> > fatal: pathspec 'untracked' did not match any files\n> > error: unrecognized input\n> >\n> >This is because we first clean up the untracked files using 'git clean\n> >', and then use a command chain involving 'git add -u\n> >' and 'git apply' to clear the changes to files that are in\n> >the index and were stashed.\n> >\n> >As the only includes untracked files that were already\n> >removed by 'git clean', the 'git add' call will barf, and so will 'git\n> >apply', as there are no changes that need to be applied.\n> >\n> >Fix this by making sure to only call this command chain if there are\n> >still files that match after the call to 'git clean'.\n> >\n> >Reported-by: Marc Strapetz \n> >Signed-off-by: Thomas Gummerer \n> >---\n> >\n> >>Either way I'll try to address this as soon as I can get some\n> >>time to look at it.\n> >\n> >I finally got around to do this. The fix (in the second patch) turns\n> >out to be fairly simple, I just forgot to pass the pathspec along to\n> >one function whene originally introducing the pathspec feature in git\n> >stash push (more explanation in the commit message for the patch\n> >itself). Thanks Marc for reporting the two breakages!\n> \n> Thanks, I confirm that both issues are resolved. There is another issue now\n> which seems to be a regression. When *successfully* stashing an untracked\n> file, local modifications of other files are cleared, too.\n> \n> $ git init\n> $ touch file1\n> $ git add file1\n> $ git commit -m \"initial import\"\n> $ echo \"a\" > file1\n> $ touch file2\n> $ git status --porcelain\n> M file1\n> ?? file2\n> $ git stash push -u -- file2\n> Saved working directory and index state WIP on master: 25352d7 initial\n> import\n> $ git status\n> On branch master\n> nothing to commit, working tree clean\n> \n> Hence, by stashing just \"file2\" the local modification of \"file1\" became\n> reset.\n\nAh yes, this is indeed a regression, thanks for catching it. I'll fix\nit in the re-roll and add another test for this case. Sorry about the\nfalse starts here :/\n\n> -Marc\n> \n> \n> \n> \n"},{"id":"341930","messageId":"20180316204306.862-1-t.gummerer@gmail.com","threadId":"47968","inReplyTo":"20180314214642.22185-1-t.gummerer@gmail.com","subject":"[PATCH v3 0/2] stash push -u -- fixes","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-03-16T20:43:04Z","receivedAt":"2018-03-16T20:40:04Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Thanks Marc for catching the regression I almost introduced and Junio\nfor the review of the second patch. Here's a re-roll that should fix\nthe issues of v2.\n\nInterdiff below:\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 7a4ec98f6b..dbedc7fb9f 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -39,7 +39,7 @@ fi\n no_changes () {\n \tgit diff-index --quiet --cached HEAD --ignore-submodules -- \"$@\" &&\n \tgit diff-files --quiet --ignore-submodules -- \"$@\" &&\n-\t(test -z \"$untracked\" || test -z \"$(untracked_files $@)\")\n+\t(test -z \"$untracked\" || test -z \"$(untracked_files \"$@\")\")\n }\n \n untracked_files () {\n@@ -320,11 +320,14 @@ push_stash () {\n \t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION -- \"$@\"\n \t\tfi\n \n-\t\tif test $# != 0 && git ls-files --error-unmatch -- \"$@\" >/dev/null 2>/dev/null\n+\t\tif test $# != 0\n \t\tthen\n-\t\t\tgit add -u -- \"$@\" |\n-\t\t\tgit checkout-index -z --force --stdin\n-\t\t\tgit diff-index -p --cached --binary HEAD -- \"$@\" | git apply --index -R\n+\t\t\tif git ls-files --error-unmatch -- \"$@\" >/dev/null 2>/dev/null\n+\t\t\tthen\n+\t\t\t\tgit add -u -- \"$@\" |\n+\t\t\t\tgit checkout-index -z --force --stdin\n+\t\t\t\tgit diff-index -p --cached --binary HEAD -- \"$@\" | git apply --index -R\n+\t\t\tfi\n \t\telse\n \t\t\tgit reset --hard -q\n \t\tfi\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 5e7078c083..7efc52fe11 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1103,6 +1103,15 @@ test_expect_success 'stash -u -- doesnt print error' '\n \ttest_line_count = 0 actual\n '\n \n+test_expect_success 'stash -u -- leaves rest of working tree in place' '\n+\t>tracked &&\n+\tgit add tracked &&\n+\t>untracked &&\n+\tgit stash push -u -- untracked &&\n+\ttest_path_is_missing untracked &&\n+\ttest_path_is_file tracked\n+'\n+\n test_expect_success 'stash -u -- shows no changes when there are none' '\n \tgit stash push -u -- non-existant >actual &&\n \techo \"No local changes to save\" >expect &&\n\nThomas Gummerer (2):\n stash push: avoid printing errors\n stash push -u: don't create empty stash\n\n git-stash.sh | 11 +++++++----\n t/t3903-stash.sh | 22 ++++++++++++++++++++++\n 2 files changed, 29 insertions(+), 4 deletions(-)\n\n-- \n2.16.2.804.g6dcf76e11\n"},{"id":"341931","messageId":"20180316204306.862-3-t.gummerer@gmail.com","threadId":"47968","inReplyTo":"20180316204306.862-1-t.gummerer@gmail.com","subject":"[PATCH v3 2/2] stash push -u: don't create empty stash","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-03-16T20:43:06Z","receivedAt":"2018-03-16T20:41:44Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"When introducing the stash push feature, and thus allowing users to pass\nin a pathspec to limit the files that would get stashed in\ndf6bba0937 (\"stash: teach 'push' (and 'create_stash') to honor\npathspec\", 2017-02-28), this developer missed one place where the\npathspec should be passed in.\n\nNamely in the call to the 'untracked_files()' function in the\n'no_changes()' function. This resulted in 'git stash push -u --\n' creating an empty stash when there are untracked files\nin the repository other that don't match the pathspec.\n\nAs 'git stash' never creates empty stashes, this behaviour is wrong and\nconfusing for users. Instead it should just show a message \"No local\nchanges to save\", and not create a stash.\n\nLuckily the 'untracked_files()' function already correctly respects\npathspecs that are passed to it, so the fix is simply to pass the\npathspec along to the function.\n\nReported-by: Marc Strapetz \nSigned-off-by: Thomas Gummerer \n---\n git-stash.sh | 2 +-\n t/t3903-stash.sh | 6 ++++++\n 2 files changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 4de9f9bea8..dbedc7fb9f 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -39,7 +39,7 @@ fi\n no_changes () {\n \tgit diff-index --quiet --cached HEAD --ignore-submodules -- \"$@\" &&\n \tgit diff-files --quiet --ignore-submodules -- \"$@\" &&\n-\t(test -z \"$untracked\" || test -z \"$(untracked_files)\")\n+\t(test -z \"$untracked\" || test -z \"$(untracked_files \"$@\")\")\n }\n \n untracked_files () {\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 8b1a8d2982..7efc52fe11 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1112,4 +1112,10 @@ test_expect_success 'stash -u -- leaves rest of working tree in plac\n \ttest_path_is_file tracked\n '\n \n+test_expect_success 'stash -u -- shows no changes when there are none' '\n+\tgit stash push -u -- non-existant >actual &&\n+\techo \"No local changes to save\" >expect &&\n+\ttest_i18ncmp expect actual\n+'\n+\n test_done\n-- \n2.16.2.804.g6dcf76e11\n\n"},{"id":"341932","messageId":"20180316204306.862-2-t.gummerer@gmail.com","threadId":"47968","inReplyTo":"20180316204306.862-1-t.gummerer@gmail.com","subject":"[PATCH v3 1/2] stash push: avoid printing errors","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-03-16T20:43:05Z","receivedAt":"2018-03-16T20:42:04Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Currently 'git stash push -u -- ' prints the following errors\nif only matches untracked files:\n\n fatal: pathspec 'untracked' did not match any files\n error: unrecognized input\n\nThis is because we first clean up the untracked files using 'git clean\n', and then use a command chain involving 'git add -u\n' and 'git apply' to clear the changes to files that are in\nthe index and were stashed.\n\nAs the only includes untracked files that were already\nremoved by 'git clean', the 'git add' call will barf, and so will 'git\napply', as there are no changes that need to be applied.\n\nFix this by making sure to only call this command chain if there are\nstill files that match after the call to 'git clean'.\n\nReported-by: Marc Strapetz \nSigned-off-by: Thomas Gummerer \n---\n git-stash.sh | 9 ++++++---\n t/t3903-stash.sh | 16 ++++++++++++++++\n 2 files changed, 22 insertions(+), 3 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex fc8f8ae640..4de9f9bea8 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -322,9 +322,12 @@ push_stash () {\n \n \t\tif test $# != 0\n \t\tthen\n-\t\t\tgit add -u -- \"$@\" |\n-\t\t\tgit checkout-index -z --force --stdin\n-\t\t\tgit diff-index -p --cached --binary HEAD -- \"$@\" | git apply --index -R\n+\t\t\tif git ls-files --error-unmatch -- \"$@\" >/dev/null 2>/dev/null\n+\t\t\tthen\n+\t\t\t\tgit add -u -- \"$@\" |\n+\t\t\t\tgit checkout-index -z --force --stdin\n+\t\t\t\tgit diff-index -p --cached --binary HEAD -- \"$@\" | git apply --index -R\n+\t\t\tfi\n \t\telse\n \t\t\tgit reset --hard -q\n \t\tfi\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex aefde7b172..8b1a8d2982 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1096,4 +1096,20 @@ test_expect_success 'stash -- works with binary files' '\n \ttest_path_is_file subdir/untracked\n '\n \n+test_expect_success 'stash -u -- doesnt print error' '\n+\t>untracked &&\n+\tgit stash push -u -- untracked 2>actual &&\n+\ttest_path_is_missing untracked &&\n+\ttest_line_count = 0 actual\n+'\n+\n+test_expect_success 'stash -u -- leaves rest of working tree in place' '\n+\t>tracked &&\n+\tgit add tracked &&\n+\t>untracked &&\n+\tgit stash push -u -- untracked &&\n+\ttest_path_is_missing untracked &&\n+\ttest_path_is_file tracked\n+'\n+\n test_done\n-- \n2.16.2.804.g6dcf76e11\n\n"},{"id":"341949","messageId":"xmqqtvtfzp0g.fsf@gitster-ct.c.googlers.com","threadId":"47968","inReplyTo":"20180316204306.862-2-t.gummerer@gmail.com","subject":"Re: [PATCH v3 1/2] stash push: avoid printing errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-16T21:31:11Z","receivedAt":"2018-03-16T21:31:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer writes:\n\n> @@ -322,9 +322,12 @@ push_stash () {\n> \n> \t\tif test $# != 0\n> \t\tthen\n> -\t\t\tgit add -u -- \"$@\" |\n> -\t\t\tgit checkout-index -z --force --stdin\n\nThis obviously is not something this patch breaks, but I am finding\nthis pipeline that was already here quite puzzling.\n\nThe \"add -u\" is about adding the changes in paths that match the\npathspec to the index; the output from it is meant for human\nconsumption and certainly is not something \"--stdin\" should expect\nto be understandable, let alone with \"-z\".\n\n\t... goes and digs ...\n\nI think you mis-copied the suggestion in\n\n https://public-inbox.org/git/xmqqpo7byjwb.fsf@gitster.mtv.corp.google.com/\n\nwhen you made bba067d2 (\"stash: don't delete untracked files that\nmatch pathspec\", 2018-01-06), and nobody caught that breakage during\nthe review.\n\n> -\t\t\tgit diff-index -p --cached --binary HEAD -- \"$@\" | git apply --index -R\n> +\t\t\tif git ls-files --error-unmatch -- \"$@\" >/dev/null 2>/dev/null\n> +\t\t\tthen\n> +\t\t\t\tgit add -u -- \"$@\" |\n> +\t\t\t\tgit checkout-index -z --force --stdin\n\nAnd the same breakage is inherited here; just drop \"|\" and\ndownstream \"checkout-index\" and you should be OK.\n\n> +\t\t\t\tgit diff-index -p --cached --binary HEAD -- \"$@\" | git apply --index -R\n\nAnd while at it, let's split this to two lines after \"|\".\n\n> +\t\t\tfi\n\n"},{"id":"341960","messageId":"20180316223715.108044-1-gitster@pobox.com","threadId":"47968","inReplyTo":"20180316204306.862-1-t.gummerer@gmail.com","subject":"[PATCH v4 0/3] stash push -u -- fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-16T22:37:12Z","receivedAt":"2018-03-16T22:37:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here is a preliminary fix for an earlier copy-pasto, plus two\npatches from your v3.\n\nI tried to reduce the nesting level by swapping the order of if/elif\nchain; please double check the logic to ensure I didn't make stupid\nmistakes while doing so.\n\nJunio C Hamano (1):\n stash: fix nonsense pipeline\n\nThomas Gummerer (2):\n stash push: avoid printing errors\n stash push -u: don't create empty stash\n\n git-stash.sh | 13 +++++++------\n t/t3903-stash.sh | 22 ++++++++++++++++++++++\n 2 files changed, 29 insertions(+), 6 deletions(-)\n\n-- \n2.17.0-rc0\n\n"},{"id":"341961","messageId":"20180316223715.108044-3-gitster@pobox.com","threadId":"47968","inReplyTo":"20180316223715.108044-1-gitster@pobox.com","subject":"[PATCH v4 2/3] stash push: avoid printing errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-16T22:37:14Z","receivedAt":"2018-03-16T22:37:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Thomas Gummerer \n\n'git stash push -u -- ' prints the following errors if\n only matches untracked files:\n\n fatal: pathspec 'untracked' did not match any files\n error: unrecognized input\n\nThis is because we first clean up the untracked files using 'git clean\n', and then use a command chain involving 'git add -u\n' and 'git apply' to clear the changes to files that are in\nthe index and were stashed.\n\nAs the only includes untracked files that were already\nremoved by 'git clean', the 'git add' call will barf, and so will 'git\napply', as there are no changes that need to be applied.\n\nFix this by making sure to only call this command chain if there are\nstill files that match after the call to 'git clean'.\n\n[jc: swapped the order of if/elseif chain to reduce nesting levels]\n\nReported-by: Marc Strapetz \nSigned-off-by: Thomas Gummerer \nSigned-off-by: Junio C Hamano \n---\n git-stash.sh | 7 ++++---\n t/t3903-stash.sh | 16 ++++++++++++++++\n 2 files changed, 20 insertions(+), 3 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 4c92ec931f..fa61151548 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -313,13 +313,14 @@ push_stash () {\n \t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION -- \"$@\"\n \t\tfi\n \n-\t\tif test $# != 0\n+\t\tif test $# = 0\n+\t\tthen\n+\t\t\tgit reset --hard -q\n+\t\telif git ls-files --error-unmatch -- \"$@\" >/dev/null 2>&1\n \t\tthen\n \t\t\tgit add -u -- \"$@\"\n \t\t\tgit diff-index -p --cached --binary HEAD -- \"$@\" |\n \t\t\tgit apply --index -R\n-\t\telse\n-\t\t\tgit reset --hard -q\n \t\tfi\n \n \t\tif test \"$keep_index\" = \"t\" && test -n \"$i_tree\"\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 1cd85e70e9..f5b102a958 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1003,4 +1003,20 @@ test_expect_success 'stash -- works with binary files' '\n \ttest_path_is_file subdir/untracked\n '\n \n+test_expect_success 'stash -u -- doesnt print error' '\n+\t>untracked &&\n+\tgit stash push -u -- untracked 2>actual &&\n+\ttest_path_is_missing untracked &&\n+\ttest_line_count = 0 actual\n+'\n+\n+test_expect_success 'stash -u -- leaves rest of working tree in place' '\n+\t>tracked &&\n+\tgit add tracked &&\n+\t>untracked &&\n+\tgit stash push -u -- untracked &&\n+\ttest_path_is_missing untracked &&\n+\ttest_path_is_file tracked\n+'\n+\n test_done\n-- \n2.17.0-rc0\n\n"},{"id":"341962","messageId":"20180316223715.108044-2-gitster@pobox.com","threadId":"47968","inReplyTo":"20180316223715.108044-1-gitster@pobox.com","subject":"[PATCH v4 1/3] stash: fix nonsense pipeline","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-16T22:37:13Z","receivedAt":"2018-03-16T22:37:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"An earlier change bba067d2 (\"stash: don't delete untracked files\nthat match pathspec\", 2018-01-06) was made by taking a suggestion in\na list discussion [1] but did not copy the suggested snippet\ncorrectly. And the bug was unnoticed during the review and slipped\nthrough.\n\nThis fixes it.\n\n[1] https://public-inbox.org/git/xmqqpo7byjwb.fsf@gitster.mtv.corp.google.com/\n\nSigned-off-by: Junio C Hamano \n---\n git-stash.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex b48b164748..4c92ec931f 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -315,9 +315,9 @@ push_stash () {\n \n \t\tif test $# != 0\n \t\tthen\n-\t\t\tgit add -u -- \"$@\" |\n-\t\t\tgit checkout-index -z --force --stdin\n-\t\t\tgit diff-index -p --cached --binary HEAD -- \"$@\" | git apply --index -R\n+\t\t\tgit add -u -- \"$@\"\n+\t\t\tgit diff-index -p --cached --binary HEAD -- \"$@\" |\n+\t\t\tgit apply --index -R\n \t\telse\n \t\t\tgit reset --hard -q\n \t\tfi\n-- \n2.17.0-rc0\n\n"},{"id":"341963","messageId":"20180316223715.108044-4-gitster@pobox.com","threadId":"47968","inReplyTo":"20180316223715.108044-1-gitster@pobox.com","subject":"[PATCH v4 3/3] stash push -u: don't create empty stash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-16T22:37:15Z","receivedAt":"2018-03-16T22:37:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Thomas Gummerer \n\nWhen introducing the stash push feature, and thus allowing users to pass\nin a pathspec to limit the files that would get stashed in\ndf6bba0937 (\"stash: teach 'push' (and 'create_stash') to honor\npathspec\", 2017-02-28), this developer missed one place where the\npathspec should be passed in.\n\nNamely in the call to the 'untracked_files()' function in the\n'no_changes()' function. This resulted in 'git stash push -u --\n' creating an empty stash when there are untracked files\nin the repository other that don't match the pathspec.\n\nAs 'git stash' never creates empty stashes, this behaviour is wrong and\nconfusing for users. Instead it should just show a message \"No local\nchanges to save\", and not create a stash.\n\nLuckily the 'untracked_files()' function already correctly respects\npathspecs that are passed to it, so the fix is simply to pass the\npathspec along to the function.\n\nReported-by: Marc Strapetz \nSigned-off-by: Thomas Gummerer \nSigned-off-by: Junio C Hamano \n---\n git-stash.sh | 2 +-\n t/t3903-stash.sh | 6 ++++++\n 2 files changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex fa61151548..1652f64b5c 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -39,7 +39,7 @@ fi\n no_changes () {\n \tgit diff-index --quiet --cached HEAD --ignore-submodules -- \"$@\" &&\n \tgit diff-files --quiet --ignore-submodules -- \"$@\" &&\n-\t(test -z \"$untracked\" || test -z \"$(untracked_files)\")\n+\t(test -z \"$untracked\" || test -z \"$(untracked_files \"$@\")\")\n }\n \n untracked_files () {\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex f5b102a958..7e0b1c248c 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -1019,4 +1019,10 @@ test_expect_success 'stash -u -- leaves rest of working tree in plac\n \ttest_path_is_file tracked\n '\n \n+test_expect_success 'stash -u -- shows no changes when there are none' '\n+\tgit stash push -u -- non-existant >actual &&\n+\techo \"No local changes to save\" >expect &&\n+\ttest_i18ncmp expect actual\n+'\n+\n test_done\n-- \n2.17.0-rc0\n\n"},{"id":"342014","messageId":"20180317113612.GD2224@hank","threadId":"47968","inReplyTo":"20180316223715.108044-1-gitster@pobox.com","subject":"Re: [PATCH v4 0/3] stash push -u -- fixes","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-03-17T11:36:12Z","receivedAt":"2018-03-17T11:33:03Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 03/16, Junio C Hamano wrote:\n> Here is a preliminary fix for an earlier copy-pasto, plus two\n> patches from your v3.\n\nThanks for catching and fixing this!\n\n> I tried to reduce the nesting level by swapping the order of if/elif\n> chain; please double check the logic to ensure I didn't make stupid\n> mistakes while doing so.\n\nI looked over what you send, and the patches and the changes you made\nlook good to me.\n\n> Junio C Hamano (1):\n> stash: fix nonsense pipeline\n> \n> Thomas Gummerer (2):\n> stash push: avoid printing errors\n> stash push -u: don't create empty stash\n> \n> git-stash.sh | 13 +++++++------\n> t/t3903-stash.sh | 22 ++++++++++++++++++++++\n> 2 files changed, 29 insertions(+), 6 deletions(-)\n> \n> -- \n> 2.17.0-rc0\n> \n"},{"id":"342193","messageId":"bc69c1f4-7098-1bbf-e69b-97808c4d8779@syntevo.com","threadId":"47968","inReplyTo":"20180316204306.862-1-t.gummerer@gmail.com","subject":"Re: [PATCH v3 0/2] stash push -u -- fixes","fromName":"Marc Strapetz","fromEmail":"marc.strapetz@syntevo.com","sentAt":"2018-03-19T15:44:17Z","receivedAt":"2018-03-19T15:44:24Z","isPatch":true,"sender":{"key":"marc.strapetz@syntevo.com","avatar":"https://avatars.githubusercontent.com/u/3380730?v=4"},"body":"On 16.03.2018 21:43, Thomas Gummerer wrote:\n> Thanks Marc for catching the regression I almost introduced and Junio\n> for the review of the second patch. Here's a re-roll that should fix\n> the issues of v2.\n\nThanks, existing issues are fixed, but cleanup of the stashed files \nseems to not work properly when stashing a mixture of tracked and \nuntracked files:\n\n$ git init\n$ touch file1\n$ touch file2\n$ git add file1 file2\n$ git commit -m \"initial import\"\n$ echo \"a\" > file1\n$ echo \"b\" > file2\n$ touch file3\n$ git status --porcelain\n M file1\n M file2\n?? file3\n$ git stash push -u -- file1 file3\nSaved working directory and index state WIP on master: 48fb140 initial \nimport\n$ git status --porcelain\n M file1\n M file2\n\nfile1 and file3 are properly stashed, but file1 still remains modified \nin the working tree. When instead stashing file1 and file2, results are \nfine:\n\n$ git stash push -u -- file1 file2\nSaved working directory and index state WIP on master: 48fb140 initial \nimport\n$ git status\nOn branch master\nnothing to commit, working tree clean\n\n-Marc\n\n\n"},{"id":"342258","messageId":"20180319215150.GF2224@hank","threadId":"47968","inReplyTo":"bc69c1f4-7098-1bbf-e69b-97808c4d8779@syntevo.com","subject":"Re: [PATCH v3 0/2] stash push -u -- fixes","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-03-19T21:51:50Z","receivedAt":"2018-03-19T21:48:42Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 03/19, Marc Strapetz wrote:\n> On 16.03.2018 21:43, Thomas Gummerer wrote:\n> >Thanks Marc for catching the regression I almost introduced and Junio\n> >for the review of the second patch. Here's a re-roll that should fix\n> >the issues of v2.\n> \n> Thanks, existing issues are fixed, but cleanup of the stashed files seems to\n> not work properly when stashing a mixture of tracked and untracked files:\n\nThanks for the continued testing, and sorry I just can't seem to get\nthis right :/ The problem here was that I misunderstood what 'git\nls-files --error-unmatch' does without testing it myself. I'll send\nv5 in a bit, which will hopefully finally fix all the cases.\n\n> $ git init\n> $ touch file1\n> $ touch file2\n> $ git add file1 file2\n> $ git commit -m \"initial import\"\n> $ echo \"a\" > file1\n> $ echo \"b\" > file2\n> $ touch file3\n> $ git status --porcelain\n> M file1\n> M file2\n> ?? file3\n> $ git stash push -u -- file1 file3\n> Saved working directory and index state WIP on master: 48fb140 initial\n> import\n> $ git status --porcelain\n> M file1\n> M file2\n> \n> file1 and file3 are properly stashed, but file1 still remains modified in\n> the working tree. When instead stashing file1 and file2, results are fine:\n> \n> $ git stash push -u -- file1 file2\n> Saved working directory and index state WIP on master: 48fb140 initial\n> import\n> $ git status\n> On branch master\n> nothing to commit, working tree clean\n> \n> -Marc\n> \n> \n"},{"id":"342264","messageId":"20180319232156.30916-1-t.gummerer@gmail.com","threadId":"47968","inReplyTo":"20180316223715.108044-1-gitster@pobox.com","subject":"[PATCH v5 0/3] stash push -u -- fixes","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-03-19T23:21:53Z","receivedAt":"2018-03-19T23:18:48Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Thanks again Marc for all the testing and Junio for fixing up my brown\npaper bag copy-pasto.\n\nThis iteration addresses the breakage Marc noticed with the latest\nversion of the patches, adds some more tests, and moves all the new\ntests to t3905 instead of t3903, which I just noticed exists and is a\nmuch better match for the new tests.\n\nPatch 1 and 3 are the same as in the previous round, Patch 2 is mostly\nrewritten. Instead of trying to avoid part of the pipeline we're\nusing to get rid of changes, we now are getting rid of the 'git clean'\ncall in the pathspec case, and use the existing pipeline to get rid of\nchanges in untracked files as well. I'm not adding an interdiff,\nbecause Patch 2 is mostly rewritten and the other two are unchanged,\nso it is probably easiest to just review patch 2.\n\nJunio C Hamano (1):\n stash: fix nonsense pipeline\n\nThomas Gummerer (2):\n stash push: avoid printing errors\n stash push -u: don't create empty stash\n\n git-stash.sh | 12 +++++----\n t/t3905-stash-include-untracked.sh | 52 ++++++++++++++++++++++++++++++++++++++\n 2 files changed, 59 insertions(+), 5 deletions(-)\n\n-- \n2.15.1.33.g3165b24a68\n\n"},{"id":"342265","messageId":"20180319232156.30916-2-t.gummerer@gmail.com","threadId":"47968","inReplyTo":"20180319232156.30916-1-t.gummerer@gmail.com","subject":"[PATCH v5 1/3] stash: fix nonsense pipeline","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-03-19T23:21:54Z","receivedAt":"2018-03-19T23:18:54Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"From: Junio C Hamano \n\nAn earlier change bba067d2 (\"stash: don't delete untracked files\nthat match pathspec\", 2018-01-06) was made by taking a suggestion in\na list discussion [1] but did not copy the suggested snippet\ncorrectly. And the bug was unnoticed during the review and slipped\nthrough.\n\nThis fixes it.\n\n[1] https://public-inbox.org/git/xmqqpo7byjwb.fsf@gitster.mtv.corp.google.com/\n\nSigned-off-by: Junio C Hamano \n---\n git-stash.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex b48b164748..4c92ec931f 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -315,9 +315,9 @@ push_stash () {\n \n \t\tif test $# != 0\n \t\tthen\n-\t\t\tgit add -u -- \"$@\" |\n-\t\t\tgit checkout-index -z --force --stdin\n-\t\t\tgit diff-index -p --cached --binary HEAD -- \"$@\" | git apply --index -R\n+\t\t\tgit add -u -- \"$@\"\n+\t\t\tgit diff-index -p --cached --binary HEAD -- \"$@\" |\n+\t\t\tgit apply --index -R\n \t\telse\n \t\t\tgit reset --hard -q\n \t\tfi\n-- \n2.15.1.33.g3165b24a68\n\n"},{"id":"342266","messageId":"20180319232156.30916-3-t.gummerer@gmail.com","threadId":"47968","inReplyTo":"20180319232156.30916-1-t.gummerer@gmail.com","subject":"[PATCH v5 2/3] stash push: avoid printing errors","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-03-19T23:21:55Z","receivedAt":"2018-03-19T23:19:00Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"'git stash push -u -- ' prints the following errors if\n only matches untracked files:\n\n fatal: pathspec 'untracked' did not match any files\n error: unrecognized input\n\nThis is because we first clean up the untracked files using 'git clean\n', and then use a command chain involving 'git add -u\n' and 'git apply' to clear the changes to files that are in\nthe index and were stashed.\n\nAs the only includes untracked files that were already\nremoved by 'git clean', the 'git add' call will barf, and so will 'git\napply', as there are no changes that need to be applied.\n\nFix this by avoiding the 'git clean' if a pathspec is given, and use the\npipeline that's used for pathspec mode to get rid of the untracked files\nas well.\n\nReported-by: Marc Strapetz \nSigned-off-by: Thomas Gummerer \n---\n git-stash.sh | 6 +++--\n t/t3905-stash-include-untracked.sh | 46 ++++++++++++++++++++++++++++++++++++++\n 2 files changed, 50 insertions(+), 2 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 4c92ec931f..5e06f96da5 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -308,14 +308,16 @@ push_stash () {\n \tif test -z \"$patch_mode\"\n \tthen\n \t\ttest \"$untracked\" = \"all\" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=\n-\t\tif test -n \"$untracked\"\n+\t\tif test -n \"$untracked\" && test $# = 0\n \t\tthen\n \t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION -- \"$@\"\n \t\tfi\n \n \t\tif test $# != 0\n \t\tthen\n-\t\t\tgit add -u -- \"$@\"\n+\t\t\ttest -z \"$untracked\" && UPDATE_OPTION=\"-u\" || UPDATE_OPTION=\n+\t\t\ttest \"$untracked\" = \"all\" && FORCE_OPTION=\"--force\" || FORCE_OPTION=\n+\t\t\tgit add $UPDATE_OPTION $FORCE_OPTION -- \"$@\"\n \t\t\tgit diff-index -p --cached --binary HEAD -- \"$@\" |\n \t\t\tgit apply --index -R\n \t\telse\ndiff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\nindex bfde4057ad..2f9045553e 100755\n--- a/t/t3905-stash-include-untracked.sh\n+++ b/t/t3905-stash-include-untracked.sh\n@@ -228,4 +228,50 @@ test_expect_success 'stash previously ignored file' '\n \ttest_path_is_file ignored.d/foo\n '\n \n+test_expect_success 'stash -u -- doesnt print error' '\n+\t>untracked &&\n+\tgit stash push -u -- untracked 2>actual &&\n+\ttest_path_is_missing untracked &&\n+\ttest_line_count = 0 actual\n+'\n+\n+test_expect_success 'stash -u -- leaves rest of working tree in place' '\n+\t>tracked &&\n+\tgit add tracked &&\n+\t>untracked &&\n+\tgit stash push -u -- untracked &&\n+\ttest_path_is_missing untracked &&\n+\ttest_path_is_file tracked\n+'\n+\n+test_expect_success 'stash -u -- clears changes in both' '\n+\t>tracked &&\n+\tgit add tracked &&\n+\t>untracked &&\n+\tgit stash push -u -- tracked untracked &&\n+\ttest_path_is_missing tracked &&\n+\ttest_path_is_missing untracked\n+'\n+\n+test_expect_success 'stash --all -- stashes ignored file' '\n+\t>ignored.d/bar &&\n+\tgit stash push --all -- ignored.d/bar &&\n+\ttest_path_is_missing ignored.d/bar\n+'\n+\n+test_expect_success 'stash --all -- clears changes in both' '\n+\t>tracked &&\n+\tgit add tracked &&\n+\t>ignored.d/bar &&\n+\tgit stash push --all -- tracked ignored.d/bar &&\n+\ttest_path_is_missing tracked &&\n+\ttest_path_is_missing ignored.d/bar\n+'\n+\n+test_expect_success 'stash -u -- leaves ignored file alone' '\n+\t>ignored.d/bar &&\n+\tgit stash push -u -- ignored.d/bar &&\n+\ttest_path_is_file ignored.d/bar\n+'\n+\n test_done\n-- \n2.15.1.33.g3165b24a68\n\n"},{"id":"342267","messageId":"20180319232156.30916-4-t.gummerer@gmail.com","threadId":"47968","inReplyTo":"20180319232156.30916-1-t.gummerer@gmail.com","subject":"[PATCH v5 3/3] stash push -u: don't create empty stash","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-03-19T23:21:56Z","receivedAt":"2018-03-19T23:19:05Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"When introducing the stash push feature, and thus allowing users to pass\nin a pathspec to limit the files that would get stashed in\ndf6bba0937 (\"stash: teach 'push' (and 'create_stash') to honor\npathspec\", 2017-02-28), this developer missed one place where the\npathspec should be passed in.\n\nNamely in the call to the 'untracked_files()' function in the\n'no_changes()' function. This resulted in 'git stash push -u --\n' creating an empty stash when there are untracked files\nin the repository other that don't match the pathspec.\n\nAs 'git stash' never creates empty stashes, this behaviour is wrong and\nconfusing for users. Instead it should just show a message \"No local\nchanges to save\", and not create a stash.\n\nLuckily the 'untracked_files()' function already correctly respects\npathspecs that are passed to it, so the fix is simply to pass the\npathspec along to the function.\n\nReported-by: Marc Strapetz \nSigned-off-by: Thomas Gummerer \n---\n git-stash.sh | 2 +-\n t/t3905-stash-include-untracked.sh | 6 ++++++\n 2 files changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 5e06f96da5..4e55f278bd 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -39,7 +39,7 @@ fi\n no_changes () {\n \tgit diff-index --quiet --cached HEAD --ignore-submodules -- \"$@\" &&\n \tgit diff-files --quiet --ignore-submodules -- \"$@\" &&\n-\t(test -z \"$untracked\" || test -z \"$(untracked_files)\")\n+\t(test -z \"$untracked\" || test -z \"$(untracked_files \"$@\")\")\n }\n \n untracked_files () {\ndiff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\nindex 2f9045553e..3ea5b9bb3f 100755\n--- a/t/t3905-stash-include-untracked.sh\n+++ b/t/t3905-stash-include-untracked.sh\n@@ -274,4 +274,10 @@ test_expect_success 'stash -u -- leaves ignored file alone' '\n \ttest_path_is_file ignored.d/bar\n '\n \n+test_expect_success 'stash -u -- shows no changes when there are none' '\n+\tgit stash push -u -- non-existant >actual &&\n+\techo \"No local changes to save\" >expect &&\n+\ttest_i18ncmp expect actual\n+'\n+\n test_done\n-- \n2.15.1.33.g3165b24a68\n\n"},{"id":"342311","messageId":"adf6ff41-cc58-89fd-8703-d620babe0cd7@syntevo.com","threadId":"47968","inReplyTo":"20180319232156.30916-1-t.gummerer@gmail.com","subject":"Re: [PATCH v5 0/3] stash push -u -- fixes","fromName":"Marc Strapetz","fromEmail":"marc.strapetz@syntevo.com","sentAt":"2018-03-20T10:06:31Z","receivedAt":"2018-03-20T10:28:34Z","isPatch":true,"sender":{"key":"marc.strapetz@syntevo.com","avatar":"https://avatars.githubusercontent.com/u/3380730?v=4"},"body":"On 20.03.2018 00:21, Thomas Gummerer wrote:\n> Thanks again Marc for all the testing and Junio for fixing up my brown\n> paper bag copy-pasto.\n> \n> This iteration addresses the breakage Marc noticed with the latest\n> version of the patches, adds some more tests, and moves all the new\n> tests to t3905 instead of t3903, which I just noticed exists and is a\n> much better match for the new tests.\n> \n> Patch 1 and 3 are the same as in the previous round, Patch 2 is mostly\n> rewritten. Instead of trying to avoid part of the pipeline we're\n> using to get rid of changes, we now are getting rid of the 'git clean'\n> call in the pathspec case, and use the existing pipeline to get rid of\n> changes in untracked files as well. I'm not adding an interdiff,\n> because Patch 2 is mostly rewritten and the other two are unchanged,\n> so it is probably easiest to just review patch 2.\n\nThanks, Thomas. All of my manual tests have been working fine now.\n\n-Marc\n"},{"id":"342341","messageId":"xmqqzi32sn5u.fsf@gitster-ct.c.googlers.com","threadId":"47968","inReplyTo":"20180319232156.30916-3-t.gummerer@gmail.com","subject":"Re: [PATCH v5 2/3] stash push: avoid printing errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-20T16:54:21Z","receivedAt":"2018-03-20T16:54:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer writes:\n\n> ...\n> Fix this by avoiding the 'git clean' if a pathspec is given, and use the\n> pipeline that's used for pathspec mode to get rid of the untracked files\n> as well.\n\nThat made me wonder if we can get rid of 'git clean' altogether by\npretending that we saw a pathspec '.' that match everything when no\npathspec is given---that way we only have to worry about a single\ncodepath.\n\nBut I guess doing it this way can minimize potential damage. Those\nwho do not use pathspec when running \"git stash\" won't be affected\neven if this change had some bugs ;-)\n\n> diff --git a/git-stash.sh b/git-stash.sh\n> index 4c92ec931f..5e06f96da5 100755\n> --- a/git-stash.sh\n> +++ b/git-stash.sh\n> @@ -308,14 +308,16 @@ push_stash () {\n> \tif test -z \"$patch_mode\"\n> \tthen\n> \t\ttest \"$untracked\" = \"all\" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=\n> -\t\tif test -n \"$untracked\"\n> +\t\tif test -n \"$untracked\" && test $# = 0\n> \t\tthen\n> \t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION -- \"$@\"\n> \t\tfi\n> \n> \t\tif test $# != 0\n> \t\tthen\n> -\t\t\tgit add -u -- \"$@\"\n> +\t\t\ttest -z \"$untracked\" && UPDATE_OPTION=\"-u\" || UPDATE_OPTION=\n> +\t\t\ttest \"$untracked\" = \"all\" && FORCE_OPTION=\"--force\" || FORCE_OPTION=\n> +\t\t\tgit add $UPDATE_OPTION $FORCE_OPTION -- \"$@\"\n> \t\t\tgit diff-index -p --cached --binary HEAD -- \"$@\" |\n> \t\t\tgit apply --index -R\n> \t\telse\n\nThanks, I'll take the change as-is.\n\nI however wonder if we restructure the code to\n\n\tif test $# = 0\n\tthen\n\t\t# no pathspec\n\t\tif test -n \"$untracked\"\n\t\tthen\n\t\t\tgit clean --force --quiet -d $CLEAN_OPTION -- \"$@\"\n\t\tfi\n\t\tgit reset --hard -q\n\telse\n\t\ttest -z \"$untracked\" && UPDATE=-u || UPDATE=\n\t\ttest \"$untracked\" = all && FORCE=--force || FORCE=\n\t\tgit add $UPDATE $FORCE-- \"$@\"\n\t\tgit diff-index -p --cached --binary HEAD -- \"$@\" |\n\t\tgit apply --index -R\n\tfi\n\nit becomes easier to understand what is going on.\n\nThat way, once we have a plumbing command to help the else clause of\nthe above, i.e. \"git checkout --index -- \"\n[*1*], then we can lose the if/then/else and rewrite the whole \"we\nhave created stash, so it's time to get rid of local modifications\nto the paths that match the pathspec\" code to:\n\n\tif test \"$untracked\"\n\tthen\n\t\tgit clean --force --quiet -d $CLEAN_OPTION -- \"$@\"\n\tfi\n\tgit checkout --index HEAD -- \"$@\"\n\n[Footnote]\ncf. https://public-inbox.org/git/xmqq4loqplou.fsf@gitster.mtv.corp.google.com/\n\nWhat we want in the case in the code in question when there is\npathspec is \"match the index entries and the working tree files to\nthose that appear in a given tree for paths that match the given\npathspec\". This is close to \"git checkout -- \"\nbut not quite. Current \"git checkout -- \" is\nan overlay operation in that paths that match but do not\nexist in are *NOT* removed from the working tree. We\nobviously cannot change the behaviour of the command.\n\nBut we can add an option to ask for the new behaviour. In general,\nfor an operation that affects the index and the working tree, we can\nhave \"--cached\" mode that affects only the index without touching\nthe working tree, and \"--index\" mode that affects both.\n\n\"git reset -- \", which is a UI mistake, is\nbetter spelled \"git checkout --cached -- \". We\ntake paths that match from and stuff into the\nindex, and remove entries from the index for paths that do not exist\nin . And if we extend that to \"--index\" mode, that is\nexactly what we want to happen.\n"},{"id":"342459","messageId":"20180321213626.GG2224@hank","threadId":"47968","inReplyTo":"xmqqzi32sn5u.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v5 2/3] stash push: avoid printing errors","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-03-21T21:36:26Z","receivedAt":"2018-03-21T21:33:14Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 03/20, Junio C Hamano wrote:\n> Thomas Gummerer writes:\n> \n> > ...\n> > Fix this by avoiding the 'git clean' if a pathspec is given, and use the\n> > pipeline that's used for pathspec mode to get rid of the untracked files\n> > as well.\n> \n> That made me wonder if we can get rid of 'git clean' altogether by\n> pretending that we saw a pathspec '.' that match everything when no\n> pathspec is given---that way we only have to worry about a single\n> codepath.\n> \n> But I guess doing it this way can minimize potential damage. Those\n> who do not use pathspec when running \"git stash\" won't be affected\n> even if this change had some bugs ;-)\n\nHeh yeah, we found enough bugs in this code so far, so it's probably\nbest to leave the part that's working alone at least for now.\n\n> > diff --git a/git-stash.sh b/git-stash.sh\n> > index 4c92ec931f..5e06f96da5 100755\n> > --- a/git-stash.sh\n> > +++ b/git-stash.sh\n> > @@ -308,14 +308,16 @@ push_stash () {\n> > \tif test -z \"$patch_mode\"\n> > \tthen\n> > \t\ttest \"$untracked\" = \"all\" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=\n> > -\t\tif test -n \"$untracked\"\n> > +\t\tif test -n \"$untracked\" && test $# = 0\n> > \t\tthen\n> > \t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION -- \"$@\"\n\nArgh I just noticed we could drop the \"$@\" here, as this is no longer\nthe pathspec case. It doesn't hurt anything, except it may be a bit\nconfusing when reading the code.\n\nAlthough if we end up implementing 'git checkout --index ',\nwe'd have to add it back, but we do have a test covering this case, so\nthere's no worries about forgetting to add it back.\n\n> > \t\tfi\n> > \n> > \t\tif test $# != 0\n> > \t\tthen\n> > -\t\t\tgit add -u -- \"$@\"\n> > +\t\t\ttest -z \"$untracked\" && UPDATE_OPTION=\"-u\" || UPDATE_OPTION=\n> > +\t\t\ttest \"$untracked\" = \"all\" && FORCE_OPTION=\"--force\" || FORCE_OPTION=\n> > +\t\t\tgit add $UPDATE_OPTION $FORCE_OPTION -- \"$@\"\n> > \t\t\tgit diff-index -p --cached --binary HEAD -- \"$@\" |\n> > \t\t\tgit apply --index -R\n> > \t\telse\n> \n> Thanks, I'll take the change as-is.\n> \n> I however wonder if we restructure the code to\n> \n> \tif test $# = 0\n> \tthen\n> \t\t# no pathspec\n> \t\tif test -n \"$untracked\"\n> \t\tthen\n> \t\t\tgit clean --force --quiet -d $CLEAN_OPTION -- \"$@\"\n> \t\tfi\n> \t\tgit reset --hard -q\n> \telse\n> \t\ttest -z \"$untracked\" && UPDATE=-u || UPDATE=\n> \t\ttest \"$untracked\" = all && FORCE=--force || FORCE=\n> \t\tgit add $UPDATE $FORCE-- \"$@\"\n> \t\tgit diff-index -p --cached --binary HEAD -- \"$@\" |\n> \t\tgit apply --index -R\n> \tfi\n> \n> it becomes easier to understand what is going on.\n\nI like that code structure more than what I have now. I see you\nalready merged what I had to next, and I like keeping the change small\nnow that we're in the rc period (assuming you want to get this into\n2.17?) Maybe we can restructure the code as a separate cleanup once\n2.17 is out, so this has more time to cook in master and hopefully\nwe'd notice regressions before the next release?\n\n> That way, once we have a plumbing command to help the else clause of\n> the above, i.e. \"git checkout --index -- \"\n> [*1*], then we can lose the if/then/else and rewrite the whole \"we\n> have created stash, so it's time to get rid of local modifications\n> to the paths that match the pathspec\" code to:\n> \n> \tif test \"$untracked\"\n> \tthen\n> \t\tgit clean --force --quiet -d $CLEAN_OPTION -- \"$@\"\n> \tfi\n> \tgit checkout --index HEAD -- \"$@\"\n\nYeah, this would be nice to have. I wanted to have a look at what it\nwould take to implement 'git checkout --{cached,index}', but I'm not\nfamiliar with the checkout code at all, so it will probably be a while\nuntil I can get around to do it. \n\n> [Footnote]\n> cf. https://public-inbox.org/git/xmqq4loqplou.fsf@gitster.mtv.corp.google.com/\n> \n> What we want in the case in the code in question when there is\n> pathspec is \"match the index entries and the working tree files to\n> those that appear in a given tree for paths that match the given\n> pathspec\". This is close to \"git checkout -- \"\n> but not quite. Current \"git checkout -- \" is\n> an overlay operation in that paths that match but do not\n> exist in are *NOT* removed from the working tree. We\n> obviously cannot change the behaviour of the command.\n> \n> But we can add an option to ask for the new behaviour. In general,\n> for an operation that affects the index and the working tree, we can\n> have \"--cached\" mode that affects only the index without touching\n> the working tree, and \"--index\" mode that affects both.\n> \n> \"git reset -- \", which is a UI mistake, is\n> better spelled \"git checkout --cached -- \". We\n> take paths that match from and stuff into the\n> index, and remove entries from the index for paths that do not exist\n> in . And if we extend that to \"--index\" mode, that is\n> exactly what we want to happen.\n"},{"id":"342462","messageId":"20180321215310.GH2224@hank","threadId":"47968","inReplyTo":"20180321213626.GG2224@hank","subject":"[PATCH] stash: drop superfluos pathspec parameter (was: Re: [PATCH v5 2/3] stash push: avoid printing errors)","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2018-03-21T21:53:10Z","receivedAt":"2018-03-21T21:49:57Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 03/21, Thomas Gummerer wrote:\n>\n> Argh I just noticed we could drop the \"$@\" here, as this is no longer\n> the pathspec case. It doesn't hurt anything, except it may be a bit\n> confusing when reading the code.\n> \n> Although if we end up implementing 'git checkout --index ',\n> we'd have to add it back, but we do have a test covering this case, so\n> there's no worries about forgetting to add it back.\n\nHere's a patch for that. Not sure it's worth doing, but since we're\nalready touching the area, this may be a good cleanup.\n\nThis is based on top of tg/stash-untracked-with-pathspec-fix.\n\n--- >8 ---\nSubject: [PATCH] stash: drop superfluos pathspec parameter\n\nSince 833622a945 (\"stash push: avoid printing errors\", 2018-03-19) we\ndon't use the 'git clean' call for the pathspec case anymore. The\ncommit however forgot to remove the pathspec argument to the call.\nRemove the superfluos argument to make the code a little more obvious.\n\nSigned-off-by: Thomas Gummerer \n---\n git-stash.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 4e55f278bd..d31924aea3 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -310,7 +310,7 @@ push_stash () {\n \t\ttest \"$untracked\" = \"all\" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=\n \t\tif test -n \"$untracked\" && test $# = 0\n \t\tthen\n-\t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION -- \"$@\"\n+\t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION\n \t\tfi\n \n \t\tif test $# != 0\n-- \n2.15.1.33.g3165b24a68\n\n"},{"id":"342465","messageId":"xmqq4ll9nld2.fsf@gitster-ct.c.googlers.com","threadId":"47968","inReplyTo":"20180321213626.GG2224@hank","subject":"Re: [PATCH v5 2/3] stash push: avoid printing errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-21T21:56:41Z","receivedAt":"2018-03-21T21:56:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer writes:\n\n>> > diff --git a/git-stash.sh b/git-stash.sh\n>> > index 4c92ec931f..5e06f96da5 100755\n>> > --- a/git-stash.sh\n>> > +++ b/git-stash.sh\n>> > @@ -308,14 +308,16 @@ push_stash () {\n>> > \tif test -z \"$patch_mode\"\n>> > \tthen\n>> > \t\ttest \"$untracked\" = \"all\" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=\n>> > -\t\tif test -n \"$untracked\"\n>> > +\t\tif test -n \"$untracked\" && test $# = 0\n>> > \t\tthen\n>> > \t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION -- \"$@\"\n>\n> Argh I just noticed we could drop the \"$@\" here, as this is no longer\n> the pathspec case. It doesn't hurt anything, except it may be a bit\n> confusing when reading the code.\n\nYes, we could, but we do not have to. I think it is fine to leave\nit in especially if we envision that the codepaths for two cases\nwill be unified in the future.\n"},{"id":"342470","messageId":"xmqqzi31m6ag.fsf@gitster-ct.c.googlers.com","threadId":"47968","inReplyTo":"20180321215310.GH2224@hank","subject":"Re: [PATCH] stash: drop superfluos pathspec parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-21T22:07:35Z","receivedAt":"2018-03-21T22:07:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer writes:\n\n> On 03/21, Thomas Gummerer wrote:\n>>\n>> Argh I just noticed we could drop the \"$@\" here, as this is no longer\n>> the pathspec case. It doesn't hurt anything, except it may be a bit\n>> confusing when reading the code.\n>> \n>> Although if we end up implementing 'git checkout --index ',\n>> we'd have to add it back, but we do have a test covering this case, so\n>> there's no worries about forgetting to add it back.\n>\n> Here's a patch for that. Not sure it's worth doing, but since we're\n> already touching the area, this may be a good cleanup.\n\nOK, I can go either way, and since you have already written a\nchange, let's not waste it ;-)\n\nThanks.\n\n>\n> This is based on top of tg/stash-untracked-with-pathspec-fix.\n>\n> --- >8 ---\n> Subject: [PATCH] stash: drop superfluos pathspec parameter\n>\n> Since 833622a945 (\"stash push: avoid printing errors\", 2018-03-19) we\n> don't use the 'git clean' call for the pathspec case anymore. The\n> commit however forgot to remove the pathspec argument to the call.\n> Remove the superfluos argument to make the code a little more obvious.\n>\n> Signed-off-by: Thomas Gummerer \n> ---\n> git-stash.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/git-stash.sh b/git-stash.sh\n> index 4e55f278bd..d31924aea3 100755\n> --- a/git-stash.sh\n> +++ b/git-stash.sh\n> @@ -310,7 +310,7 @@ push_stash () {\n> \t\ttest \"$untracked\" = \"all\" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=\n> \t\tif test -n \"$untracked\" && test $# = 0\n> \t\tthen\n> -\t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION -- \"$@\"\n> +\t\t\tgit clean --force --quiet -d $CLEAN_X_OPTION\n> \t\tfi\n> \n> \t\tif test $# != 0\n"}]}