{"thread":{"id":"33806","subject":"[PATCH 0/6] Get stash to help rebase.autostash","startedAt":"2013-05-13T12:45:48Z","lastAt":"2013-05-13T19:25:25Z","messageCount":21,"participants":["Ramkumar Ramachandra","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"217154","messageId":"1368449154-21882-1-git-send-email-artagnon@gmail.com","threadId":"33806","inReplyTo":null,"subject":"[PATCH 0/6] Get stash to help rebase.autostash","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-13T12:45:48Z","receivedAt":"2013-05-13T12:45:48Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nThis topic is based on the rebase.autostash topic.  It cleans up a few\nthings, introduces 'git stash store', and patches rebase to use it.\nShould be simple enough.\n\nThanks.\n\nRamkumar Ramachandra (6):\n  Documentation/stash: correct synopsis for create\n  Documentation/stash: document short form -p in synopsis\n  stash: simplify option parser for create\n  stash: introduce 'git stash store'\n  stash: tweak error message in store_stash ()\n  rebase: use 'git stash store' to simplify logic\n\n Documentation/git-stash.txt | 10 ++++++++--\n git-rebase.sh               |  6 +-----\n git-stash.sh                | 32 +++++++++++++++++++++-----------\n t/t3903-stash.sh            | 20 ++++++++++++++++++++\n 4 files changed, 50 insertions(+), 18 deletions(-)\n\n-- \n1.8.3.rc1.57.g4ac1522\n"},{"id":"217159","messageId":"1368449154-21882-2-git-send-email-artagnon@gmail.com","threadId":"33806","inReplyTo":"1368449154-21882-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/6] Documentation/stash: correct synopsis for create","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-13T12:45:49Z","receivedAt":"2013-05-13T12:45:49Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"'git stash create' can optionally take one or two arguments.  Document\nthis.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Documentation/git-stash.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 711ffe1..35a0134 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -16,7 +16,7 @@ SYNOPSIS\n 'git stash' [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]\n \t     [-u|--include-untracked] [-a|--all] [<message>]]\n 'git stash' clear\n-'git stash' create\n+'git stash' create [<message> [<include-untracked-p>]]\n \n DESCRIPTION\n -----------\n-- \n1.8.3.rc1.57.g4ac1522\n"},{"id":"217160","messageId":"1368449154-21882-3-git-send-email-artagnon@gmail.com","threadId":"33806","inReplyTo":"1368449154-21882-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/6] Documentation/stash: document short form -p in synopsis","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-13T12:45:50Z","receivedAt":"2013-05-13T12:45:50Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"'git stash save' can take -p, the short form of --patch, as an\noptional.  Document this.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Documentation/git-stash.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 35a0134..05e462b 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n 'git stash' drop [-q|--quiet] [<stash>]\n 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]\n 'git stash' branch <branchname> [<stash>]\n-'git stash' [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]\n+'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n \t     [-u|--include-untracked] [-a|--all] [<message>]]\n 'git stash' clear\n 'git stash' create [<message> [<include-untracked-p>]]\n-- \n1.8.3.rc1.57.g4ac1522\n"},{"id":"217155","messageId":"1368449154-21882-4-git-send-email-artagnon@gmail.com","threadId":"33806","inReplyTo":"1368449154-21882-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 3/6] stash: simplify option parser for create","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-13T12:45:51Z","receivedAt":"2013-05-13T12:45:51Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The option parser for create unnecessarily checks \"$1\" inside a case\nstatement that matches \"$1\" in the first place.  Also, use \"$@\", not\n\"$*\", as our caller is expecting \"$1\" \"$2\", not \"$1c$2\" (where c is the\nfirst character of IFS).\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n git-stash.sh | 7 ++-----\n 1 file changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex bbefdf6..0ede313 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -546,11 +546,8 @@ clear)\n \tclear_stash \"$@\"\n \t;;\n create)\n-\tif test $# -gt 0 && test \"$1\" = create\n-\tthen\n-\t\tshift\n-\tfi\n-\tcreate_stash \"$*\" && echo \"$w_commit\"\n+\tshift\n+\tcreate_stash \"$@\" && echo \"$w_commit\"\n \t;;\n drop)\n \tshift\n-- \n1.8.3.rc1.57.g4ac1522\n"},{"id":"217156","messageId":"1368449154-21882-5-git-send-email-artagnon@gmail.com","threadId":"33806","inReplyTo":"1368449154-21882-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 4/6] stash: introduce 'git stash store'","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-13T12:45:52Z","receivedAt":"2013-05-13T12:45:52Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"save_stash() contains the logic for doing two potentially independent\noperations; the first is preparing the stash merge commit, and the\nsecond is updating the stash ref/ reflog accordingly.  While the first\noperation is abstracted out into a create_stash() for callers to access\nvia 'git stash create', the second one is not.  Fix this by factoring\nout the logic for storing the stash into a store_stash() that callers\ncan access via 'git stash store'.\n\nLike create, store is not intended for end user interactive use, but for\ncallers in other scripts.  We can simplify the logic in the\nrebase.autostash feature using this new subcommand.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Documentation/git-stash.txt |  6 ++++++\n git-stash.sh                | 25 +++++++++++++++++++------\n t/t3903-stash.sh            | 20 ++++++++++++++++++++\n 3 files changed, 45 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 05e462b..e58ab74 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -17,6 +17,7 @@ SYNOPSIS\n \t     [-u|--include-untracked] [-a|--all] [<message>]]\n 'git stash' clear\n 'git stash' create [<message> [<include-untracked-p>]]\n+'git stash' store <commit> <message>\n \n DESCRIPTION\n -----------\n@@ -152,6 +153,11 @@ create::\n \tCreate a stash (which is a regular commit object) and return its\n \tobject name, without storing it anywhere in the ref namespace.\n \n+store::\n+\n+\tStore a given stash created via 'git stash create' (which is a\n+\tdangling merge commit) in the stash ref, updating the stash\n+\treflog.\n \n DISCUSSION\n ----------\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 0ede313..1d483f5 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -156,6 +156,20 @@ create_stash () {\n \tdie \"$(gettext \"Cannot record working tree state\")\"\n }\n \n+store_stash () {\n+\tif test $# != 2\n+\tthen\n+\t\tdie \"$(gettext \"git stash store requires two arguments\")\"\n+\tfi\n+\tw_commit=\"$1\"\n+\tstash_msg=\"$2\"\n+\n+\t# Make sure the reflog for stash is kept.\n+\t: >>\"$GIT_DIR/logs/$ref_stash\"\n+\tgit update-ref -m \"$stash_msg\" $ref_stash $w_commit ||\n+\tdie \"$(gettext \"Cannot save the current status\")\"\n+}\n+\n save_stash () {\n \tkeep_index=\n \tpatch_mode=\n@@ -227,12 +241,7 @@ save_stash () {\n \t\tclear_stash || die \"$(gettext \"Cannot initialize stash\")\"\n \n \tcreate_stash \"$stash_msg\" $untracked\n-\n-\t# Make sure the reflog for stash is kept.\n-\t: >>\"$GIT_DIR/logs/$ref_stash\"\n-\n-\tgit update-ref -m \"$stash_msg\" $ref_stash $w_commit ||\n-\t\tdie \"$(gettext \"Cannot save the current status\")\"\n+\tstore_stash $w_commit \"$stash_msg\"\n \tsay Saved working directory and index state \"$stash_msg\"\n \n \tif test -z \"$patch_mode\"\n@@ -549,6 +558,10 @@ create)\n \tshift\n \tcreate_stash \"$@\" && echo \"$w_commit\"\n \t;;\n+store)\n+\tshift\n+\tstore_stash \"$@\"\n+\t;;\n drop)\n \tshift\n \tdrop_stash \"$@\"\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 5dfbda7..2ff3afd 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -637,4 +637,24 @@ test_expect_success 'stash where working directory contains \"HEAD\" file' '\n \ttest_cmp output expect\n '\n \n+test_expect_success 'store - argument 1 not a commit' '\n+\ttest_must_fail git stash store foo bar\n+'\n+\n+test_expect_success 'store - updates stash ref and reflog' '\n+\tgit stash clear &&\n+\tgit reset --hard &&\n+\techo quux >file &&\n+\tgit add file &&\n+\tSTASH_ID=$(git stash create) &&\n+\tgit reset --hard &&\n+\t! grep quux file &&\n+\tgit stash store ${STASH_ID} quuxery &&\n+\ttest $(cat .git/refs/stash) = ${STASH_ID} &&\n+\tgrep ${STASH_ID} .git/logs/refs/stash &&\n+\tgit stash pop &&\n+\tgrep quux file &&\n+\tfalse\n+'\n+\n test_done\n-- \n1.8.3.rc1.57.g4ac1522\n"},{"id":"217157","messageId":"1368449154-21882-6-git-send-email-artagnon@gmail.com","threadId":"33806","inReplyTo":"1368449154-21882-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 5/6] stash: tweak error message in store_stash ()","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-13T12:45:53Z","receivedAt":"2013-05-13T12:45:53Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"When the update-ref invocation fails, store_stash currently prints:\n\n    Cannot save the current status\n\nThis is not very useful for diagnosing the problem.  Instead, print:\n\n    Cannot store 688268c4254ca5dc6e2effa83bae4f0dbbe75e5b\n\nso we can inspect the object and analyze why the update-ref failed.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\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 1d483f5..24d72fc 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -167,7 +167,7 @@ store_stash () {\n \t# Make sure the reflog for stash is kept.\n \t: >>\"$GIT_DIR/logs/$ref_stash\"\n \tgit update-ref -m \"$stash_msg\" $ref_stash $w_commit ||\n-\tdie \"$(gettext \"Cannot save the current status\")\"\n+\tdie \"$(gettext \"Cannot store $w_commit\")\"\n }\n \n save_stash () {\n-- \n1.8.3.rc1.57.g4ac1522\n"},{"id":"217158","messageId":"1368449154-21882-7-git-send-email-artagnon@gmail.com","threadId":"33806","inReplyTo":"1368449154-21882-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 6/6] rebase: use 'git stash store' to simplify logic","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-13T12:45:54Z","receivedAt":"2013-05-13T12:45:54Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"rebase has no reason to know about the implementation of the stash.  In\nthe case when applying the autostash results in conflicts, replace the\nrelevant code in finish_rebase () to simply call 'git stash store'.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n git-rebase.sh | 6 +-----\n 1 file changed, 1 insertion(+), 5 deletions(-)\n\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex f4a3a26..7c82f95 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -153,11 +153,7 @@ finish_rebase () {\n \t\tthen\n \t\t\tgettext \"Applied autostash.\"\n \t\telse\n-\t\t\tref_stash=refs/stash &&\n-\t\t\t: >>\"$GIT_DIR/logs/$ref_stash\" &&\n-\t\t\tgit update-ref -m \"autostash\" $ref_stash $stash_sha1 ||\n-\t\t\tdie \"$(eval_gettext 'Cannot store $stash_sha1')\"\n-\n+\t\t\tgit stash store $stash_sha1 \"autostash\" &&\n \t\t\tgettext 'Applying autostash resulted in conflicts.\n Your changes are safe in the stash.\n You can run \"git stash pop\" or \"git stash drop\" at any time.\n-- \n1.8.3.rc1.57.g4ac1522\n"},{"id":"217182","messageId":"7vfvxrosnf.fsf@alter.siamese.dyndns.org","threadId":"33806","inReplyTo":"1368449154-21882-4-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 3/6] stash: simplify option parser for create","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-13T14:34:28Z","receivedAt":"2013-05-13T14:34:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> The option parser for create unnecessarily checks \"$1\" inside a case\n> statement that matches \"$1\" in the first place.  Also, use \"$@\", not\n> \"$*\", as our caller is expecting \"$1\" \"$2\", not \"$1c$2\" (where c is the\n> first character of IFS).\n\nThe first part of the patch may be OK but the rest unfortunately is\nwrong.\n\nThe semi-user facing \"git stash create\" never was meant to take\nanything but a message sentence and \"$*\" is the proper way to say\n\"everything is meant for a single message (just like echo)\".\nChanging it to \"$@\" will change the semantics in a big way.\n\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> ---\n>  git-stash.sh | 7 ++-----\n>  1 file changed, 2 insertions(+), 5 deletions(-)\n>\n> diff --git a/git-stash.sh b/git-stash.sh\n> index bbefdf6..0ede313 100755\n> --- a/git-stash.sh\n> +++ b/git-stash.sh\n> @@ -546,11 +546,8 @@ clear)\n>  \tclear_stash \"$@\"\n>  \t;;\n>  create)\n> -\tif test $# -gt 0 && test \"$1\" = create\n> -\tthen\n> -\t\tshift\n> -\tfi\n> -\tcreate_stash \"$*\" && echo \"$w_commit\"\n> +\tshift\n> +\tcreate_stash \"$@\" && echo \"$w_commit\"\n>  \t;;\n>  drop)\n>  \tshift\n"},{"id":"217183","messageId":"7vbo8fosd9.fsf@alter.siamese.dyndns.org","threadId":"33806","inReplyTo":"1368449154-21882-5-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 4/6] stash: introduce 'git stash store'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-13T14:40:34Z","receivedAt":"2013-05-13T14:40:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\n> index 05e462b..e58ab74 100644\n> --- a/Documentation/git-stash.txt\n> +++ b/Documentation/git-stash.txt\n> @@ -17,6 +17,7 @@ SYNOPSIS\n>  \t     [-u|--include-untracked] [-a|--all] [<message>]]\n>  'git stash' clear\n>  'git stash' create [<message> [<include-untracked-p>]]\n> +'git stash' store <commit> <message>\n\nTwo points:\n\n - We should not advertise \"store\" (and \"create\" for that matter) in\n   the end-user facing documentation.  IIRC, \"git stash -h\"\n   deliberately omits 'create'; having it in the documentation is\n   unavoidable, but it was a mistake that it was not marked with\n   \"this is most likely not what you want to use; see 'save'\".\n\n   It may even be better with a leading underscore or two in the\n   name that clearly marks it as \"not meant for direct end-user\n   consumption\".\n\n - The error message store_stash() gives should not be hardcoded in\n   that function.\n\n   Save-stash may want to keep saying 'the current status' as it\n   said before, but a caller like your rebase-autostash will not be\n   saving the \"current\" status and would want to have a different\n   message.\n\nOther than that, the overfall structure of the patch looks OK to me.\n"},{"id":"217184","messageId":"7v7gj3os9a.fsf@alter.siamese.dyndns.org","threadId":"33806","inReplyTo":"1368449154-21882-6-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 5/6] stash: tweak error message in store_stash ()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-13T14:42:57Z","receivedAt":"2013-05-13T14:42:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> When the update-ref invocation fails, store_stash currently prints:\n>\n>     Cannot save the current status\n>\n> This is not very useful for diagnosing the problem.  Instead, print:\n>\n>     Cannot store 688268c4254ca5dc6e2effa83bae4f0dbbe75e5b\n>\n> so we can inspect the object and analyze why the update-ref failed.\n\nThis would break the error message for save_stash with your current\npatch series, wouldn't it?  I think this patch is a solution to a\nwrong problem.  As I already said in 4/6, store_stash should allow\nits caller to supply a customised error message.\n\n>\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\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 1d483f5..24d72fc 100755\n> --- a/git-stash.sh\n> +++ b/git-stash.sh\n> @@ -167,7 +167,7 @@ store_stash () {\n>  \t# Make sure the reflog for stash is kept.\n>  \t: >>\"$GIT_DIR/logs/$ref_stash\"\n>  \tgit update-ref -m \"$stash_msg\" $ref_stash $w_commit ||\n> -\tdie \"$(gettext \"Cannot save the current status\")\"\n> +\tdie \"$(gettext \"Cannot store $w_commit\")\"\n>  }\n>  \n>  save_stash () {\n"},{"id":"217192","messageId":"CALkWK0n_QEUhNUSTTgzBjQDMqC7W86P_C5QDE_X-P06GDuj6wg@mail.gmail.com","threadId":"33806","inReplyTo":"7vfvxrosnf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/6] stash: simplify option parser for create","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-13T14:59:06Z","receivedAt":"2013-05-13T14:59:06Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> The semi-user facing \"git stash create\" never was meant to take\n> anything but a message sentence and \"$*\" is the proper way to say\n> \"everything is meant for a single message (just like echo)\".\n> Changing it to \"$@\" will change the semantics in a big way.\n\nAh, I see.  As an interactive caller, it is impossible to set\n$untracked (I thought this was a mistake, but you're indicating that\nit's intentional).  Okay, I'll fix the patch and documentation.\n\nThanks.\n"},{"id":"217196","messageId":"CALkWK0kMj6q1rppzfxGWrthub4K_+C+jnvF=K4m2Fhk0pAnt7A@mail.gmail.com","threadId":"33806","inReplyTo":"7vbo8fosd9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/6] stash: introduce 'git stash store'","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-13T15:02:38Z","receivedAt":"2013-05-13T15:02:38Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n>  - The error message store_stash() gives should not be hardcoded in\n>    that function.\n>\n>    Save-stash may want to keep saying 'the current status' as it\n>    said before, but a caller like your rebase-autostash will not be\n>    saving the \"current\" status and would want to have a different\n>    message.\n\nMakes sense.\n\nThanks.\n"},{"id":"217202","messageId":"7v61ymop61.fsf@alter.siamese.dyndns.org","threadId":"33806","inReplyTo":"CALkWK0kMj6q1rppzfxGWrthub4K_+C+jnvF=K4m2Fhk0pAnt7A@mail.gmail.com","subject":"Re: [PATCH 4/6] stash: introduce 'git stash store'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-13T15:49:42Z","receivedAt":"2013-05-13T15:49:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> Junio C Hamano wrote:\n>>  - The error message store_stash() gives should not be hardcoded in\n>>    that function.\n>>\n>>    Save-stash may want to keep saying 'the current status' as it\n>>    said before, but a caller like your rebase-autostash will not be\n>>    saving the \"current\" status and would want to have a different\n>>    message.\n>\n> Makes sense.\n\nI think it is fine to have a \"default\" message to be used when\nstore_stash shell function is not given an optional error message.\n\nThe command line for it may become:\n\n\tgit stash store [-m <message>] [-e <error message>] $stash_sha1\n\nor something like that.\n"},{"id":"217208","messageId":"CALkWK0mQhzrndaEp28nOpLs5STq136PY8hntCpX5RUZKrdJVAA@mail.gmail.com","threadId":"33806","inReplyTo":"7v61ymop61.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/6] stash: introduce 'git stash store'","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-13T17:15:02Z","receivedAt":"2013-05-13T17:15:02Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n>         git stash store [-m <message>] [-e <error message>] $stash_sha1\n\n1. The message is NOT optional.  If the user says 'git stash store\nb8bb3fe9', what \"default message\" can we possibly put in?  There is\nabsolutely no context: no branch name, nothing.  So, the best we can\ndo is \"generic WIP\".  What is the point of putting in such a useless\nmessage?\n\n2. We have already determined that the command is NOT for end user\ninteractive use.  So, why do we need a default error message at all?\nThe last statement is an update-ref, and you can infer whether it\nsucceeded or failed from the exit status.\n\n3. Why are we designing a command-line interface?  git stash store\n\"$stash_sha1\" \"$message\" is sufficient for scripts, and there is\nabsolutely no point in parsing '-m', '-e', or any such thing.\n"},{"id":"217210","messageId":"7vppwun5zk.fsf@alter.siamese.dyndns.org","threadId":"33806","inReplyTo":"CALkWK0mQhzrndaEp28nOpLs5STq136PY8hntCpX5RUZKrdJVAA@mail.gmail.com","subject":"Re: [PATCH 4/6] stash: introduce 'git stash store'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-13T17:29:19Z","receivedAt":"2013-05-13T17:29:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> Junio C Hamano wrote:\n>>         git stash store [-m <message>] [-e <error message>] $stash_sha1\n> ...\n> 3. Why are we designing a command-line interface?  git stash store\n> \"$stash_sha1\" \"$message\" is sufficient for scripts, and there is\n> absolutely no point in parsing '-m', '-e', or any such thing.\n\n\"git stash store $stash_sha1 $message [ $error_message ]\" is\nadequate an internal API _for now_.\n\nI however suspect that you would regret later when you need more\ncustomization.  It already happened once for \"git merge\" when it was\nan internal API for \"git pull\" and it was painful to support saner\ninterface and the traditional one at the same time [*1*].\n\n[Footnote]\n\n*1* And no, don't even try to rewrite \"git merge\" call inside \"git\npull\" to use the modern style with \"-m <message>\"; you will likely\nbreak it (I've tried once and decided it was not worth the hassle).\n"},{"id":"217211","messageId":"CALkWK0nmDopEVArofKt42u5JHRBXmGiHP051jhMxtaihWLu95A@mail.gmail.com","threadId":"33806","inReplyTo":"7vppwun5zk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/6] stash: introduce 'git stash store'","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-13T18:09:06Z","receivedAt":"2013-05-13T18:09:06Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> I however suspect that you would regret later when you need more\n> customization.  It already happened once for \"git merge\" when it was\n> an internal API for \"git pull\" and it was painful to support saner\n> interface and the traditional one at the same time [*1*].\n\nOh god.\n\n  git-merge --stat --progress \"$merge_name\" HEAD 04c5b83c46760573\n\nWe made a design mistake at the command-level in merge.  This is at a\nsubcommand-level.\n\n1. Will git stash store ever be more than a one-liner?  Can you think\nof how this function could be larger?\n\n2. Will git stash store ever become an interactive command?  Isn't the\nwhole point of interactive stash something that operates on a\nworktree?  Why will I ever want to operate on a commit with stash,\ninteractively?\n\nWhile it is absolutely necessary to avoid calamities like the merge\ninvocation in git-pull.sh, we shouldn't be over-engineering either.\n"},{"id":"217212","messageId":"CALkWK0m=oz_U6kRbGAjdU=m5g_ZiJf016af-KpdEesm02CMCkQ@mail.gmail.com","threadId":"33806","inReplyTo":"7vppwun5zk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/6] stash: introduce 'git stash store'","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-13T18:15:37Z","receivedAt":"2013-05-13T18:15:37Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> *1* And no, don't even try to rewrite \"git merge\" call inside \"git\n> pull\" to use the modern style with \"-m <message>\"; you will likely\n> break it (I've tried once and decided it was not worth the hassle).\n\nThis falls in my basket of \"nice theoretical exercise\": a lot of work\nfor no tangible benefit ;)\n\nFootnote: I'm not saying that code is not important; you've seen me\narguing for beautiful implementations several times before, against\nall odds.\n"},{"id":"217214","messageId":"7vli7in2wq.fsf@alter.siamese.dyndns.org","threadId":"33806","inReplyTo":"CALkWK0nmDopEVArofKt42u5JHRBXmGiHP051jhMxtaihWLu95A@mail.gmail.com","subject":"Re: [PATCH 4/6] stash: introduce 'git stash store'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-13T18:35:49Z","receivedAt":"2013-05-13T18:35:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n>   git-merge --stat --progress \"$merge_name\" HEAD 04c5b83c46760573\n>\n> We made a design mistake at the command-level in merge.  This is at a\n> subcommand-level.\n>\n> 1. Will git stash store ever be more than a one-liner?  Can you think\n> of how this function could be larger?\n\nThat is not a valid question.  When Linus added the original\n\n    git merge 'message' HEAD $other_branch\n\nthere wasn't any merge strategy switch and all the other frills even\nenvisioned yet.  With what we know about Git today, even you could\nhave answered \"Will git merge be more involved compared to the\noriginal?\" 8 years ago, but without that, not even Linus nor I did\nanticipate that the command line interface would become a pain point\nwhen enhancing it.\n"},{"id":"217216","messageId":"CALkWK0nygD7dpwZkXYMuXHhVZpRzb968wAYq-xZrj=y7duL1ZQ@mail.gmail.com","threadId":"33806","inReplyTo":"7vli7in2wq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/6] stash: introduce 'git stash store'","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-13T18:45:45Z","receivedAt":"2013-05-13T18:45:45Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> That is not a valid question.\n\nI was just asking to see if you could think of something.  I just did:\nnamed stashes (each one has a different ref/ reflog) for internal use.\n\nSure, we'll go with the -m -e approach.\n"},{"id":"217221","messageId":"7v61ymn10i.fsf@alter.siamese.dyndns.org","threadId":"33806","inReplyTo":"CALkWK0nygD7dpwZkXYMuXHhVZpRzb968wAYq-xZrj=y7duL1ZQ@mail.gmail.com","subject":"Re: [PATCH 4/6] stash: introduce 'git stash store'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-13T19:16:45Z","receivedAt":"2013-05-13T19:16:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> Junio C Hamano wrote:\n>> That is not a valid question.\n>\n> I was just asking to see if you could think of something.  I just did:\n> named stashes (each one has a different ref/ reflog) for internal use.\n>\n> Sure, we'll go with the -m -e approach.\n\nThe whole point of my response is that it is not a valid approach to\ndecide to add (or not to add) a reasonable enhancement mechanism\nbuilt in from the beginning by asking \"what future enhancement do\nyou foresee today?\".  It is unclear if you got that point.\n\nAs long as the end result is to have something that does not paint\nus into a corner from which it will be painful to escape, I'll be\nhappy anyway.\n"},{"id":"217224","messageId":"CALkWK0nH598SGojvcYS0TdhA=5r9kUY6XTzOa3VWQwd85qs_Ug@mail.gmail.com","threadId":"33806","inReplyTo":"7v61ymn10i.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/6] stash: introduce 'git stash store'","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-13T19:25:25Z","receivedAt":"2013-05-13T19:25:25Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> The whole point of my response is that it is not a valid approach to\n> decide to add (or not to add) a reasonable enhancement mechanism\n> built in from the beginning by asking \"what future enhancement do\n> you foresee today?\".  It is unclear if you got that point.\n\nRight, got it.  The fact that we weren't able to foresee the merge UI\nproblem tells us that we're capable of repeating the mistake no matter\nhow much we think we've thought about it.\n"}]}