{"thread":{"id":"10354","subject":"git stash apply usability issues","startedAt":"2007-10-18T08:32:33Z","lastAt":"2007-10-19T13:57:23Z","messageCount":8,"participants":["Johannes Sixt","Steven Grimm","Shawn O. Pearce","Karl Hasselström","David Kastrup"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"56374","messageId":"47171A21.9030003@viscovery.net","threadId":"10354","inReplyTo":null,"subject":"git stash apply usability issues","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-10-18T08:32:33Z","receivedAt":"2007-10-18T08:32:33Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"(1) Looking at git-stash.sh I see a few uses of 'git diff' in apply_stash(). \nShouldn't these use one of git-diff-{tree,index,files)? The reason is that \nporcelain 'git diff' invokes custom diff drivers (that in my case run a UI \nprogram), whereas the plumbing does not.\n\nIs there a particular reason to use porcelain 'git diff'?\n\n(2) when 'git stash apply' runs merge-recursive, it treats the current state \nas 'ours' and the stash as 'theirs'. IMHO it should be the other way round: \nI have stashed away changes to a binary file. Then committed a different \nmodification to it, and now want to apply the stash. This results in a \nconflict that leaves the current state in the working tree, but I had \npreferred that the stashed binary file were in the working tree now.\n\nWhat do other git-stash users think about changing the order?\n\n-- Hannes\n"},{"id":"56400","messageId":"471769D3.4080908@midwinter.com","threadId":"10354","inReplyTo":"47171A21.9030003@viscovery.net","subject":"Re: git stash apply usability issues","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-10-18T14:12:35Z","receivedAt":"2007-10-18T14:12:35Z","isPatch":false,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"Johannes Sixt wrote:\n> (2) when 'git stash apply' runs merge-recursive, it treats the current \n> state as 'ours' and the stash as 'theirs'. IMHO it should be the other \n> way round: I have stashed away changes to a binary file. Then \n> committed a different modification to it, and now want to apply the \n> stash. This results in a conflict that leaves the current state in the \n> working tree, but I had preferred that the stashed binary file were in \n> the working tree now.\n>\n> What do other git-stash users think about changing the order?\n\nSeems right to me. I'd expect to get the stashed version in the working \ntree in that case.\n\n-Steve\n"},{"id":"56462","messageId":"20071019013156.GU14735@spearce.org","threadId":"10354","inReplyTo":"47171A21.9030003@viscovery.net","subject":"Re: git stash apply usability issues","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-19T01:31:56Z","receivedAt":"2007-10-19T01:31:56Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> wrote:\n> (2) when 'git stash apply' runs merge-recursive, it treats the current \n> state as 'ours' and the stash as 'theirs'. IMHO it should be the other way \n> round: I have stashed away changes to a binary file. Then committed a \n> different modification to it, and now want to apply the stash. This results \n> in a conflict that leaves the current state in the working tree, but I had \n> preferred that the stashed binary file were in the working tree now.\n> \n> What do other git-stash users think about changing the order?\n\nThe current order is the same order that git-rebase uses.  I'm not\nsaying its correct, just that its the same as rebase.  I think rebase\nis also backwards and if we change git-stash we should also change\ngit-rebase at the same time (though probably not in the same commit).\n\n-- \nShawn.\n"},{"id":"56463","messageId":"20071019013350.GA14020@spearce.org","threadId":"10354","inReplyTo":"47171A21.9030003@viscovery.net","subject":"[PATCH] Avoid invoking diff drivers during git-stash","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-19T01:33:50Z","receivedAt":"2007-10-19T01:33:50Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"git-stash needs to restrict itself to plumbing when running automated\ndiffs as part of its operation as the user may have configured a\ncustom diff driver that opens an interactive UI for certain/all\nfiles.  Doing that during scripted actions is very unfriendly to\nthe end-user and may cause git-stash to fail to work.\n\nReported by Johannes Sixt\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n\n Johannes Sixt <j.sixt@viscovery.net> wrote:\n > (1) Looking at git-stash.sh I see a few uses of 'git diff' in\n > apply_stash(). Shouldn't these use one of git-diff-{tree,index,files)? The\n > reason is that porcelain 'git diff' invokes custom diff drivers (that in my   \n > case run a UI program), whereas the plumbing does not.\n >\n > Is there a particular reason to use porcelain 'git diff'?\n\n Does this fix the problem?\n\n git-stash.sh |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 7ba6162..def3163 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -110,7 +110,7 @@ show_stash () {\n \n \tw_commit=$(git rev-parse --verify \"$s\") &&\n \tb_commit=$(git rev-parse --verify \"$s^\") &&\n-\tgit diff $flags $b_commit $w_commit\n+\tgit diff-tree $flags $b_commit $w_commit\n }\n \n apply_stash () {\n@@ -139,7 +139,7 @@ apply_stash () {\n \tunstashed_index_tree=\n \tif test -n \"$unstash_index\" && test \"$b_tree\" != \"$i_tree\"\n \tthen\n-\t\tgit diff --binary $s^2^..$s^2 | git apply --cached\n+\t\tgit diff-tree --binary $s^2^..$s^2 | git apply --cached\n \t\ttest $? -ne 0 &&\n \t\t\tdie 'Conflicts in index. Try without --index.'\n \t\tunstashed_index_tree=$(git-write-tree) ||\n@@ -162,7 +162,7 @@ apply_stash () {\n \t\t\tgit read-tree \"$unstashed_index_tree\"\n \t\telse\n \t\t\ta=\"$TMP-added\" &&\n-\t\t\tgit diff --cached --name-only --diff-filter=A $c_tree >\"$a\" &&\n+\t\t\tgit diff-index --cached --name-only --diff-filter=A $c_tree >\"$a\" &&\n \t\t\tgit read-tree --reset $c_tree &&\n \t\t\tgit update-index --add --stdin <\"$a\" ||\n \t\t\t\tdie \"Cannot unstage modified files\"\n-- \n1.5.3.4.1249.g895be\n"},{"id":"56524","messageId":"471849D7.1020303@viscovery.net","threadId":"10354","inReplyTo":"20071019013350.GA14020@spearce.org","subject":"Re: [PATCH] Avoid invoking diff drivers during git-stash","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-10-19T06:08:23Z","receivedAt":"2007-10-19T06:08:23Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Shawn,\n\nthanks for the fast response with a patch.\n\nShawn O. Pearce schrieb:\n>  Johannes Sixt <j.sixt@viscovery.net> wrote:\n>  > (1) Looking at git-stash.sh I see a few uses of 'git diff' in\n>  > apply_stash(). Shouldn't these use one of git-diff-{tree,index,files)? The\n>  > reason is that porcelain 'git diff' invokes custom diff drivers (that in my   \n>  > case run a UI program), whereas the plumbing does not.\n>  >\n>  > Is there a particular reason to use porcelain 'git diff'?\n> \n>  Does this fix the problem?\n\nIt does!\n\n> @@ -110,7 +110,7 @@ show_stash () {\n>  \n>  \tw_commit=$(git rev-parse --verify \"$s\") &&\n>  \tb_commit=$(git rev-parse --verify \"$s^\") &&\n> -\tgit diff $flags $b_commit $w_commit\n> +\tgit diff-tree $flags $b_commit $w_commit\n\nHowever, this porcelain 'git diff' should actually remain because it's part \nof show_stash().\n\n-- Hannes\n"},{"id":"56525","messageId":"20071019061351.GL14735@spearce.org","threadId":"10354","inReplyTo":"471849D7.1020303@viscovery.net","subject":"Re: [PATCH] Avoid invoking diff drivers during git-stash","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-19T06:13:51Z","receivedAt":"2007-10-19T06:13:51Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> wrote:\n> Shawn O. Pearce schrieb:\n> > Johannes Sixt <j.sixt@viscovery.net> wrote:\n> > > (1) Looking at git-stash.sh I see a few uses of 'git diff' in\n> > > apply_stash(). Shouldn't these use one of git-diff-{tree,index,files)? \n> > The\n> > > reason is that porcelain 'git diff' invokes custom diff drivers (that \n> > in my   > case run a UI program), whereas the plumbing does not.\n> > >\n> > > Is there a particular reason to use porcelain 'git diff'?\n> >\n> > Does this fix the problem?\n> \n> It does!\n> \n> >@@ -110,7 +110,7 @@ show_stash () {\n> > \n> > \tw_commit=$(git rev-parse --verify \"$s\") &&\n> > \tb_commit=$(git rev-parse --verify \"$s^\") &&\n> >-\tgit diff $flags $b_commit $w_commit\n> >+\tgit diff-tree $flags $b_commit $w_commit\n> \n> However, this porcelain 'git diff' should actually remain because it's part \n> of show_stash().\n\nHeh.  Damn.  I was just starting to prepare my evening push and\nthis patch is in maint, which I just merged to master, and I just\nrebased all of my pu topic branches over that.  Junio's Meta toolkit\ndoesn't have an \"unrebase\" so I can go back and amend that damn\ncommit before pushing.\n\nI'm feeling lazy and don't want to create an unRB right now.\nI'll probably just throw another commit into maint to fix the\nabove hunk.  Thanks for catching it.\n\n-- \nShawn.\n"},{"id":"56574","messageId":"20071019132753.GA23765@diana.vm.bytemark.co.uk","threadId":"10354","inReplyTo":"20071019013156.GU14735@spearce.org","subject":"Re: git stash apply usability issues","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-10-19T13:27:53Z","receivedAt":"2007-10-19T13:27:53Z","isPatch":false,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-10-18 21:31:56 -0400, Shawn O. Pearce wrote:\n\n> Johannes Sixt <j.sixt@viscovery.net> wrote:\n\n> > (2) when 'git stash apply' runs merge-recursive, it treats the\n> > current state as 'ours' and the stash as 'theirs'. IMHO it should\n> > be the other way round: I have stashed away changes to a binary\n> > file. Then committed a different modification to it, and now want\n> > to apply the stash. This results in a conflict that leaves the\n> > current state in the working tree, but I had preferred that the\n> > stashed binary file were in the working tree now.\n> >\n> > What do other git-stash users think about changing the order?\n>\n> The current order is the same order that git-rebase uses. I'm not\n> saying its correct, just that its the same as rebase.\n\nFWIW, StGit push works the same way. The idea being that the current\nHEAD is our current state (\"ours\"), and the patch we're pushing is\nsome change we want to apply (\"theirs\"). I always felt that this was a\nvery natural order of things. But I guess the philosophy in the\n\"stash\" case is subtly different, so maybe the change is warranted\nthere.\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"56580","messageId":"86y7dz9ph8.fsf@lola.quinscape.zz","threadId":"10354","inReplyTo":"20071019132753.GA23765@diana.vm.bytemark.co.uk","subject":"Re: git stash apply usability issues","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-10-19T13:57:23Z","receivedAt":"2007-10-19T13:57:23Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Karl Hasselström <kha@treskal.com> writes:\n\n> On 2007-10-18 21:31:56 -0400, Shawn O. Pearce wrote:\n>\n>> Johannes Sixt <j.sixt@viscovery.net> wrote:\n>\n>> > (2) when 'git stash apply' runs merge-recursive, it treats the\n>> > current state as 'ours' and the stash as 'theirs'. IMHO it should\n>> > be the other way round: I have stashed away changes to a binary\n>> > file. Then committed a different modification to it, and now want\n>> > to apply the stash. This results in a conflict that leaves the\n>> > current state in the working tree, but I had preferred that the\n>> > stashed binary file were in the working tree now.\n>> >\n>> > What do other git-stash users think about changing the order?\n>>\n>> The current order is the same order that git-rebase uses. I'm not\n>> saying its correct, just that its the same as rebase.\n>\n> FWIW, StGit push works the same way. The idea being that the current\n> HEAD is our current state (\"ours\"), and the patch we're pushing is\n> some change we want to apply (\"theirs\"). I always felt that this was a\n> very natural order of things. But I guess the philosophy in the\n> \"stash\" case is subtly different, so maybe the change is warranted\n> there.\n\nWell, maybe one should then just name this \"current\" and \"separate\"\ninstead of \"ours\" and \"theirs\".\n\n-- \nDavid Kastrup\n"}]}