{"thread":{"id":"39431","subject":"[PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits","startedAt":"2015-05-26T21:38:37Z","lastAt":"2015-05-28T17:45:32Z","messageCount":24,"participants":["Galan Rémi","Eric Sunshine","Johannes Schindelin","Stephen Kelly","Matthieu Moy","Remi Galan Alfonso","Junio C Hamano","Stefan Beller","Philip Oakley"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"262166","messageId":"1432676318-22852-1-git-send-email-remi.galan-alfonso@ensimag.grenoble-inp.fr","threadId":"39431","inReplyTo":null,"subject":"[PATCH/RFC 1/2] git-rebase -i: Add key word \"drop\" to remove a commit","fromName":"Galan Rémi","fromEmail":"remi.galan-alfonso@ensimag.grenoble-inp.fr","sentAt":"2015-05-26T21:38:37Z","receivedAt":"2015-05-26T21:38:37Z","isPatch":true,"sender":{"key":"remi.galan-alfonso@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12509162?v=4"},"body":"Instead of removing a line to remove the commit, you can use the key\nword \"drop\" (just like \"pick\" or \"edit\"). It has the same effect as\ndeleting the line (removing the commit) except that you keep a visual\ntrace of your actions, allowing a better control and reducing the\npossibility of removing a commit by mistake.\n\nSigned-off-by: Galan Rémi \n---\n Documentation/git-rebase.txt | 3 +++\n git-rebase--interactive.sh | 4 ++++\n t/lib-rebase.sh | 4 ++--\n t/t3404-rebase-interactive.sh | 11 +++++++++++\n 4 files changed, 20 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 1d01baa..3cd2ef2 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -514,6 +514,9 @@ rebasing.\n If you just want to edit the commit message for a commit, replace the\n command \"pick\" with the command \"reword\".\n \n+If you want to remove a commit, replace the command \"pick\" by the\n+command \"drop\".\n+\n If you want to fold two or more commits into one, replace the command\n \"pick\" for the second and subsequent commits with \"squash\" or \"fixup\".\n If the commits had different authors, the folded commit will be\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex dc3133f..cb749e8 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -152,6 +152,7 @@ Commands:\n s, squash = use commit, but meld into previous commit\n f, fixup = like \"squash\", but discard this commit's log message\n x, exec = run command (the rest of the line) using shell\n+ d, drop = remove commit\n \n These lines can be re-ordered; they are executed from top to bottom.\n \n@@ -515,6 +516,9 @@ do_next () {\n \t\tdo_pick $sha1 \"$rest\"\n \t\trecord_in_rewritten $sha1\n \t\t;;\n+\tdrop|d)\n+\t\tmark_action_done\n+\t\t;;\n \treword|r)\n \t\tcomment_for_reflog reword\n \ndiff --git a/t/lib-rebase.sh b/t/lib-rebase.sh\nindex 6bd2522..fdbc900 100644\n--- a/t/lib-rebase.sh\n+++ b/t/lib-rebase.sh\n@@ -14,7 +14,7 @@\n # specified line.\n #\n # \" \" -- add a line with the specified command\n-# (\"squash\", \"fixup\", \"edit\", or \"reword\") and the SHA1 taken\n+# (\"squash\", \"fixup\", \"edit\", \"reword\" or \"drop\") and the SHA1 taken\n # from the specified line.\n #\n # \"exec_cmd_with_args\" -- add an \"exec cmd with args\" line.\n@@ -46,7 +46,7 @@ set_fake_editor () {\n \taction=pick\n \tfor line in $FAKE_LINES; do\n \t\tcase $line in\n-\t\tsquash|fixup|edit|reword)\n+\t\tsquash|fixup|edit|reword|drop)\n \t\t\taction=\"$line\";;\n \t\texec*)\n \t\t\techo \"$line\" | sed 's/_/ /g' >> \"$1\";;\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex ac429a0..1bad068 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1102,4 +1102,15 @@ test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' '\n \ttest $(git cat-file commit HEAD | sed -ne \\$p) = I\n '\n \n+test_expect_success 'drop' '\n+\tgit checkout master &&\n+\ttest_when_finished \"git checkout master\" &&\n+\tgit checkout -b dropBranchTest master &&\n+\tset_fake_editor &&\n+\tFAKE_LINES=\"1 drop 2 3 drop 4 5\" git rebase -i --root &&\n+\ttest E = $(git cat-file commit HEAD | sed -ne \\$p) &&\n+\ttest C = $(git cat-file commit HEAD^ | sed -ne \\$p) &&\n+\ttest A = $(git cat-file commit HEAD^^ | sed -ne \\$p)\n+'\n+\n test_done\n-- \n2.4.1.174.g28bfe8e\n"},{"id":"262163","messageId":"1432676318-22852-2-git-send-email-remi.galan-alfonso@ensimag.grenoble-inp.fr","threadId":"39431","inReplyTo":"1432676318-22852-1-git-send-email-remi.galan-alfonso@ensimag.grenoble-inp.fr","subject":"[PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits","fromName":"Galan Rémi","fromEmail":"remi.galan-alfonso@ensimag.grenoble-inp.fr","sentAt":"2015-05-26T21:38:38Z","receivedAt":"2015-05-26T21:38:38Z","isPatch":true,"sender":{"key":"remi.galan-alfonso@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12509162?v=4"},"body":"Check if commits were removed (i.e. a line was deleted) or dupplicated\n(e.g. the same commit is picked twice), can print warnings or abort\ngit rebase according to the value of the configuration variable\nrebase.checkLevel.\n\nAdd the configuration variable rebase.checkLevel.\n - When unset or set to \"IGNORED\", no checking is done.\n - When set to \"WARN\", the commits are checked, warnings are\n displayed but git rebase still proceeds.\n - When set to \"ERROR\", the commits are checked, warnings are\n displayed and the rebase is aborted.\n\nSigned-off-by: Galan Rémi \n---\n This part of the patch has no test yet, it is more for rfc.\n\n Documentation/config.txt | 8 +++++\n Documentation/git-rebase.txt | 5 +++\n git-rebase--interactive.sh | 76 ++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 89 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex d44bc85..2152e27 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2204,6 +2204,14 @@ rebase.autoStash::\n \tsuccessful rebase might result in non-trivial conflicts.\n \tDefaults to false.\n \n+rebase.checkLevel::\n+\tIf set to \"warn\", git rebase -i will print a warning if some\n+\tcommits are removed (i.e. a line was deleted) or if some\n+\tcommits appear more than one time (e.g. the same commit is\n+\tpicked twice), however the rebase will still proceed. If set\n+\tto \"error\", it will print the previous warnings and abort the\n+\trebase.\n+\n receive.advertiseAtomic::\n \tBy default, git-receive-pack will advertise the atomic push\n \tcapability to its clients. If you don't want to this capability\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 3cd2ef2..cb05cbb 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -213,6 +213,11 @@ rebase.autoSquash::\n rebase.autoStash::\n \tIf set to true enable '--autostash' option by default.\n \n+rebase.checkLevel::\n+\tIf set to \"warn\" print warnings about removed commits and\n+\tduplicated commits in interactive mode. If set to \"error\"\n+\tprint the warnings and abort the rebase. No check by default.\n+\n OPTIONS\n -------\n --onto ::\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex cb749e8..8a837ca 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -837,6 +837,80 @@ add_exec_commands () {\n \tmv \"$1.new\" \"$1\"\n }\n \n+# Print the list of the sha-1 of the commits\n+# from a todo list in a file.\n+# $1 : todo-file, $2 : outfile\n+todo_list_to_sha_list () {\n+\ttodo_list=$(git stripspace --strip-comments < \"$1\")\n+\ttemp_file=$(mktemp)\n+\techo \"$todo_list\" > \"$temp_file\"\n+\twhile read -r command sha1 rest < \"$temp_file\"\n+\tdo\n+\t\tcase \"$command\" in\n+\t\tx|\"exec\")\n+\t\t\t;;\n+\t\t*)\n+\t\t\techo \"$sha1\" >> \"$2\"\n+\t\t\t;;\n+\t\tesac\n+\t\tsed -i '1d' \"$temp_file\"\n+\tdone\n+\trm \"$temp_file\"\n+}\n+\n+# Check if the user dropped some commits by mistake\n+# or if there are two identical commits.\n+# Behaviour determined by .gitconfig.\n+check_commits () {\n+\tcheckLevel=$(git config --get rebase.checkLevel)\n+\tcheckLevel=${checkLevel:-\"IGNORE\"}\n+\t# To uppercase\n+\tcheckLevel=$(echo \"$checkLevel\" | tr '[:lower:]' '[:upper:]')\n+\n+\tcase \"$checkLevel\" in\n+\t\"WARN\"|\"ERROR\")\n+\t\ttodo_list_to_sha_list \"$todo\".backup \"$todo\".oldsha1\n+\t\ttodo_list_to_sha_list \"$todo\" \"$todo\".newsha1\n+\n+\t\tduplicates=$(sort \"$todo\".newsha1 | uniq -d)\n+\n+\t\techo \"$(sort -u \"$todo\".oldsha1)\" > \"$todo\".oldsha1\n+\t\techo \"$(sort -u \"$todo\".newsha1)\" > \"$todo\".newsha1\n+\t\tmissing=$(comm -2 -3 \"$todo\".oldsha1 \"$todo\".newsha1)\n+\n+\t\t# check missing commits\n+\t\tif ! test -z \"$missing\"\n+\t\tthen\n+\t\t\twarn \"Warning : some commits may have been dropped accidentally.\"\n+\t\t\twarn \"Dropped commits:\"\n+\t\t\twarn \"$missing\"\n+\t\t\twarn \"To avoid this message, use \\\"drop\\\" to explicitely remove a commit.\"\n+\t\t\twarn \"Use git --config rebase.checkLevel to change\"\n+\t\t\twarn \"the level of warnings (ignore,warn,error).\"\n+\t\t\twarn \"\"\n+\n+\t\t\tif test \"$checkLevel\" = \"ERROR\"\n+\t\t\tthen\n+\t\t\t\tdie_abort \"Rebase aborted due to dropped commits.\"\n+\t\t\tfi\n+\t\tfi\n+\n+\t\t# check duplicate commits\n+\t\tif ! test -z \"$duplicates\"\n+\t\tthen\n+\t\t\twarn \"Warning : some commits have been used twice:\"\n+\t\t\twarn \"$duplicates\"\n+\t\t\twarn \"\"\n+\t\tfi\n+\t\t;;\n+\t\"IGNORE\")\n+\t\t;;\n+\t*)\n+\t\twarn \"Unrecognized setting for option rebase.checkLevel in git rebase -i\"\n+\t\t;;\n+\tesac\n+}\n+\n # The whole contents of this file is run by dot-sourcing it from\n # inside a shell function. It used to be that \"return\"s we see\n # below were not inside any function, and expected to return\n@@ -1082,6 +1156,8 @@ has_action \"$todo\" ||\n \n expand_todo_ids\n \n+check_commits\n+\n test -d \"$rewritten\" || test -n \"$force_rebase\" || skip_unnecessary_picks\n \n GIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $onto_name\"\n-- \n2.4.1.174.g28bfe8e\n"},{"id":"262188","messageId":"CAPig+cThB1xC0B6wC29Bm0QUiPpGbkVe9j_Qu5LLdVq3XFMgoQ@mail.gmail.com","threadId":"39431","inReplyTo":"1432676318-22852-1-git-send-email-remi.galan-alfonso@ensimag.grenoble-inp.fr","subject":"Re: [PATCH/RFC 1/2] git-rebase -i: Add key word \"drop\" to remove a commit","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-26T22:52:15Z","receivedAt":"2015-05-26T22:52:15Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, May 26, 2015 at 5:38 PM, Galan Rémi\n wrote:\n> git-rebase -i: Add key word \"drop\" to remove a commit\n\n\"key word\" is unusual. More typical is \"keyword\". However, perhaps\n\"command\" might be even better. Also, custom on this project is not to\ncapitalize, so:\n\n git-rebase -i: add command \"drop\" to remove a commit\n\n> Instead of removing a line to remove the commit, you can use the key\n> word \"drop\" (just like \"pick\" or \"edit\"). It has the same effect as\n> deleting the line (removing the commit) except that you keep a visual\n> trace of your actions, allowing a better control and reducing the\n> possibility of removing a commit by mistake.\n\nNicely explained.\n\nDitto regarding \"key word\".\n\n> Signed-off-by: Galan Rémi \n> ---\n> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\n> index 1d01baa..3cd2ef2 100644\n> --- a/Documentation/git-rebase.txt\n> +++ b/Documentation/git-rebase.txt\n> @@ -514,6 +514,9 @@ rebasing.\n> If you just want to edit the commit message for a commit, replace the\n> command \"pick\" with the command \"reword\".\n>\n> +If you want to remove a commit, replace the command \"pick\" by the\n> +command \"drop\".\n\nI think the existing method of removing a commit merits mention here. Perhaps:\n\n To drop a commit, delete its line or replace the command\n \"pick\" with \"drop\".\n\nOr, if you want to emphasize \"drop\":\n\n To drop a commit, replace the command \"pick\" with \"drop\",\n or just delete its line.\n\n> If you want to fold two or more commits into one, replace the command\n> \"pick\" for the second and subsequent commits with \"squash\" or \"fixup\".\n> If the commits had different authors, the folded commit will be\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index dc3133f..cb749e8 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n"},{"id":"262189","messageId":"CAPig+cQJMSjS=fiwMHE93efSsa2QYQ8TphyyfcLg7kAXRi_+cw@mail.gmail.com","threadId":"39431","inReplyTo":"1432676318-22852-2-git-send-email-remi.galan-alfonso@ensimag.grenoble-inp.fr","subject":"Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-26T23:27:32Z","receivedAt":"2015-05-26T23:27:32Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, May 26, 2015 at 5:38 PM, Galan Rémi\n wrote:\n> git rebase -i: Warn removed or dupplicated commits\n\ns/dupplicated/duplicated/\n\nAlso, drop capitalization, and insert \"about\":\n\n git rebase -i: warn about removed or duplicated commits\n\n> Check if commits were removed (i.e. a line was deleted) or dupplicated\n\ns/dupplicated/duplicated/\n\n> (e.g. the same commit is picked twice), can print warnings or abort\n\ns/can/and/, I think.\n\n> git rebase according to the value of the configuration variable\n> rebase.checkLevel.\n>\n> Add the configuration variable rebase.checkLevel.\n> - When unset or set to \"IGNORED\", no checking is done.\n\ns/IGNORED/IGNORE/\n\n> - When set to \"WARN\", the commits are checked, warnings are\n> displayed but git rebase still proceeds.\n> - When set to \"ERROR\", the commits are checked, warnings are\n> displayed and the rebase is aborted.\n\nWhy uppercase for these names? Is there precedence for that? I think\nlowercase is more common.\n\n> Signed-off-by: Galan Rémi \n> ---\n> This part of the patch has no test yet, it is more for rfc.\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index d44bc85..2152e27 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -2204,6 +2204,14 @@ rebase.autoStash::\n> successful rebase might result in non-trivial conflicts.\n> Defaults to false.\n>\n> +rebase.checkLevel::\n> + If set to \"warn\", git rebase -i will print a warning if some\n> + commits are removed (i.e. a line was deleted) or if some\n> + commits appear more than one time (e.g. the same commit is\n> + picked twice), however the rebase will still proceed. If set\n> + to \"error\", it will print the previous warnings and abort the\n> + rebase.\n\nThe commit message talks about \"ignore\", but there is no mention here.\n\nAlso, what is the default behavior if not specified? That should be documented.\n\nFinally, this talks about lowercase \"warn\" and \"error\", whereas the\ncommit message uses upper case \"WARN\" and \"ERROR\", as does the code.\nWhy the inconsistency?\n\n> receive.advertiseAtomic::\n> By default, git-receive-pack will advertise the atomic push\n> capability to its clients. If you don't want to this capability\n> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\n> index 3cd2ef2..cb05cbb 100644\n> --- a/Documentation/git-rebase.txt\n> +++ b/Documentation/git-rebase.txt\n> @@ -213,6 +213,11 @@ rebase.autoSquash::\n> rebase.autoStash::\n> If set to true enable '--autostash' option by default.\n>\n> +rebase.checkLevel::\n> + If set to \"warn\" print warnings about removed commits and\n> + duplicated commits in interactive mode. If set to \"error\"\n> + print the warnings and abort the rebase. No check by default.\n\nDitto: Fails to mention \"ignore\".\n\n> OPTIONS\n> -------\n> --onto ::\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index cb749e8..8a837ca 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -837,6 +837,80 @@ add_exec_commands () {\n> mv \"$1.new\" \"$1\"\n> }\n>\n> +# Print the list of the sha-1 of the commits\n> +# from a todo list in a file.\n> +# $1 : todo-file, $2 : outfile\n> +todo_list_to_sha_list () {\n> + todo_list=$(git stripspace --strip-comments < \"$1\")\n> + temp_file=$(mktemp)\n> + echo \"$todo_list\" > \"$temp_file\"\n> + while read -r command sha1 rest < \"$temp_file\"\n\nOn this project it is typical to drop the space after redirection\noperators (<, >, >>), however, git-rebase--interactive.sh is filled\nwith both styles (space and no space after redirection). New code\nprobably ought to drop the space.\n\n> + do\n> + case \"$command\" in\n> + x|\"exec\")\n> + ;;\n> + *)\n> + echo \"$sha1\" >> \"$2\"\n> + ;;\n> + esac\n> + sed -i '1d' \"$temp_file\"\n> + done\n> + rm \"$temp_file\"\n> +}\n> +\n> +# Check if the user dropped some commits by mistake\n> +# or if there are two identical commits.\n> +# Behaviour determined by .gitconfig.\n> +check_commits () {\n> + checkLevel=$(git config --get rebase.checkLevel)\n> + checkLevel=${checkLevel:-\"IGNORE\"}\n\nMinor aside: Unnecessary quoting increases the noise level, thus\nmaking the code slightly more difficult to read. This could just as\nwell have been:\n\n checkLevel=${checkLevel:-IGNORE}\n\nThere are plenty of other places throughout this patch which exhibit\nthe same shortcoming, but I won't point them out individually.\n\n> + # To uppercase\n> + checkLevel=$(echo \"$checkLevel\" | tr '[:lower:]' '[:upper:]')\n\nIs there precedence elsewhere for recognizing uppercase and lowercase\nvariants of config values?\n\n> + case \"$checkLevel\" in\n> + \"WARN\"|\"ERROR\")\n> + todo_list_to_sha_list \"$todo\".backup \"$todo\".oldsha1\n> + todo_list_to_sha_list \"$todo\" \"$todo\".newsha1\n> +\n> + duplicates=$(sort \"$todo\".newsha1 | uniq -d)\n> +\n> + echo \"$(sort -u \"$todo\".oldsha1)\" > \"$todo\".oldsha1\n> + echo \"$(sort -u \"$todo\".newsha1)\" > \"$todo\".newsha1\n> + missing=$(comm -2 -3 \"$todo\".oldsha1 \"$todo\".newsha1)\n> +\n> + # check missing commits\n> + if ! test -z \"$missing\"\n\nIsn't \"! test -z\" just a verbose way of saying \"test -n\"?\n\n> + then\n> + warn \"Warning : some commits may have been dropped accidentally.\"\n> + warn \"Dropped commits:\"\n> + warn \"$missing\"\n> + warn \"To avoid this message, use \\\"drop\\\" to explicitely remove a commit.\"\n\ns/explicitely/explicitly/\n\n> + warn \"Use git --config rebase.checkLevel to change\"\n> + warn \"the level of warnings (ignore,warn,error).\"\n> + warn \"\"\n> +\n> + if test \"$checkLevel\" = \"ERROR\"\n> + then\n> + die_abort \"Rebase aborted due to dropped commits.\"\n> + fi\n> + fi\n> +\n> + # check duplicate commits\n> + if ! test -z \"$duplicates\"\n> + then\n> + warn \"Warning : some commits have been used twice:\"\n> + warn \"$duplicates\"\n> + warn \"\"\n> + fi\n\nShouldn't this case also 'die' when rebase.checkLevel is \"error\"? And,\nwhy doesn't the user get advice about configuring rebase.checkLevel in\nthis case?\n\nIn fact, the current logic flow seems a bit borked. I would have\nexpected it to be more like this:\n\n if test -n \"$missing\"\n then\n ...warn about accidental drops...\n fi\n\n if test -n \"$duplicates\"\n then\n ...warn about accidental duplicates...\n fi\n\n if test -n \"$missing$duplicates\"\n then\n ...show advice about configuring rebase.checkLevel...\n\n if test $checkLevel = ERROR\n then\n die_abort \"...\"\n fi\n fi\n\n> + ;;\n> + \"IGNORE\")\n> + ;;\n> + *)\n> + warn \"Unrecognized setting for option rebase.checkLevel in git rebase -i\"\n\nThis message might be more useful if it mentioned the actual unrecognized value.\n\n> + ;;\n> + esac\n> +}\n> +\n> # The whole contents of this file is run by dot-sourcing it from\n> # inside a shell function. It used to be that \"return\"s we see\n> # below were not inside any function, and expected to return\n> @@ -1082,6 +1156,8 @@ has_action \"$todo\" ||\n>\n> expand_todo_ids\n>\n> +check_commits\n> +\n> test -d \"$rewritten\" || test -n \"$force_rebase\" || skip_unnecessary_picks\n>\n> GIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $onto_name\"\n> --\n> 2.4.1.174.g28bfe8e\n"},{"id":"262199","messageId":"c78cd2ac17333a2e70d1113d95495c41@www.dscho.org","threadId":"39431","inReplyTo":"1432676318-22852-1-git-send-email-remi.galan-alfonso@ensimag.grenoble-inp.fr","subject":"Re: [PATCH/RFC 1/2] git-rebase -i: Add key word \"drop\" to remove a commit","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-27T06:28:42Z","receivedAt":"2015-05-27T06:28:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Rémi,\n\nOn 2015-05-26 23:38, Galan Rémi wrote:\n> Instead of removing a line to remove the commit, you can use the key\n> word \"drop\" (just like \"pick\" or \"edit\"). It has the same effect as\n> deleting the line (removing the commit) except that you keep a visual\n> trace of your actions, allowing a better control and reducing the\n> possibility of removing a commit by mistake.\n\nPlease note that you can already just comment-out the line if you need to keep a visual trace.\n\nAlternatively, you can replace the `pick` command by `noop`.\n\nIf you really need the `drop` command (with which I am not 100% happy because I already envisage users appending a `drop A` to an edit script \"pick A; pick B; pick C\" and expecting A *not to be picked*), then it is better to just add the `drop` part to the already existing `noop` clause:\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex f7deeb0..8355be8 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -489,7 +489,7 @@ do_next () {\n \trm -f \"$msg\" \"$author_script\" \"$amend\" || exit\n \tread -r command sha1 rest < \"$todo\"\n \tcase \"$command\" in\n-\t\"$comment_char\"*|''|noop)\n+\t\"$comment_char\"*|''|noop|drop)\n \t\tmark_action_done\n \t\t;;\n \tpick|p)\n\nCiao,\nJohannes\n"},{"id":"262220","messageId":"loom.20150527T105315-517@post.gmane.org","threadId":"39431","inReplyTo":"1432676318-22852-2-git-send-email-remi.galan-alfonso@ensimag.grenoble-inp.fr","subject":"Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits","fromName":"Stephen Kelly","fromEmail":"steveire@gmail.com","sentAt":"2015-05-27T08:54:55Z","receivedAt":"2015-05-27T08:54:55Z","isPatch":true,"sender":{"key":"steveire@gmail.com","avatar":null},"body":"Galan Rémi ensimag.grenoble-inp.fr> writes:\n\n> \n> Check if commits were removed (i.e. a line was deleted) or dupplicated\n> (e.g. the same commit is picked twice), can print warnings or abort\n> git rebase according to the value of the configuration variable\n> rebase.checkLevel.\n\nI sometimes duplicate commits deliberately if I want to split a commit in\ntwo. I move a copy up and fix the conflict, and I know that I'll still get\nthe right thing later even if I make a mistake with the conflict resolution."},{"id":"262223","messageId":"vpqy4ka5jyp.fsf@anie.imag.fr","threadId":"39431","inReplyTo":"loom.20150527T105315-517@post.gmane.org","subject":"Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-05-27T11:38:22Z","receivedAt":"2015-05-27T11:38:22Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Stephen Kelly writes:\n\n> Galan Rémi ensimag.grenoble-inp.fr> writes:\n>\n>> \n>> Check if commits were removed (i.e. a line was deleted) or dupplicated\n>> (e.g. the same commit is picked twice), can print warnings or abort\n>> git rebase according to the value of the configuration variable\n>> rebase.checkLevel.\n>\n> I sometimes duplicate commits deliberately if I want to split a commit in\n> two. I move a copy up and fix the conflict, and I know that I'll still get\n> the right thing later even if I make a mistake with the conflict\n> resolution.\n\nThe more I think about it, the more I think we should either not warn at\nall on duplicate commits, or have a separate config variable.\n\nIt's rare to duplicate by mistake, and when you do so, it's already easy\nto notice: you get conflicts, and you can git rebase --skip the second\noccurence. Accidentally dropped commits are another story: it's rather\neasy to cut-and-forget-to-paste, and the consequence currently is silent\ndata loss ...\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"262225","messageId":"579982712.39028.1432732759119.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"39431","inReplyTo":"CAPig+cQJMSjS=fiwMHE93efSsa2QYQ8TphyyfcLg7kAXRi_+cw@mail.gmail.com","subject":"Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits","fromName":"Remi Galan Alfonso","fromEmail":"remi.galan-alfonso@ensimag.grenoble-inp.fr","sentAt":"2015-05-27T13:19:19Z","receivedAt":"2015-05-27T13:19:19Z","isPatch":true,"sender":{"key":"remi.galan-alfonso@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12509162?v=4"},"body":"Thank you for reviewing the code. \n\nEric Sunshine writes:\n> > + # To uppercase\n> > + checkLevel=$(echo \"$checkLevel\" | tr '[:lower:]' '[:upper:]')\n> \n> Is there precedence elsewhere for recognizing uppercase and lowercase\n> variants of config values?\n\nIt seems to be commonly used when parsing options in the C files\nthrough strcasecmp. For exemple, in config.c:818 :\nif (!strcmp(var, \"core.safecrlf\")) {\n\tif (value && !strcasecmp(value, \"warn\")) {\n\t\t[...]\nHowever we didn't see any precedence in shell files. Do you think we\nshould remove it?\n"},{"id":"262227","messageId":"1660192861.39291.1432732981552.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"39431","inReplyTo":"CAPig+cQJMSjS=fiwMHE93efSsa2QYQ8TphyyfcLg7kAXRi_+cw@mail.gmail.com","subject":"Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits","fromName":"Remi Galan Alfonso","fromEmail":"remi.galan-alfonso@ensimag.grenoble-inp.fr","sentAt":"2015-05-27T13:23:01Z","receivedAt":"2015-05-27T13:23:01Z","isPatch":true,"sender":{"key":"remi.galan-alfonso@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12509162?v=4"},"body":"Eric Sunshine writes:\n> Shouldn't this case also 'die' when rebase.checkLevel is \"error\"? And,\n> why doesn't the user get advice about configuring rebase.checkLevel in\n> this case?\nStephen Kelly writes:\n> I sometimes duplicate commits deliberately if I want to split a commit in\n> two.\nMatthieu Moy writes:\n> The more I think about it, the more I think we should either not warn at\n> all on duplicate commits, or have a separate config variable.\nPut in common because two config variables would have an effect on the\n'die' and advise part.\n\nIn this patch we didn't put the 'die' in the duplicate commit part\nsince there was only one config variable and there are cases where the\nuser might want to duplicate commits.\n\nAfter the code reviewing of Eric Sunshine and Stephen Kelly, we also\ncame to the conclusion that we should use two config variables, one\nabout missing commits and the other about duplicate commits.\n\nThis way if you deliberately want to use duplicate commits, you can\njust set the value to 'ignore' for duplicate commits and still have\n'warn'/'error' for missing commits. Moreover, each part would have its\n'die' depending on the value of the corresponding config variable.\n"},{"id":"262238","messageId":"1506177855.44397.1432738386768.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"39431","inReplyTo":"c78cd2ac17333a2e70d1113d95495c41@www.dscho.org","subject":"Re: [PATCH/RFC 1/2] git-rebase -i: Add key word \"drop\" to remove a commit","fromName":"Remi Galan Alfonso","fromEmail":"remi.galan-alfonso@ensimag.grenoble-inp.fr","sentAt":"2015-05-27T14:53:06Z","receivedAt":"2015-05-27T14:53:06Z","isPatch":true,"sender":{"key":"remi.galan-alfonso@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12509162?v=4"},"body":"Thank you for reviewing the code.\n\nJohannes Schindelin writes:\n> Please note that you can already just comment-out the line if you need to keep a visual trace.\n> \n> Alternatively, you can replace the `pick` command by `noop`.\n> \n> If you really need the `drop` command (with which I am not 100%\n> happy because I already envisage users appending a `drop A` to an\n> edit script \"pick A; pick B; pick C\" and expecting A *not to be\n> picked*)\n\nIt is true that drop has the same effect as noop or commenting,\nhowever we thought that drop is more understandable for average users of\ngit. Moreover when using git rebase -i, the 'help' displayed below the\nlist of commits doesn't mention neither the noop command nor the\neffect of commenting the line (though considering what removing a line\ndoes, it can be easily deduced).\n\nThe drop command was inspired by the drop command from histedit in\nmercurial.\n\nIt also has some effects with the second part of this patch (checks\nremoved and/or duplicated commits): if you comment the line, the\ncommit will be considered as removed, thus ending in a warning if the\nconfig variable is set to warn/error; however this problem won't\nappear with noop.\n"},{"id":"262239","messageId":"vpq1ti23vva.fsf@anie.imag.fr","threadId":"39431","inReplyTo":"1506177855.44397.1432738386768.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH/RFC 1/2] git-rebase -i: Add key word \"drop\" to remove a commit","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-05-27T15:04:09Z","receivedAt":"2015-05-27T15:04:09Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Remi Galan Alfonso writes:\n\n> It also has some effects with the second part of this patch (checks\n> removed and/or duplicated commits): if you comment the line, the\n> commit will be considered as removed, thus ending in a warning if the\n> config variable is set to warn/error; however this problem won't\n> appear with noop.\n\nIndeed, that's the whole point of having a \"drop\" command.\n\nAs an advice for your next submission: use \"git send-email\n--cover-letter\", and explain the overall idea before the patches.\n\nI personally prefer \"drop\" to \"noop\" as a command name: I understand\n\"noop\" as a command without argument (useful to say \"this is actually an\nempty list of commands, not an empty file to ask rebase to abort\"), but\nI find it weird to write\n\nnoop \n\nAs Remi wrote, the inspiration comes from Mercurial. Perhaps we should\nask on the mercurial ml how happy they are with the name.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"262243","messageId":"CAPig+cTTDtL+zkjU0iasN2+q=C0P8npEVOuyHBaUN4cFB4ibZQ@mail.gmail.com","threadId":"39431","inReplyTo":"579982712.39028.1432732759119.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-27T17:41:10Z","receivedAt":"2015-05-27T17:41:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, May 27, 2015 at 9:19 AM, Remi Galan Alfonso\n<remi.galan-alfonso@ensimag.grenoble-inp.fr> wrote:\n> Eric Sunshine<sunshine@sunshineco.com> writes:\n>> > + # To uppercase\n>> > + checkLevel=$(echo \"$checkLevel\" | tr '[:lower:]' '[:upper:]')\n>>\n>> Is there precedence elsewhere for recognizing uppercase and lowercase\n>> variants of config values?\n>\n> It seems to be commonly used when parsing options in the C files\n> through strcasecmp. For exemple, in config.c:818 :\n> if (!strcmp(var, \"core.safecrlf\")) {\n> if (value && !strcasecmp(value, \"warn\")) {\n> [...]\n> However we didn't see any precedence in shell files. Do you think we\n> should remove it?\n\nPrecedence in C code is good enough for me, and it makes sense for\nyour new code to follow suit by being insensitive to case (as you have\nalready done).\n\nHowever, it would be a good idea to be consistent in your use of\nuppercase/lowercase in the commit message, documentation, and code,\nrather than having a mix. I'd suggest sticking with lowercase\nthroughout since lowercase is more commonly used in the codebase (and\njust easier to read).\n"},{"id":"262249","messageId":"xmqq1ti1n825.fsf@gitster.dls.corp.google.com","threadId":"39431","inReplyTo":"579982712.39028.1432732759119.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-27T19:18:10Z","receivedAt":"2015-05-27T19:18:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Remi Galan Alfonso <remi.galan-alfonso@ensimag.grenoble-inp.fr>\nwrites:\n\n> Thank you for reviewing the code. \n>\n> Eric Sunshine<sunshine@sunshineco.com> writes:\n>> > + # To uppercase\n>> > + checkLevel=$(echo \"$checkLevel\" | tr '[:lower:]' '[:upper:]')\n>> \n>> Is there precedence elsewhere for recognizing uppercase and lowercase\n>> variants of config values?\n>\n> It seems to be commonly used when parsing options in the C files\n> through strcasecmp. For exemple, in config.c:818 :\n> if (!strcmp(var, \"core.safecrlf\")) {\n> \tif (value && !strcasecmp(value, \"warn\")) {\n> \t\t[...]\n> However we didn't see any precedence in shell files. Do you think we\n> should remove it?\n\nI think there is a difference between (silently) accepting just to\nbe lenient and documenting and advocating mixed case uses.\n\nPersonally, I'd rather not to see gratuitous flexibility to allow\nthe same thing spelled in 47 different ways for no good reason.\n"},{"id":"262250","messageId":"xmqqwpztltei.fsf@gitster.dls.corp.google.com","threadId":"39431","inReplyTo":"vpqy4ka5jyp.fsf@anie.imag.fr","subject":"Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-27T19:20:05Z","receivedAt":"2015-05-27T19:20:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Stephen Kelly <steveire@gmail.com> writes:\n>\n>> Galan Rémi <remi.galan-alfonso <at> ensimag.grenoble-inp.fr> writes:\n>>\n>>> \n>>> Check if commits were removed (i.e. a line was deleted) or dupplicated\n>>> (e.g. the same commit is picked twice), can print warnings or abort\n>>> git rebase according to the value of the configuration variable\n>>> rebase.checkLevel.\n>>\n>> I sometimes duplicate commits deliberately if I want to split a commit in\n>> two. I move a copy up and fix the conflict, and I know that I'll still get\n>> the right thing later even if I make a mistake with the conflict\n>> resolution.\n>\n> The more I think about it, the more I think we should either not warn at\n> all on duplicate commits, or have a separate config variable.\n\nYeah, I'd say we shouldn't warn, without configuration to keep\nthings simple.\n\n>\n> It's rare to duplicate by mistake, and when you do so, it's already easy\n> to notice: you get conflicts, and you can git rebase --skip the second\n> occurence. Accidentally dropped commits are another story: it's rather\n> easy to cut-and-forget-to-paste, and the consequence currently is silent\n> data loss ...\n"},{"id":"262251","messageId":"xmqqsiahltbu.fsf@gitster.dls.corp.google.com","threadId":"39431","inReplyTo":"vpq1ti23vva.fsf@anie.imag.fr","subject":"Re: [PATCH/RFC 1/2] git-rebase -i: Add key word \"drop\" to remove a commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-27T19:21:41Z","receivedAt":"2015-05-27T19:21:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> I find it weird to write\n>\n> noop <sha1> <title>\n\nTrue, but then it can be spelled\n\n # <sha1> <title>\n\ntoo, so do we still want 'drop'? Unless we have a strong reason to\nbelieve migrants from Hg cannot be (re)trained, personally, I'd feel\nthat we do not need this 'drop' thing.\n"},{"id":"262253","messageId":"vpq8uc9yfdp.fsf@anie.imag.fr","threadId":"39431","inReplyTo":"xmqqsiahltbu.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC 1/2] git-rebase -i: Add key word \"drop\" to remove a commit","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-05-27T19:44:34Z","receivedAt":"2015-05-27T19:44:34Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n>\n>> I find it weird to write\n>>\n>> noop <sha1> <title>\n>\n> True, but then it can be spelled\n>\n> # <sha1> <title>\n\nI do find it weird too. \"#\" means \"comment\", which means \"do as if it\nwas not there\" to me. And in this case it does change the semantics once\nyou activate the safety feature: error out without the \"# <sha1>\n<title>\", rebase dropping the commit if the comment is present.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"262267","messageId":"xmqq382hlpwt.fsf@gitster.dls.corp.google.com","threadId":"39431","inReplyTo":"vpq8uc9yfdp.fsf@anie.imag.fr","subject":"Re: [PATCH/RFC 1/2] git-rebase -i: Add key word \"drop\" to remove a commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-27T20:35:30Z","receivedAt":"2015-05-27T20:35:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n>>\n>>> I find it weird to write\n>>>\n>>> noop <sha1> <title>\n>>\n>> True, but then it can be spelled\n>>\n>> # <sha1> <title>\n>\n> I do find it weird too. \"#\" means \"comment\", which means \"do as if it\n> was not there\" to me. And in this case it does change the semantics once\n> you activate the safety feature: error out without the \"# <sha1>\n> <title>\", rebase dropping the commit if the comment is present.\n\nWell, I do not agree with the premise that \"A line was removed, the\nuser may have made a mistake, we need to warn about it\" is a good\nidea in the first place. Removing an insn that is not wanted has\nbeen the way to skip and not replay a change from the beginning of\nthe time, and users shouldn't be trained into thinking that somehow\nis a bad practice by having such an option that warns.\n"},{"id":"262283","messageId":"CAGZ79kansAUWsjBsBznqaxRFeN3uF1u2hUZgO8b+OjOw8SKsUw@mail.gmail.com","threadId":"39431","inReplyTo":"xmqq382hlpwt.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC 1/2] git-rebase -i: Add key word \"drop\" to remove a commit","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-05-27T21:47:47Z","receivedAt":"2015-05-27T21:47:47Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, May 27, 2015 at 1:35 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n>\n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n>>>\n>>>> I find it weird to write\n>>>>\n>>>> noop <sha1> <title>\n>>>\n>>> True, but then it can be spelled\n>>>\n>>> # <sha1> <title>\n>>\n>> I do find it weird too. \"#\" means \"comment\", which means \"do as if it\n>> was not there\" to me. And in this case it does change the semantics once\n>> you activate the safety feature: error out without the \"# <sha1>\n>> <title>\", rebase dropping the commit if the comment is present.\n>\n> Well, I do not agree with the premise that \"A line was removed, the\n> user may have made a mistake, we need to warn about it\" is a good\n> idea in the first place. Removing an insn that is not wanted has\n> been the way to skip and not replay a change from the beginning of\n> the time, and users shouldn't be trained into thinking that somehow\n> is a bad practice by having such an option that warns.\n\nTalking about ideas:\nI sometimes have the wrong branch checked out when doing a small\nfixup commit. So I want to drop that patch from the current branch\nand apply it to another branch. Maybe an instruction like\ncherry-pick-to-branch-(and-do-not-apply-here) would help me there.\n\nOn the other hand I do understand the reasoning for having more\nsafety features in rebase as that exposes lots of power and many people\nfind the power a bit daunting.\n\nSo maybe you don't want to check the rebase instructions, but rather\nafter the fact, when the rebase is done:\n\n$ git rebase -i origin/master\nSuccessfully rebased and updated refs/heads/mytopic\nRebased the following commits:\n 0e33744 Document protocol version 2\n 6b6e3a7 t5544: add a test case for the new protocol\n d6aff73 transport: get_refs_via_connect exchanges capabilities before refs.\n cbb6089 transport: connect_setup appends protocol version number\n 0b86fa1 fetch-pack: use the configured transport protocol\n 23ed0ff remote.h: add get_remote_capabilities, request_capabilities\n e18b6dc transport: add infrastructure to support a protocol version number\n fd8d40d upload-pack-2: Implement the version 2 of upload-pack\n bf781ae upload-pack: move capabilities out of send_ref\n 4c9cb59 upload-pack: make client capability parsing code a separate function\nDropped the following commits:\n deadbee upload-pack: only accept capabilities on the first \"want\" line\nNew commits: (due to rewording, double picking, etc)\n c0ffee1 More Documentation\n\nI'd guess you would construct the information from the reflog\n(The line before \"rebase -i (start)\" in the reflog) delta'd against HEAD,\nso it's a crude incantation of git log maybe?\n\nAlso we need to turn this off for the power users, though I'd welcome if\nwe'd make it default on in git 3. (Being maximally verbose is good for new\nusers I assume, and turning it off is easy for advanced folks, so we can do\nthat for all porcelain commands?)\n\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"262301","messageId":"68A62E92218D4FD4A71EA138C9F3725A@PhilipOakley","threadId":"39431","inReplyTo":"xmqqsiahltbu.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC 1/2] git-rebase -i: Add key word \"drop\" to remove a commit","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2015-05-27T23:42:48Z","receivedAt":"2015-05-27T23:42:48Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" <gitster@pobox.com>\n> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n>\n>> I find it weird to write\n>>\n>> noop <sha1> <title>\n>\n> True, but then it can be spelled\n>\n> # <sha1> <title>\n>\n> too, so do we still want 'drop'? Unless we have a strong reason to\n> believe migrants from Hg cannot be (re)trained, personally, I'd feel\n> that we do not need this 'drop' thing.\n>\nTo me, the addition of \"drop\" would be a better completion of the list \nof action verbs for 'normal' users.\n\nTraining/Retraining users to use atypical techniques is a never ending \ntask, so making drop a synonym for the existing noop appeals to my \nexperience of users (of all sorts of tools, including personal \nexperience ;-).\n\n--\nPhilip \n"},{"id":"262308","messageId":"1388345544.70438.1432799047393.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"39431","inReplyTo":"xmqq1ti1n825.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits","fromName":"Remi Galan Alfonso","fromEmail":"remi.galan-alfonso@ensimag.grenoble-inp.fr","sentAt":"2015-05-28T07:44:07Z","receivedAt":"2015-05-28T07:44:07Z","isPatch":true,"sender":{"key":"remi.galan-alfonso@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12509162?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n> I think there is a difference between (silently) accepting just to\n> be lenient and documenting and advocating mixed case uses.\n> \n> Personally, I'd rather not to see gratuitous flexibility to allow\n> the same thing spelled in 47 different ways for no good reason.\n\nIt was more of a mistake on our part rather than actually wanting to\ndocument mixed case uses.\n\nIn the v2 of the patch (not sent to the mailing list yet since we want\nto take into consideration the conclusion of this discussion before)\nit is entirely in lower case in both the documentation and the code\nwhile we silently allow upper and mixed case.\n"},{"id":"262340","messageId":"xmqqfv6giqyu.fsf@gitster.dls.corp.google.com","threadId":"39431","inReplyTo":"1388345544.70438.1432799047393.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-28T16:53:13Z","receivedAt":"2015-05-28T16:53:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Remi Galan Alfonso <remi.galan-alfonso@ensimag.grenoble-inp.fr>\nwrites:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>> I think there is a difference between (silently) accepting just to\n>> be lenient and documenting and advocating mixed case uses.\n>> \n>> Personally, I'd rather not to see gratuitous flexibility to allow\n>> the same thing spelled in 47 different ways for no good reason.\n>\n> It was more of a mistake on our part rather than actually wanting to\n> document mixed case uses.\n>\n> In the v2 of the patch (not sent to the mailing list yet since we want\n> to take into consideration the conclusion of this discussion before)\n> it is entirely in lower case in both the documentation and the code\n> while we silently allow upper and mixed case.\n\nUnderstood; I am not sold on the whole \"warning\" business, though.\n\nI think I saw you did 'tr [:upper:]' or something like that in the\npatch; we tend to avoid [:class:] and [=equiv=] when not needed,\nunless we know that the matching engine used supports them (i.e. it\nis OK to use them in Perl scripts and it is OK to feed them to the\nwildmatch-based matcher in Git itself, but not in general shell\nscripts). As the values can all be represented in US-ASCII, it\nshould be sufficient to do \"tr 'A-Z' 'a-z'\", I would think.\n"},{"id":"262341","messageId":"f5ed9832bba0381314d01fba13e20667@www.dscho.org","threadId":"39431","inReplyTo":"CAGZ79kansAUWsjBsBznqaxRFeN3uF1u2hUZgO8b+OjOw8SKsUw@mail.gmail.com","subject":"Re: [PATCH/RFC 1/2] git-rebase -i: Add key word \"drop\" to remove a commit","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-28T17:06:28Z","receivedAt":"2015-05-28T17:06:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn 2015-05-27 23:47, Stefan Beller wrote:\n> On Wed, May 27, 2015 at 1:35 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Talking about ideas:\n> I sometimes have the wrong branch checked out when doing a small\n> fixup commit. So I want to drop that patch from the current branch\n> and apply it to another branch. Maybe an instruction like\n> cherry-pick-to-branch-(and-do-not-apply-here) would help me there.\n\nOh, is it wish-anything time? *claps-his-hands*\n\nI would wish for a graphical tool which visualizes the commit graph in a visually pleasing manner, where I can select one or more commits and drop them onto a commit in the graph, and then it goes and magically cherry-picks-and-drops them.\n\n:-)\n\nCiao,\nDscho\n"},{"id":"262344","messageId":"CAGZ79ka-Y2-2j=3WmoNmKG9=8JPtDZHGxPkkBkR6q-HYmHEYJw@mail.gmail.com","threadId":"39431","inReplyTo":"f5ed9832bba0381314d01fba13e20667@www.dscho.org","subject":"Re: [PATCH/RFC 1/2] git-rebase -i: Add key word \"drop\" to remove a commit","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-05-28T17:12:19Z","receivedAt":"2015-05-28T17:12:19Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, May 28, 2015 at 10:06 AM, Johannes Schindelin\n<johannes.schindelin@gmx.de> wrote:\n> Hi Stefan,\n>\n> On 2015-05-27 23:47, Stefan Beller wrote:\n>> On Wed, May 27, 2015 at 1:35 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Talking about ideas:\n>> I sometimes have the wrong branch checked out when doing a small\n>> fixup commit. So I want to drop that patch from the current branch\n>> and apply it to another branch. Maybe an instruction like\n>> cherry-pick-to-branch-(and-do-not-apply-here) would help me there.\n>\n> Oh, is it wish-anything time? *claps-his-hands*\n\nMaybe my wording was bad, sorry about that.\nI think throwing around ideas (which are closely related to what\nis trying to be accomplished here IMHO) is not necessarily bad,\nbut the exchange of ideas helps in understanding the issue better\n(\"I like your idea as I have not thought about it that way.\", \"What about\nuse case X\", Your idea is nuts because Y\")\n\n>\n> I would wish for a graphical tool which visualizes the commit graph in a\n> visually pleasing manner, where I can select one or more commits and drop\n> them onto a commit in the graph, and then it goes and magically cherry-picks-and-drops them.\n\nDrag and Drop, I get it. ;)\n\nAdditionally, if dropped on an unnamed branch, it should come up with\na reasonable\nnew branch name.\n\n>\n> :-)\n>\n> Ciao,\n> Dscho\n>\n"},{"id":"262351","messageId":"vpqpp5ksiir.fsf@anie.imag.fr","threadId":"39431","inReplyTo":"f5ed9832bba0381314d01fba13e20667@www.dscho.org","subject":"Re: [PATCH/RFC 1/2] git-rebase -i: Add key word \"drop\" to remove a commit","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-05-28T17:45:32Z","receivedAt":"2015-05-28T17:45:32Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n\n> Hi Stefan,\n>\n> On 2015-05-27 23:47, Stefan Beller wrote:\n>> On Wed, May 27, 2015 at 1:35 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Talking about ideas:\n>> I sometimes have the wrong branch checked out when doing a small\n>> fixup commit. So I want to drop that patch from the current branch\n>> and apply it to another branch. Maybe an instruction like\n>> cherry-pick-to-branch-(and-do-not-apply-here) would help me there.\n>\n> Oh, is it wish-anything time? *claps-his-hands*\n>\n> I would wish for a graphical tool which visualizes the commit graph in\n> a visually pleasing manner, where I can select one or more commits and\n> drop them onto a commit in the graph, and then it goes and magically\n> cherry-picks-and-drops them.\n\nYou need to argue a bit more to convince my students to schedule this\nfor the end of their projects ;-).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}