{"thread":{"id":"47485","subject":"[PATCH 0/5] A couple of sequencer cleanups","startedAt":"2017-12-22T23:55:44Z","lastAt":"2017-12-27T22:19:53Z","messageCount":8,"participants":["Johannes Schindelin","Junio C Hamano","Liam Beguin"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"335206","messageId":"cover.1513986836.git.johannes.schindelin@gmx.de","threadId":"47485","inReplyTo":null,"subject":"[PATCH 0/5] A couple of sequencer cleanups","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-12-22T23:55:33Z","receivedAt":"2017-12-22T23:55:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"While working on patches to teach `git rebase -i` to recreate branch topology\nproperly, i.e. replace the design of `--preserve-merges` by something much\nbetter, I stumbled across a couple of issues that I thought I should fix on the\nway.\n\nThe patches are based on lb/rebase-i-short-command-names, mainly because the\nnew `--recreate-merges` patches benefit from the patch \"rebase -i: update\nfunctions to use a flags parameter\", and I was too lazy to resolve the merge\nconflicts while rebasing the patch \"sequencer: do not invent whitespace when\ntransforming OIDs\" to the current `master` branch.\n\nOh, and by the way, the `--recreate-merges` feature already works. I used it\nto develop the patches themselves. I do not have time to pass one last time\nover them, so they'll have to wait for next year to see the Git mailing list.\nIf anyone wants to have a look over them, play with them, or even wants to\nreview the patches:\n\n\thttps://github.com/git/git/compare/master...dscho:sequencer-shears\n\n\nJohannes Schindelin (5):\n rebase: do not continue when the todo list generation failed\n sequencer: strip bogus LF at end of error messages\n sequencer: remove superfluous conditional\n sequencer: report when noop has an argument\n sequencer: do not invent whitespace when transforming OIDs\n\n git-rebase--interactive.sh | 3 ++-\n sequencer.c | 30 ++++++++++++++++++------------\n 2 files changed, 20 insertions(+), 13 deletions(-)\n\n\nbase-commit: 1795993488bef1b48e4224db096e9d12df075db2\nBased-On: lb/rebase-i-short-command-names at https://github.com/dscho/git\nFetch-Base-Via: git fetch https://github.com/dscho/git lb/rebase-i-short-command-names\nPublished-As: https://github.com/dscho/git/releases/tag/sequencer-cleanups-v1\nFetch-It-Via: git fetch https://github.com/dscho/git sequencer-cleanups-v1\n-- \n2.15.1.windows.2\n\n"},{"id":"335207","messageId":"834ea65b997a052dc47f1bcda5d95865d79a20c9.1513986836.git.johannes.schindelin@gmx.de","threadId":"47485","inReplyTo":"cover.1513986836.git.johannes.schindelin@gmx.de","subject":"[PATCH 1/5] rebase: do not continue when the todo list generation failed","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-12-22T23:55:38Z","receivedAt":"2017-12-22T23:55:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This is a *really* long-standing bug. As a matter of fact, this bug has\nbeen with us from the very beginning of `rebase -i`: 1b1dce4bae7 (Teach\nrebase an interactive mode, 2007-06-25), where the output of `rev-list`\nwas piped to `sed` (and any failure of the `rev-list` process would go\ncompletely undetected).\n\nSigned-off-by: Johannes Schindelin \n---\n git-rebase--interactive.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex e3f5a0abf3c..b7f95672bd9 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -893,7 +893,8 @@ fi\n if test t != \"$preserve_merges\"\n then\n \tgit rebase--helper --make-script ${keep_empty:+--keep-empty} \\\n-\t\t$revisions ${restrict_revision+^$restrict_revision} >\"$todo\"\n+\t\t$revisions ${restrict_revision+^$restrict_revision} >\"$todo\" ||\n+\tdie \"$(gettext \"Could not generate todo list\")\"\n else\n \tformat=$(git config --get rebase.instructionFormat)\n \t# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse\n-- \n2.15.1.windows.2\n\n\n"},{"id":"335208","messageId":"c4343a241548152f99e3f90162c8539ba4f993e5.1513986836.git.johannes.schindelin@gmx.de","threadId":"47485","inReplyTo":"cover.1513986836.git.johannes.schindelin@gmx.de","subject":"[PATCH 2/5] sequencer: strip bogus LF at end of error messages","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-12-22T23:55:43Z","receivedAt":"2017-12-22T23:55:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Signed-off-by: Johannes Schindelin \n---\n sequencer.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 115085d39ca..8e6b6289be6 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -491,7 +491,7 @@ static int is_index_unchanged(void)\n \tstruct commit *head_commit;\n \n \tif (!resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING, &head_oid, NULL))\n-\t\treturn error(_(\"could not resolve HEAD commit\\n\"));\n+\t\treturn error(_(\"could not resolve HEAD commit\"));\n \n \thead_commit = lookup_commit(&head_oid);\n \n@@ -511,7 +511,7 @@ static int is_index_unchanged(void)\n \n \tif (!cache_tree_fully_valid(active_cache_tree))\n \t\tif (cache_tree_update(&the_index, 0))\n-\t\t\treturn error(_(\"unable to update cache tree\\n\"));\n+\t\t\treturn error(_(\"unable to update cache tree\"));\n \n \treturn !oidcmp(&active_cache_tree->oid,\n \t\t &head_commit->tree->object.oid);\n@@ -697,12 +697,12 @@ static int is_original_commit_empty(struct commit *commit)\n \tconst struct object_id *ptree_oid;\n \n \tif (parse_commit(commit))\n-\t\treturn error(_(\"could not parse commit %s\\n\"),\n+\t\treturn error(_(\"could not parse commit %s\"),\n \t\t\t oid_to_hex(&commit->object.oid));\n \tif (commit->parents) {\n \t\tstruct commit *parent = commit->parents->item;\n \t\tif (parse_commit(parent))\n-\t\t\treturn error(_(\"could not parse parent commit %s\\n\"),\n+\t\t\treturn error(_(\"could not parse parent commit %s\"),\n \t\t\t\toid_to_hex(&parent->object.oid));\n \t\tptree_oid = &parent->tree->object.oid;\n \t} else {\n-- \n2.15.1.windows.2\n\n\n"},{"id":"335209","messageId":"2e30f42e0bbe6d1f86f46e6461d3218adb2ede5b.1513986836.git.johannes.schindelin@gmx.de","threadId":"47485","inReplyTo":"cover.1513986836.git.johannes.schindelin@gmx.de","subject":"[PATCH 3/5] sequencer: remove superfluous conditional","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-12-22T23:55:53Z","receivedAt":"2017-12-22T23:56:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In a conditional block that is only reached when handling a TODO_REWORD\n(as seen even from a 3-line context), there is absolutely no need to\nnest another block under the identical condition.\n\nSigned-off-by: Johannes Schindelin \n---\n sequencer.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 8e6b6289be6..38266c3c228 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1011,9 +1011,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,\n \t\t\topts);\n \t\tif (res || command != TODO_REWORD)\n \t\t\tgoto leave;\n-\t\tflags |= EDIT_MSG | AMEND_MSG;\n-\t\tif (command == TODO_REWORD)\n-\t\t\tflags |= VERIFY_MSG;\n+\t\tflags |= EDIT_MSG | AMEND_MSG | VERIFY_MSG;\n \t\tmsg_file = NULL;\n \t\tgoto fast_forward_edit;\n \t}\n-- \n2.15.1.windows.2\n\n\n"},{"id":"335210","messageId":"5fe76040e25993662002c421a5fbcb3ea2d00c96.1513986836.git.johannes.schindelin@gmx.de","threadId":"47485","inReplyTo":"cover.1513986836.git.johannes.schindelin@gmx.de","subject":"[PATCH 4/5] sequencer: report when noop has an argument","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-12-22T23:55:57Z","receivedAt":"2017-12-22T23:56:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The noop command cannot accept any argument, but we never told the user\nabout any bogus argument. Fix that.\n\nwhile at it, mention clearly when an argument is required but missing\n(for commands *other* than noop).\n\nSigned-off-by: Johannes Schindelin \n---\n sequencer.c | 13 +++++++++----\n 1 file changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 38266c3c228..5632415ea2d 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1259,18 +1259,23 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)\n \tif (i >= TODO_COMMENT)\n \t\treturn -1;\n \n+\t/* Eat up extra spaces/ tabs before object name */\n+\tpadding = strspn(bol, \" \\t\");\n+\tbol += padding;\n+\n \tif (item->command == TODO_NOOP) {\n+\t\tif (bol != eol)\n+\t\t\treturn error(_(\"%s does not accept arguments: '%s'\"),\n+\t\t\t\t command_to_string(item->command), bol);\n \t\titem->commit = NULL;\n \t\titem->arg = bol;\n \t\titem->arg_len = eol - bol;\n \t\treturn 0;\n \t}\n \n-\t/* Eat up extra spaces/ tabs before object name */\n-\tpadding = strspn(bol, \" \\t\");\n \tif (!padding)\n-\t\treturn -1;\n-\tbol += padding;\n+\t\treturn error(_(\"missing arguments for %s\"),\n+\t\t\t command_to_string(item->command));\n \n \tif (item->command == TODO_EXEC) {\n \t\titem->commit = NULL;\n-- \n2.15.1.windows.2\n\n\n"},{"id":"335211","messageId":"da288dac49ac1e54db99349a50a7cd684956671c.1513986836.git.johannes.schindelin@gmx.de","threadId":"47485","inReplyTo":"cover.1513986836.git.johannes.schindelin@gmx.de","subject":"[PATCH 5/5] sequencer: do not invent whitespace when transforming OIDs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-12-22T23:56:00Z","receivedAt":"2017-12-22T23:56:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"For commands that do not have an argument, there is no need to append a\ntrailing space at the end of the line.\n\nSigned-off-by: Johannes Schindelin \n---\n sequencer.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 5632415ea2d..970842e3fcc 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2584,7 +2584,10 @@ int transform_todos(unsigned flags)\n \t\t\tstrbuf_addf(&buf, \" %s\", oid);\n \t\t}\n \t\t/* add all the rest */\n-\t\tstrbuf_addf(&buf, \" %.*s\\n\", item->arg_len, item->arg);\n+\t\tif (!item->arg_len)\n+\t\t\tstrbuf_addch(&buf, '\\n');\n+\t\telse\n+\t\t\tstrbuf_addf(&buf, \" %.*s\\n\", item->arg_len, item->arg);\n \t}\n \n \ti = write_message(buf.buf, buf.len, todo_file, 0);\n-- \n2.15.1.windows.2\n"},{"id":"335394","messageId":"xmqqa7y3nc6y.fsf@gitster.mtv.corp.google.com","threadId":"47485","inReplyTo":"cover.1513986836.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 0/5] A couple of sequencer cleanups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-27T20:33:09Z","receivedAt":"2017-12-27T20:33:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Johannes Schindelin (5):\n> rebase: do not continue when the todo list generation failed\n> sequencer: strip bogus LF at end of error messages\n> sequencer: remove superfluous conditional\n> sequencer: report when noop has an argument\n> sequencer: do not invent whitespace when transforming OIDs\n\nAll looked obviously correct and good clean-up changes. \n\nThanks; will queue.\n\n\n"},{"id":"335398","messageId":"CAKm4OoX+S47iQnBogvU9j4k3pUBtn0uVWkjOKt-M+D6-O+4BzA@mail.gmail.com","threadId":"47485","inReplyTo":"da288dac49ac1e54db99349a50a7cd684956671c.1513986836.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 5/5] sequencer: do not invent whitespace when transforming OIDs","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-27T22:19:27Z","receivedAt":"2017-12-27T22:19:53Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Hi Johannes,\n\nOn 23 December 2017 at 00:56, Johannes Schindelin\n wrote:\n> For commands that do not have an argument, there is no need to append a\n> trailing space at the end of the line.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> sequencer.c | 5 ++++-\n> 1 file changed, 4 insertions(+), 1 deletion(-)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index 5632415ea2d..970842e3fcc 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2584,7 +2584,10 @@ int transform_todos(unsigned flags)\n> strbuf_addf(&buf, \" %s\", oid);\n> }\n> /* add all the rest */\n> - strbuf_addf(&buf, \" %.*s\\n\", item->arg_len, item->arg);\n> + if (!item->arg_len)\n> + strbuf_addch(&buf, '\\n');\n> + else\n> + strbuf_addf(&buf, \" %.*s\\n\", item->arg_len, item->arg);\n\nI also went with that when I was working on this but I thought leaving the\nextra whitespace would make the code a little shorter.\nOther than that, this change and the others look good.\n\n> }\n>\n> i = write_message(buf.buf, buf.len, todo_file, 0);\n> --\n> 2.15.1.windows.2\n\nThanks,\nLiam\n"}]}