{"thread":{"id":"47327","subject":"[PATCH 0/5] rebase -i: add config to abbreviate command names","startedAt":"2017-11-27T04:55:44Z","lastAt":"2017-12-28T18:55:19Z","messageCount":70,"participants":["Liam Beguin","Junio C Hamano","Johannes Schindelin","Jeff King","liam Beguin","Kerry, Richard","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"333579","messageId":"20171127045514.25647-1-liambeguin@gmail.com","threadId":"47327","inReplyTo":null,"subject":"[PATCH 0/5] rebase -i: add config to abbreviate command names","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-11-27T04:55:09Z","receivedAt":"2017-11-27T04:55:44Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Hi everyone,\n\nThis series is a respin of something [1] I sent a few months ago. This\ntime, instead of shell, It's based on top of the C implementation of the\ninteractive rebase. I've also tried to address the comments that were\nleft in the last thread.\n\nThis series will add the 'rebase.abbreviateCommands' configuration\noption to allow `git rebase -i` to default to the single-letter command\nnames when generating the todo list.\n\nUsing single-letter command names can present two benefits. First, it\nmakes it easier to change the action since you only need to replace a\nsingle character (i.e.: in vim \"r\" instead of\n\"ciw\"). Second, using this with a large enough value of\n'core.abbrev' enables the lines of the todo list to remain aligned\nmaking the files easier to read.\n\nChanges since last time:\n- Implement abbreviateCommands in rebase--helper\n- Add note on the --[no-]autosquash option in rebase.autoSquash\n- Add exec commands via the rebase--helper\n- Add test case for rebase.abbreviateCommands\n\nLiam Beguin (5):\n Documentation: move rebase.* configs to new file\n Documentation: use preferred name for the 'todo list' script\n rebase -i: add exec commands via the rebase--helper\n rebase -i: learn to abbreviate command names\n t3404: add test case for abbreviated commands\n\n Documentation/config.txt | 31 +------------\n Documentation/git-rebase.txt | 19 +-------\n Documentation/rebase-config.txt | 51 ++++++++++++++++++++\n builtin/rebase--helper.c | 17 +++++--\n git-rebase--interactive.sh | 23 +--------\n sequencer.c | 100 ++++++++++++++++++++++++++++++++++------\n sequencer.h | 5 +-\n t/t3404-rebase-interactive.sh | 32 +++++++++++++\n 8 files changed, 186 insertions(+), 92 deletions(-)\n create mode 100644 Documentation/rebase-config.txt\n\n[1] https://public-inbox.org/git/20170502040048.9065-1-liambeguin@gmail.com/\n--\n2.15.0.321.g19bf2bb99cee.dirty\n\n"},{"id":"333580","messageId":"20171127045514.25647-4-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171127045514.25647-1-liambeguin@gmail.com","subject":"[PATCH 3/5] rebase -i: add exec commands via the rebase--helper","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-11-27T04:55:12Z","receivedAt":"2017-11-27T04:55:47Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Recent work on `git-rebase--interactive` aim to convert shell code to C.\nEven if this is most likely not a big performance enhacement, let's\nconvert it too since a comming change to abbreviate command names requires\nit to be updated.\n\nSigned-off-by: Liam Beguin \n---\n builtin/rebase--helper.c | 7 ++++++-\n git-rebase--interactive.sh | 23 +----------------------\n sequencer.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++\n sequencer.h | 1 +\n 4 files changed, 54 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex f8519363a393..9d94c874c5bb 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -15,7 +15,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \tint keep_empty = 0;\n \tenum {\n \t\tCONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S,\n-\t\tCHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH\n+\t\tCHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,\n+\t\tADD_EXEC\n \t} command = 0;\n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"ff\", &opts.allow_ff, N_(\"allow fast-forward\")),\n@@ -36,6 +37,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\t\tN_(\"skip unnecessary picks\"), SKIP_UNNECESSARY_PICKS),\n \t\tOPT_CMDMODE(0, \"rearrange-squash\", &command,\n \t\t\tN_(\"rearrange fixup/squash lines\"), REARRANGE_SQUASH),\n+\t\tOPT_CMDMODE(0, \"add-exec\", &command,\n+\t\t\tN_(\"insert exec commands in todo list\"), ADD_EXEC),\n \t\tOPT_END()\n \t};\n \n@@ -64,5 +67,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\treturn !!skip_unnecessary_picks();\n \tif (command == REARRANGE_SQUASH && argc == 1)\n \t\treturn !!rearrange_squash();\n+\tif (command == ADD_EXEC && argc == 2)\n+\t\treturn !!add_exec_commands(argv[1]);\n \tusage_with_options(builtin_rebase_helper_usage, options);\n }\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 437815669f00..760334d3a8b3 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -722,27 +722,6 @@ collapse_todo_ids() {\n \tgit rebase--helper --shorten-ids\n }\n \n-# Add commands after a pick or after a squash/fixup series\n-# in the todo list.\n-add_exec_commands () {\n-\t{\n-\t\tfirst=t\n-\t\twhile read -r insn rest\n-\t\tdo\n-\t\t\tcase $insn in\n-\t\t\tpick)\n-\t\t\t\ttest -n \"$first\" ||\n-\t\t\t\tprintf \"%s\" \"$cmd\"\n-\t\t\t\t;;\n-\t\t\tesac\n-\t\t\tprintf \"%s %s\\n\" \"$insn\" \"$rest\"\n-\t\t\tfirst=\n-\t\tdone\n-\t\tprintf \"%s\" \"$cmd\"\n-\t} <\"$1\" >\"$1.new\" &&\n-\tmv \"$1.new\" \"$1\"\n-}\n-\n # Switch to the branch in $into and notify it in the reflog\n checkout_onto () {\n \tGIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $onto_name\"\n@@ -982,7 +961,7 @@ fi\n \n test -s \"$todo\" || echo noop >> \"$todo\"\n test -z \"$autosquash\" || git rebase--helper --rearrange-squash || exit\n-test -n \"$cmd\" && add_exec_commands \"$todo\"\n+test -n \"$cmd\" && git rebase--helper --add-exec \"$cmd\"\n \n todocount=$(git stripspace --strip-comments <\"$todo\" | wc -l)\n todocount=${todocount##* }\ndiff --git a/sequencer.c b/sequencer.c\nindex fa94ed652d2c..810b7850748e 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2492,6 +2492,52 @@ int sequencer_make_script(int keep_empty, FILE *out,\n \treturn 0;\n }\n \n+int add_exec_commands(const char *command)\n+{\n+\tconst char *todo_file = rebase_path_todo();\n+\tstruct todo_list todo_list = TODO_LIST_INIT;\n+\tint fd, res, i, first = 1;\n+\tFILE *out;\n+\n+\tstrbuf_reset(&todo_list.buf);\n+\tfd = open(todo_file, O_RDONLY);\n+\tif (fd < 0)\n+\t\treturn error_errno(_(\"could not open '%s'\"), todo_file);\n+\tif (strbuf_read(&todo_list.buf, fd, 0) < 0) {\n+\t\tclose(fd);\n+\t\treturn error(_(\"could not read '%s'.\"), todo_file);\n+\t}\n+\tclose(fd);\n+\n+\tres = parse_insn_buffer(todo_list.buf.buf, &todo_list);\n+\tif (res) {\n+\t\ttodo_list_release(&todo_list);\n+\t\treturn error(_(\"unusable todo list: '%s'\"), todo_file);\n+\t}\n+\n+\tout = fopen(todo_file, \"w\");\n+\tif (!out) {\n+\t\ttodo_list_release(&todo_list);\n+\t\treturn error(_(\"unable to open '%s' for writing\"), todo_file);\n+\t}\n+\tfor (i = 0; i < todo_list.nr; i++) {\n+\t\tstruct todo_item *item = todo_list.items + i;\n+\t\tint bol = item->offset_in_buf;\n+\t\tconst char *p = todo_list.buf.buf + bol;\n+\t\tint eol = i + 1 < todo_list.nr ?\n+\t\t\ttodo_list.items[i + 1].offset_in_buf :\n+\t\t\ttodo_list.buf.len;\n+\n+\t\tif (item->command == TODO_PICK && !first)\n+\t\t\tfputs(command, out);\n+\t\tfwrite(p, eol - bol, 1, out);\n+\t\tfirst = 0;\n+\t}\n+\tfputs(command, out);\n+\tfclose(out);\n+\ttodo_list_release(&todo_list);\n+\treturn 0;\n+}\n \n int transform_todo_ids(int shorten_ids)\n {\ndiff --git a/sequencer.h b/sequencer.h\nindex 6f3d3df82c0a..a2715e6c7589 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -48,6 +48,7 @@ int sequencer_remove_state(struct replay_opts *opts);\n int sequencer_make_script(int keep_empty, FILE *out,\n \t\tint argc, const char **argv);\n \n+int add_exec_commands(const char *command);\n int transform_todo_ids(int shorten_ids);\n int check_todo_list(void);\n int skip_unnecessary_picks(void);\n-- \n2.15.0.321.g19bf2bb99cee.dirty\n\n"},{"id":"333581","messageId":"20171127045514.25647-5-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171127045514.25647-1-liambeguin@gmail.com","subject":"[PATCH 4/5] rebase -i: learn to abbreviate command names","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-11-27T04:55:13Z","receivedAt":"2017-11-27T04:55:49Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"`git rebase -i` already know how to interpret single-letter command\nnames. Teach it to generate the todo list with these same abbreviated\nnames.\n\nBased-on-patch-by: Johannes Schindelin \nSigned-off-by: Liam Beguin \n---\n Documentation/rebase-config.txt | 19 +++++++++++++++\n builtin/rebase--helper.c | 10 +++++---\n sequencer.c | 54 +++++++++++++++++++++++++++++------------\n sequencer.h | 4 +--\n 4 files changed, 66 insertions(+), 21 deletions(-)\n\ndiff --git a/Documentation/rebase-config.txt b/Documentation/rebase-config.txt\nindex 30ae08cb5a4b..0820b60f6e12 100644\n--- a/Documentation/rebase-config.txt\n+++ b/Documentation/rebase-config.txt\n@@ -30,3 +30,22 @@ rebase.instructionFormat::\n \tA format string, as specified in linkgit:git-log[1], to be used for the\n \ttodo list during an interactive rebase. The format will\n \tautomatically have the long commit hash prepended to the format.\n+\n+rebase.abbreviateCommands::\n+\tIf set to true, `git rebase` will use abbreviated command names in the\n+\ttodo list resulting in something like this:\n+\n+-------------------------------------------\n+\tp deadbee The oneline of the commit\n+\tp fa1afe1 The oneline of the next commit\n+\t...\n+-------------------------------------------\n+\n+\tinstead of:\n+\n+-------------------------------------------\n+\tpick deadbee The oneline of the commit\n+\tpick fa1afe1 The oneline of the next commit\n+\t...\n+-------------------------------------------\n+\tDefaults to false.\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex 9d94c874c5bb..7b1fe825a877 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -12,7 +12,7 @@ static const char * const builtin_rebase_helper_usage[] = {\n int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n {\n \tstruct replay_opts opts = REPLAY_OPTS_INIT;\n-\tint keep_empty = 0;\n+\tint keep_empty = 0, abbreviate_commands = 0;\n \tenum {\n \t\tCONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S,\n \t\tCHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,\n@@ -43,6 +43,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t};\n \n \tgit_config(git_default_config, NULL);\n+\tgit_config_get_bool(\"rebase.abbreviatecommands\", &abbreviate_commands);\n \n \topts.action = REPLAY_INTERACTIVE_REBASE;\n \topts.allow_ff = 1;\n@@ -56,11 +57,12 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \tif (command == ABORT && argc == 1)\n \t\treturn !!sequencer_remove_state(&opts);\n \tif (command == MAKE_SCRIPT && argc > 1)\n-\t\treturn !!sequencer_make_script(keep_empty, stdout, argc, argv);\n+\t\treturn !!sequencer_make_script(keep_empty, abbreviate_commands,\n+\t\t\t\t\t stdout, argc, argv);\n \tif (command == SHORTEN_SHA1S && argc == 1)\n-\t\treturn !!transform_todo_ids(1);\n+\t\treturn !!transform_todo_ids(1, abbreviate_commands);\n \tif (command == EXPAND_SHA1S && argc == 1)\n-\t\treturn !!transform_todo_ids(0);\n+\t\treturn !!transform_todo_ids(0, abbreviate_commands);\n \tif (command == CHECK_TODO_LIST && argc == 1)\n \t\treturn !!check_todo_list();\n \tif (command == SKIP_UNNECESSARY_PICKS && argc == 1)\ndiff --git a/sequencer.c b/sequencer.c\nindex 810b7850748e..aa01e8bd9280 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -795,6 +795,13 @@ static const char *command_to_string(const enum todo_command command)\n \tdie(\"Unknown command: %d\", command);\n }\n \n+static const char command_to_char(const enum todo_command command)\n+{\n+\tif (command < TODO_COMMENT && todo_command_info[command].c)\n+\t\treturn todo_command_info[command].c;\n+\treturn -1;\n+}\n+\n static int is_noop(const enum todo_command command)\n {\n \treturn TODO_NOOP <= command;\n@@ -1242,15 +1249,16 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)\n \t\treturn 0;\n \t}\n \n-\tfor (i = 0; i < TODO_COMMENT; i++)\n+\tfor (i = 0; i < TODO_COMMENT; i++) {\n \t\tif (skip_prefix(bol, todo_command_info[i].str, &bol)) {\n \t\t\titem->command = i;\n \t\t\tbreak;\n-\t\t} else if (bol[1] == ' ' && *bol == todo_command_info[i].c) {\n+\t\t} else if (bol[1] == ' ' && *bol == command_to_char(i)) {\n \t\t\tbol++;\n \t\t\titem->command = i;\n \t\t\tbreak;\n \t\t}\n+\t}\n \tif (i >= TODO_COMMENT)\n \t\treturn -1;\n \n@@ -2443,8 +2451,8 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)\n \tstrbuf_release(&sob);\n }\n \n-int sequencer_make_script(int keep_empty, FILE *out,\n-\t\tint argc, const char **argv)\n+int sequencer_make_script(int keep_empty, int abbreviate_commands, FILE *out,\n+\t\t\t int argc, const char **argv)\n {\n \tchar *format = NULL;\n \tstruct pretty_print_context pp = {0};\n@@ -2483,7 +2491,9 @@ int sequencer_make_script(int keep_empty, FILE *out,\n \t\tstrbuf_reset(&buf);\n \t\tif (!keep_empty && is_original_commit_empty(commit))\n \t\t\tstrbuf_addf(&buf, \"%c \", comment_line_char);\n-\t\tstrbuf_addf(&buf, \"pick %s \", oid_to_hex(&commit->object.oid));\n+\t\tstrbuf_addf(&buf, \"%s %s \",\n+\t\t\t abbreviate_commands ? \"p\" : \"pick\",\n+\t\t\t oid_to_hex(&commit->object.oid));\n \t\tpretty_print_commit(&pp, commit, &buf);\n \t\tstrbuf_addch(&buf, '\\n');\n \t\tfputs(buf.buf, out);\n@@ -2539,7 +2549,7 @@ int add_exec_commands(const char *command)\n \treturn 0;\n }\n \n-int transform_todo_ids(int shorten_ids)\n+int transform_todo_ids(int shorten_ids, int abbreviate_commands)\n {\n \tconst char *todo_file = rebase_path_todo();\n \tstruct todo_list todo_list = TODO_LIST_INIT;\n@@ -2575,19 +2585,33 @@ int transform_todo_ids(int shorten_ids)\n \t\t\ttodo_list.items[i + 1].offset_in_buf :\n \t\t\ttodo_list.buf.len;\n \n-\t\tif (item->command >= TODO_EXEC && item->command != TODO_DROP)\n-\t\t\tfwrite(p, eol - bol, 1, out);\n-\t\telse {\n+\t\tif (item->command >= TODO_EXEC && item->command != TODO_DROP) {\n+\t\t\tif (!abbreviate_commands || command_to_char(item->command) < 0) {\n+\t\t\t\tfwrite(p, eol - bol, 1, out);\n+\t\t\t} else {\n+\t\t\t\tconst char *end_of_line = strchrnul(p, '\\n');\n+\t\t\t\tp += strspn(p, \" \\t\"); /* skip whitespace */\n+\t\t\t\tp += strcspn(p, \" \\t\"); /* skip command */\n+\t\t\t\tfprintf(out, \"%c%.*s\\n\",\n+\t\t\t\t\tcommand_to_char(item->command),\n+\t\t\t\t\t(int)(end_of_line - p), p);\n+\t\t\t}\n+\t\t} else {\n \t\t\tconst char *id = shorten_ids ?\n \t\t\t\tshort_commit_name(item->commit) :\n \t\t\t\toid_to_hex(&item->commit->object.oid);\n-\t\t\tint len;\n \n-\t\t\tp += strspn(p, \" \\t\"); /* left-trim command */\n-\t\t\tlen = strcspn(p, \" \\t\"); /* length of command */\n-\n-\t\t\tfprintf(out, \"%.*s %s %.*s\\n\",\n-\t\t\t\tlen, p, id, item->arg_len, item->arg);\n+\t\t\tif (abbreviate_commands) {\n+\t\t\t\tfprintf(out, \"%c %s %.*s\\n\",\n+\t\t\t\t\tcommand_to_char(item->command),\n+\t\t\t\t\tid, item->arg_len, item->arg);\n+\t\t\t} else {\n+\t\t\t\tint len;\n+\t\t\t\tp += strspn(p, \" \\t\"); /* left-trim command */\n+\t\t\t\tlen = strcspn(p, \" \\t\"); /* length of command */\n+\t\t\t\tfprintf(out, \"%.*s %s %.*s\\n\",\n+\t\t\t\t\tlen, p, id, item->arg_len, item->arg);\n+\t\t\t}\n \t\t}\n \t}\n \tfclose(out);\ndiff --git a/sequencer.h b/sequencer.h\nindex a2715e6c7589..cee8394673de 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -45,11 +45,11 @@ int sequencer_continue(struct replay_opts *opts);\n int sequencer_rollback(struct replay_opts *opts);\n int sequencer_remove_state(struct replay_opts *opts);\n \n-int sequencer_make_script(int keep_empty, FILE *out,\n+int sequencer_make_script(int keep_empty, int abbreviate_commands, FILE *out,\n \t\tint argc, const char **argv);\n \n int add_exec_commands(const char *command);\n-int transform_todo_ids(int shorten_ids);\n+int transform_todo_ids(int shorten_ids, int abbreviate_commands);\n int check_todo_list(void);\n int skip_unnecessary_picks(void);\n int rearrange_squash(void);\n-- \n2.15.0.321.g19bf2bb99cee.dirty\n\n"},{"id":"333582","messageId":"20171127045514.25647-6-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171127045514.25647-1-liambeguin@gmail.com","subject":"[PATCH 5/5] t3404: add test case for abbreviated commands","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-11-27T04:55:14Z","receivedAt":"2017-11-27T04:55:50Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Make sure the todo list ends up using single-letter command\nabbreviations when the rebase.abbreviateCommands is enabled.\nThis configuration options should not change anything else.\n\nSigned-off-by: Liam Beguin \n---\n t/t3404-rebase-interactive.sh | 32 ++++++++++++++++++++++++++++++++\n 1 file changed, 32 insertions(+)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 6a82d1ed876d..e460ebde3393 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1260,6 +1260,38 @@ test_expect_success 'rebase -i respects rebase.missingCommitsCheck = error' '\n \ttest B = $(git cat-file commit HEAD^ | sed -ne \\$p)\n '\n \n+test_expect_success 'prepare rebase.abbreviateCommands' '\n+\treset_rebase &&\n+\tgit checkout -b abbrevcmd master &&\n+\ttest_commit \"first\" file1.txt \"first line\" first &&\n+\ttest_commit \"second\" file1.txt \"another line\" second &&\n+\ttest_commit \"fixup! first\" file2.txt \"first line again\" first_fixup &&\n+\ttest_commit \"squash! second\" file1.txt \"another line here\" second_squash\n+'\n+\n+cat >expected <actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'static check of bad command' '\n \trebase_setup_and_clean bad-cmd &&\n \tset_fake_editor &&\n-- \n2.15.0.321.g19bf2bb99cee.dirty\n\n"},{"id":"333583","messageId":"20171127045514.25647-3-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171127045514.25647-1-liambeguin@gmail.com","subject":"[PATCH 2/5] Documentation: use preferred name for the 'todo list' script","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-11-27T04:55:11Z","receivedAt":"2017-11-27T04:55:53Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Use \"todo list\" instead of \"instruction list\" or \"todo-list\" to\nreduce further confusion regarding the name of this script.\n\nSigned-off-by: Liam Beguin \n---\n Documentation/rebase-config.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/rebase-config.txt b/Documentation/rebase-config.txt\nindex dba088d7c68f..30ae08cb5a4b 100644\n--- a/Documentation/rebase-config.txt\n+++ b/Documentation/rebase-config.txt\n@@ -23,10 +23,10 @@ rebase.missingCommitsCheck::\n \t--edit-todo' can then be used to correct the error. If set to\n \t\"ignore\", no checking is done.\n \tTo drop a commit without warning or error, use the `drop`\n-\tcommand in the todo-list.\n+\tcommand in the todo list.\n \tDefaults to \"ignore\".\n \n rebase.instructionFormat::\n \tA format string, as specified in linkgit:git-log[1], to be used for the\n-\tinstruction list during an interactive rebase. The format will\n+\ttodo list during an interactive rebase. The format will\n \tautomatically have the long commit hash prepended to the format.\n-- \n2.15.0.321.g19bf2bb99cee.dirty\n\n"},{"id":"333584","messageId":"20171127045514.25647-2-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171127045514.25647-1-liambeguin@gmail.com","subject":"[PATCH 1/5] Documentation: move rebase.* configs to new file","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-11-27T04:55:10Z","receivedAt":"2017-11-27T04:55:55Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Move all rebase.* configuration variables to a separate file in order to\nremove duplicates, and include it in config.txt and git-rebase.txt. The\nnew descriptions are mostly taken from config.txt as they are more\nverbose.\n\nSigned-off-by: Liam Beguin \n---\n Documentation/config.txt | 31 +------------------------------\n Documentation/git-rebase.txt | 19 +------------------\n Documentation/rebase-config.txt | 32 ++++++++++++++++++++++++++++++++\n 3 files changed, 34 insertions(+), 48 deletions(-)\n create mode 100644 Documentation/rebase-config.txt\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 531649cb40ea..e424b7de90b5 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2691,36 +2691,7 @@ push.recurseSubmodules::\n \tis retained. You may override this configuration at time of push by\n \tspecifying '--recurse-submodules=check|on-demand|no'.\n \n-rebase.stat::\n-\tWhether to show a diffstat of what changed upstream since the last\n-\trebase. False by default.\n-\n-rebase.autoSquash::\n-\tIf set to true enable `--autosquash` option by default.\n-\n-rebase.autoStash::\n-\tWhen set to true, automatically create a temporary stash entry\n-\tbefore the operation begins, and apply it after the operation\n-\tends. This means that you can run rebase on a dirty worktree.\n-\tHowever, use with care: the final stash application after a\n-\tsuccessful rebase might result in non-trivial conflicts.\n-\tDefaults to false.\n-\n-rebase.missingCommitsCheck::\n-\tIf set to \"warn\", git rebase -i will print a warning if some\n-\tcommits are removed (e.g. a line was deleted), however the\n-\trebase will still proceed. If set to \"error\", it will print\n-\tthe previous warning and stop the rebase, 'git rebase\n-\t--edit-todo' can then be used to correct the error. If set to\n-\t\"ignore\", no checking is done.\n-\tTo drop a commit without warning or error, use the `drop`\n-\tcommand in the todo-list.\n-\tDefaults to \"ignore\".\n-\n-rebase.instructionFormat::\n-\tA format string, as specified in linkgit:git-log[1], to be used for\n-\tthe instruction list during an interactive rebase. The format will automatically\n-\thave the long commit hash prepended to the format.\n+include::rebase-config.txt[]\n \n receive.advertiseAtomic::\n \tBy default, git-receive-pack will advertise the atomic push\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 3cedfb0fd22b..8a861c1e0d69 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -203,24 +203,7 @@ Alternatively, you can undo the 'git rebase' with\n CONFIGURATION\n -------------\n \n-rebase.stat::\n-\tWhether to show a diffstat of what changed upstream since the last\n-\trebase. False by default.\n-\n-rebase.autoSquash::\n-\tIf set to true enable `--autosquash` option by default.\n-\n-rebase.autoStash::\n-\tIf set to true enable `--autostash` option by default.\n-\n-rebase.missingCommitsCheck::\n-\tIf set to \"warn\", print warnings about removed commits in\n-\tinteractive mode. If set to \"error\", print the warnings and\n-\tstop the rebase. If set to \"ignore\", no checking is\n-\tdone. \"ignore\" by default.\n-\n-rebase.instructionFormat::\n-\tCustom commit list format to use during an `--interactive` rebase.\n+include::rebase-config.txt[]\n \n OPTIONS\n -------\ndiff --git a/Documentation/rebase-config.txt b/Documentation/rebase-config.txt\nnew file mode 100644\nindex 000000000000..dba088d7c68f\n--- /dev/null\n+++ b/Documentation/rebase-config.txt\n@@ -0,0 +1,32 @@\n+rebase.stat::\n+\tWhether to show a diffstat of what changed upstream since the last\n+\trebase. False by default.\n+\n+rebase.autoSquash::\n+\tIf set to true enable `--autosquash` option by default.\n+\n+rebase.autoStash::\n+\tWhen set to true, automatically create a temporary stash entry\n+\tbefore the operation begins, and apply it after the operation\n+\tends. This means that you can run rebase on a dirty worktree.\n+\tHowever, use with care: the final stash application after a\n+\tsuccessful rebase might result in non-trivial conflicts.\n+\tThis option can be overridden by the `--no-autostash` and\n+\t`--autostash` options of linkgit:git-rebase[1].\n+\tDefaults to false.\n+\n+rebase.missingCommitsCheck::\n+\tIf set to \"warn\", git rebase -i will print a warning if some\n+\tcommits are removed (e.g. a line was deleted), however the\n+\trebase will still proceed. If set to \"error\", it will print\n+\tthe previous warning and stop the rebase, 'git rebase\n+\t--edit-todo' can then be used to correct the error. If set to\n+\t\"ignore\", no checking is done.\n+\tTo drop a commit without warning or error, use the `drop`\n+\tcommand in the todo-list.\n+\tDefaults to \"ignore\".\n+\n+rebase.instructionFormat::\n+\tA format string, as specified in linkgit:git-log[1], to be used for the\n+\tinstruction list during an interactive rebase. The format will\n+\tautomatically have the long commit hash prepended to the format.\n-- \n2.15.0.321.g19bf2bb99cee.dirty\n\n"},{"id":"333589","messageId":"xmqq609we20v.fsf@gitster.mtv.corp.google.com","threadId":"47327","inReplyTo":"20171127045514.25647-4-liambeguin@gmail.com","subject":"Re: [PATCH 3/5] rebase -i: add exec commands via the rebase--helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-27T05:14:40Z","receivedAt":"2017-11-27T05:14:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Liam Beguin writes:\n\n> diff --git a/sequencer.c b/sequencer.c\n> index fa94ed652d2c..810b7850748e 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2492,6 +2492,52 @@ int sequencer_make_script(int keep_empty, FILE *out,\n> \treturn 0;\n> }\n> \n> +int add_exec_commands(const char *command)\n> +{\n\nAs the name of a public function, it does not feel that this hints\nit strongly enough that it is from and a part of sequencer.c API.\n\n> +\tconst char *todo_file = rebase_path_todo();\n> +\tstruct todo_list todo_list = TODO_LIST_INIT;\n> +\tint fd, res, i, first = 1;\n> +\tFILE *out;\n\nHaving had to scan backwards while trying to see what the loop that\nuses this variable is doing and if it gets affected by things that\nhappened before we entered the loop, I'd rather not to see 'first'\ninitialized here, left unused for quite some time until the loop is\nentered. It would make it a lot easier to follow if it is declared\nand left uninitilized here, and set to 1 immediately before the\nfor() loop that uses it.\n\n> +\n> +\tstrbuf_reset(&todo_list.buf);\n> +\tfd = open(todo_file, O_RDONLY);\n> +\tif (fd < 0)\n> +\t\treturn error_errno(_(\"could not open '%s'\"), todo_file);\n> +\tif (strbuf_read(&todo_list.buf, fd, 0) < 0) {\n> +\t\tclose(fd);\n> +\t\treturn error(_(\"could not read '%s'.\"), todo_file);\n> +\t}\n> +\tclose(fd);\n\nIs this strbuf_read_file() written in longhand?\n\n> +\tres = parse_insn_buffer(todo_list.buf.buf, &todo_list);\n> +\tif (res) {\n> +\t\ttodo_list_release(&todo_list);\n> +\t\treturn error(_(\"unusable todo list: '%s'\"), todo_file);\n> +\t}\n> +\n> +\tout = fopen(todo_file, \"w\");\n> +\tif (!out) {\n> +\t\ttodo_list_release(&todo_list);\n> +\t\treturn error(_(\"unable to open '%s' for writing\"), todo_file);\n> +\t}\n> +\tfor (i = 0; i < todo_list.nr; i++) {\n> +\t\tstruct todo_item *item = todo_list.items + i;\n> +\t\tint bol = item->offset_in_buf;\n> +\t\tconst char *p = todo_list.buf.buf + bol;\n> +\t\tint eol = i + 1 < todo_list.nr ?\n> +\t\t\ttodo_list.items[i + 1].offset_in_buf :\n> +\t\t\ttodo_list.buf.len;\n\nShould bol and eol be of type size_t instead? The values that get\nassigned to them from other structures are.\n"},{"id":"333590","messageId":"xmqq1skke1so.fsf@gitster.mtv.corp.google.com","threadId":"47327","inReplyTo":"20171127045514.25647-5-liambeguin@gmail.com","subject":"Re: [PATCH 4/5] rebase -i: learn to abbreviate command names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-27T05:19:35Z","receivedAt":"2017-11-27T05:19:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Liam Beguin writes:\n\n> \tif (command == MAKE_SCRIPT && argc > 1)\n> -\t\treturn !!sequencer_make_script(keep_empty, stdout, argc, argv);\n> +\t\treturn !!sequencer_make_script(keep_empty, abbreviate_commands,\n> +\t\t\t\t\t stdout, argc, argv);\n\nThis suggests that a preliminary clean-up to update the parameter\nlist of sequencer_make_script() is in order just before this step.\nHow about making it like so, perhaps:\n\n int sequencer_make_script(FILE *out, int ac, char **av, unsigned flags)\n\nwhere keep_empty becomes just one bit in that flags word. Then another\nbit in the same flags word can be used for this option.\n\nOtherwise, every time somebody comes up with a new and shiny feature\nfor the function, we'd end up adding more to its parameter list.\n"},{"id":"333592","messageId":"xmqqwp2ccn2i.fsf@gitster.mtv.corp.google.com","threadId":"47327","inReplyTo":"20171127045514.25647-1-liambeguin@gmail.com","subject":"Re: [PATCH 0/5] rebase -i: add config to abbreviate command names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-27T05:23:01Z","receivedAt":"2017-11-27T05:23:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Liam Beguin writes:\n\n> Liam Beguin (5):\n> Documentation: move rebase.* configs to new file\n> Documentation: use preferred name for the 'todo list' script\n> rebase -i: add exec commands via the rebase--helper\n> rebase -i: learn to abbreviate command names\n> t3404: add test case for abbreviated commands\n\nI didn't send any comment on [1&2/5] but they both looked good.\n"},{"id":"333594","messageId":"xmqqr2skcmte.fsf@gitster.mtv.corp.google.com","threadId":"47327","inReplyTo":"20171127045514.25647-6-liambeguin@gmail.com","subject":"Re: [PATCH 5/5] t3404: add test case for abbreviated commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-27T05:28:29Z","receivedAt":"2017-11-27T05:28:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Liam Beguin writes:\n\n> Make sure the todo list ends up using single-letter command\n> abbreviations when the rebase.abbreviateCommands is enabled.\n> This configuration options should not change anything else.\n>\n> Signed-off-by: Liam Beguin \n> ---\n> t/t3404-rebase-interactive.sh | 32 ++++++++++++++++++++++++++++++++\n> 1 file changed, 32 insertions(+)\n>\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 6a82d1ed876d..e460ebde3393 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -1260,6 +1260,38 @@ test_expect_success 'rebase -i respects rebase.missingCommitsCheck = error' '\n> \ttest B = $(git cat-file commit HEAD^ | sed -ne \\$p)\n> '\n> \n> +test_expect_success 'prepare rebase.abbreviateCommands' '\n> +\treset_rebase &&\n> +\tgit checkout -b abbrevcmd master &&\n> +\ttest_commit \"first\" file1.txt \"first line\" first &&\n> +\ttest_commit \"second\" file1.txt \"another line\" second &&\n> +\ttest_commit \"fixup! first\" file2.txt \"first line again\" first_fixup &&\n> +\ttest_commit \"squash! second\" file1.txt \"another line here\" second_squash\n> +'\n> +\n> +cat >expected < +p $(git rev-list --abbrev-commit -1 first) first\n> +f $(git rev-list --abbrev-commit -1 first_fixup) fixup! first\n> +x git show HEAD\n> +p $(git rev-list --abbrev-commit -1 second) second\n> +s $(git rev-list --abbrev-commit -1 second_squash) squash! second\n> +x git show HEAD\n> +EOF\n\nPlease have this cat inside and at the beginning of the next\ntest_expect_success, preferably indented with HT to align with other\ncommands (adding '-' to the opening of your here-document makes the\nshell strip all leading HTs from the here-document), like this:\n\n\ttest_expect_success 'respects rebase....' '\n\t\tcat expect <<-EOF &&\n\t\tp $(git rev-list ...)\n\t\tf $(git rev-list ...)\n\t\t...\n\t\tEOF\n\t\ttest_when_finished \"\n \t...\n\t\t\" &&\n\t\t...\n\t\ttest_cmp expect actual\n\t'\n\n"},{"id":"333640","messageId":"alpine.DEB.2.21.1.1711272226500.6482@virtualbox","threadId":"47327","inReplyTo":"20171127045514.25647-2-liambeguin@gmail.com","subject":"Re: [PATCH 1/5] Documentation: move rebase.* configs to new file","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-27T21:27:13Z","receivedAt":"2017-11-27T21:27:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Liam,\n\nOn Sun, 26 Nov 2017, Liam Beguin wrote:\n\n> 3 files changed, 34 insertions(+), 48 deletions(-)\n\nVery nice!\nJohannes\n"},{"id":"333641","messageId":"alpine.DEB.2.21.1.1711272227520.6482@virtualbox","threadId":"47327","inReplyTo":"20171127045514.25647-3-liambeguin@gmail.com","subject":"Re: [PATCH 2/5] Documentation: use preferred name for the 'todo list' script","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-27T21:28:05Z","receivedAt":"2017-11-27T21:28:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Liam,\n\nOn Sun, 26 Nov 2017, Liam Beguin wrote:\n\n> Use \"todo list\" instead of \"instruction list\" or \"todo-list\" to\n> reduce further confusion regarding the name of this script.\n\nMakes sense,\nJohannes\n"},{"id":"333642","messageId":"alpine.DEB.2.21.1.1711272230490.6482@virtualbox","threadId":"47327","inReplyTo":"xmqq609we20v.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/5] rebase -i: add exec commands via the rebase--helper","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-27T21:41:37Z","receivedAt":"2017-11-27T21:42:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 27 Nov 2017, Junio C Hamano wrote:\n\n> Liam Beguin writes:\n> \n> > diff --git a/sequencer.c b/sequencer.c\n> > index fa94ed652d2c..810b7850748e 100644\n> > --- a/sequencer.c\n> > +++ b/sequencer.c\n> > @@ -2492,6 +2492,52 @@ int sequencer_make_script(int keep_empty, FILE *out,\n> > \treturn 0;\n> > }\n> > \n> > +int add_exec_commands(const char *command)\n> > +{\n> \n> As the name of a public function, it does not feel that this hints\n> it strongly enough that it is from and a part of sequencer.c API.\n\nHow about a \"yes, and\" instead? As in:\n\nTo further improve this patch, let's use the name\nsequencer_add_exec_commands() for this function because it is defined\nglobally now.\n\n> > +\tconst char *todo_file = rebase_path_todo();\n> > +\tstruct todo_list todo_list = TODO_LIST_INIT;\n> > +\tint fd, res, i, first = 1;\n> > +\tFILE *out;\n> \n> Having had to scan backwards while trying to see what the loop that\n> uses this variable is doing and if it gets affected by things that\n> happened before we entered the loop, I'd rather not to see 'first'\n> initialized here, left unused for quite some time until the loop is\n> entered. It would make it a lot easier to follow if it is declared\n> and left uninitilized here, and set to 1 immediately before the\n> for() loop that uses it.\n\nFunny, I would have assumed it the other way round: since \"first\" always\nhas to be initialized with 1, I would have been surprised to see an\nexplicit assignment much later than it is declared.\n\n> > +\tstrbuf_reset(&todo_list.buf);\n> > +\tfd = open(todo_file, O_RDONLY);\n> > +\tif (fd < 0)\n> > +\t\treturn error_errno(_(\"could not open '%s'\"), todo_file);\n> > +\tif (strbuf_read(&todo_list.buf, fd, 0) < 0) {\n> > +\t\tclose(fd);\n> > +\t\treturn error(_(\"could not read '%s'.\"), todo_file);\n> > +\t}\n> > +\tclose(fd);\n> \n> Is this strbuf_read_file() written in longhand?\n\nAh, this is one of the downsides of patch-based review. If it was reviewed\nin context, you would have easily spotted that Liam was merely\ncopy-editing my code that is still around.\n\nAnd indeed, I had missed that function when I started to write the\nrebase--helper patches.\n\n> > +\tres = parse_insn_buffer(todo_list.buf.buf, &todo_list);\n> > +\tif (res) {\n> > +\t\ttodo_list_release(&todo_list);\n> > +\t\treturn error(_(\"unusable todo list: '%s'\"), todo_file);\n> > +\t}\n> > +\n> > +\tout = fopen(todo_file, \"w\");\n> > +\tif (!out) {\n> > +\t\ttodo_list_release(&todo_list);\n> > +\t\treturn error(_(\"unable to open '%s' for writing\"), todo_file);\n> > +\t}\n> > +\tfor (i = 0; i < todo_list.nr; i++) {\n> > +\t\tstruct todo_item *item = todo_list.items + i;\n> > +\t\tint bol = item->offset_in_buf;\n> > +\t\tconst char *p = todo_list.buf.buf + bol;\n> > +\t\tint eol = i + 1 < todo_list.nr ?\n> > +\t\t\ttodo_list.items[i + 1].offset_in_buf :\n> > +\t\t\ttodo_list.buf.len;\n> \n> Should bol and eol be of type size_t instead? The values that get\n> assigned to them from other structures are.\n\nWhile it won't matter in practice, this would be \"more correct\" to do,\nyes.\n\nCiao,\nDscho\n"},{"id":"333644","messageId":"alpine.DEB.2.21.1.1711272241590.6482@virtualbox","threadId":"47327","inReplyTo":"20171127045514.25647-4-liambeguin@gmail.com","subject":"Re: [PATCH 3/5] rebase -i: add exec commands via the rebase--helper","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-27T22:42:32Z","receivedAt":"2017-11-27T22:42:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Liam,\n\ncould I ask for a favor? I'd like the oneline to start with\n\n\trebase -i -x: ...\n\n(this would help future me to realize what this commit touches already\nfrom the concise graph output I favor).\n\nOn Sun, 26 Nov 2017, Liam Beguin wrote:\n\n> Recent work on `git-rebase--interactive` aim to convert shell code to C.\n> Even if this is most likely not a big performance enhacement, let's\n> convert it too since a comming change to abbreviate command names requires\n> it to be updated.\n\nSince Junio did not comment on the commit message: could you replace\n`aim` by `aims`, `enhacement` by `enhancement` and `comming` by `coming`?\n\n> @@ -36,6 +37,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n> \t\t\tN_(\"skip unnecessary picks\"), SKIP_UNNECESSARY_PICKS),\n> \t\tOPT_CMDMODE(0, \"rearrange-squash\", &command,\n> \t\t\tN_(\"rearrange fixup/squash lines\"), REARRANGE_SQUASH),\n> +\t\tOPT_CMDMODE(0, \"add-exec\", &command,\n> +\t\t\tN_(\"insert exec commands in todo list\"), ADD_EXEC),\n\nMaybe `add-exec-commands`? I know it is longer to type, but these options do\nnot need to be typed interactively and the longer name would be consistent\nwith the function name.\n\n> diff --git a/sequencer.c b/sequencer.c\n> index fa94ed652d2c..810b7850748e 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2492,6 +2492,52 @@ int sequencer_make_script(int keep_empty, FILE *out,\n> \treturn 0;\n> }\n> \n\nAs the code in add_exec_commands() may appear convoluted (why not simply\nappend the command after any pick?), the original comment would be really\nnice here:\n\n\t/*\n\t * Add commands after pick and (series of) squash/fixup commands\n\t * in the todo list.\n\t */\n\n> +int add_exec_commands(const char *command)\n> +{\n> +\tconst char *todo_file = rebase_path_todo();\n> +\tstruct todo_list todo_list = TODO_LIST_INIT;\n> +\tint fd, res, i, first = 1;\n> +\tFILE *out;\n> +\n> +\tstrbuf_reset(&todo_list.buf);\n\nThe todo_list.buf has been initialized already (via TODO_LIST_INIT), no\nneed to reset it again.\n\n> +\tfd = open(todo_file, O_RDONLY);\n> +\tif (fd < 0)\n> +\t\treturn error_errno(_(\"could not open '%s'\"), todo_file);\n> +\tif (strbuf_read(&todo_list.buf, fd, 0) < 0) {\n> +\t\tclose(fd);\n> +\t\treturn error(_(\"could not read '%s'.\"), todo_file);\n> +\t}\n> +\tclose(fd);\n\nAs Junio pointed out so gently: there is a helper function that does this\nall very conveniently for us:\n\n\tif (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)\n\t\treturn error_errno(_(\"could not read '%s'\"), todo_file);\n\nAnd as I realized looking at the surrounding code: you probably just\ninherited my inelegant code by copy-editing from another function in\nsequencer.c. Should you decide to add a preparatory patch to your patch\nseries that converts these other callers, or even refactors all that code\nthat reads the git-rebase-todo file and then parses it, I would be quite\nhappy... :-) (although I would understand if you deemed this outside the\npurpose of your patch series).\n\n> +\tres = parse_insn_buffer(todo_list.buf.buf, &todo_list);\n> +\tif (res) {\n> +\t\ttodo_list_release(&todo_list);\n> +\t\treturn error(_(\"unusable todo list: '%s'\"), todo_file);\n> +\t}\n\nThe variable `res` is not really used here. Let's just put the\nparse_insn_buffer() call inside the if ().\n\n> +\tout = fopen(todo_file, \"w\");\n> +\tif (!out) {\n> +\t\ttodo_list_release(&todo_list);\n> +\t\treturn error(_(\"unable to open '%s' for writing\"), todo_file);\n> +\t}\n> +\tfor (i = 0; i < todo_list.nr; i++) {\n> +\t\tstruct todo_item *item = todo_list.items + i;\n> +\t\tint bol = item->offset_in_buf;\n> +\t\tconst char *p = todo_list.buf.buf + bol;\n> +\t\tint eol = i + 1 < todo_list.nr ?\n> +\t\t\ttodo_list.items[i + 1].offset_in_buf :\n> +\t\t\ttodo_list.buf.len;\n\nThis smells like another copy-edited snippet that originated from my\nbrain, and I am not at all proud by the complexity I used there.\n\nThe function should also check for errors during writing. So how about\nsomething like this instead?\n\n\tstruct strbuf *buf = &todo_list.buf;\n\tsize_t offset = 0, command_len = strlen(command);\n\tint first = 1, i;\n\tstruct todo_item *item;\n\n\t...\n\n\t/* insert before every pick except the first one */\n\tfor (item = todo_list.items, i = 0; i < todo_list.nr; i++, item++)\n\t\tif (item->command == TODO_PICK) {\n\t\t\tif (first)\n\t\t\t\tfirst = 0;\n\t\t\telse {\n\t\t\t\tstrbuf_splice(buf,\n\t\t\t\t\t item->offset_in_buf + offset, 0,\n\t\t\t\t\t command, command_len);\n\t\t\t\toffset += command_len;\n\t\t\t}\n\t\t}\n\n\t/* append a final */\n\tstrbuf_complete_list(buf);\n\tstrbuf_add(buf, command, command_len);\n\n\ti = write_message(buf->buf, buf->len, todo_file, 0);\n\ttodo_list_release(&todo_list);\n\treturn i;\n\nCiao,\nDscho\n"},{"id":"333646","messageId":"alpine.DEB.2.21.1.1711272344290.6482@virtualbox","threadId":"47327","inReplyTo":"20171127045514.25647-5-liambeguin@gmail.com","subject":"Re: [PATCH 4/5] rebase -i: learn to abbreviate command names","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-27T23:04:45Z","receivedAt":"2017-11-27T23:04:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Liam,\n\nOn Sun, 26 Nov 2017, Liam Beguin wrote:\n\n> diff --git a/Documentation/rebase-config.txt b/Documentation/rebase-config.txt\n> index 30ae08cb5a4b..0820b60f6e12 100644\n> --- a/Documentation/rebase-config.txt\n> +++ b/Documentation/rebase-config.txt\n> @@ -30,3 +30,22 @@ rebase.instructionFormat::\n> \tA format string, as specified in linkgit:git-log[1], to be used for the\n> \ttodo list during an interactive rebase. The format will\n> \tautomatically have the long commit hash prepended to the format.\n> +\n> +rebase.abbreviateCommands::\n> +\tIf set to true, `git rebase` will use abbreviated command names in the\n> +\ttodo list resulting in something like this:\n> +\n> +-------------------------------------------\n> +\tp deadbee The oneline of the commit\n> +\tp fa1afe1 The oneline of the next commit\n> +\t...\n> +-------------------------------------------\n\nI *think* that AsciiDoc will render this in a different way from what we\nwant, but I am not an AsciiDoc expert. In my hands, I always had to add a\nsingle + in an otherwise empty line to start a new indented paragraph *and\nthen continue with non-indented lines*.\n\n> diff --git a/sequencer.c b/sequencer.c\n> index 810b7850748e..aa01e8bd9280 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -795,6 +795,13 @@ static const char *command_to_string(const enum todo_command command)\n> \tdie(\"Unknown command: %d\", command);\n> }\n> \n> +static const char command_to_char(const enum todo_command command)\n> +{\n> +\tif (command < TODO_COMMENT && todo_command_info[command].c)\n> +\t\treturn todo_command_info[command].c;\n> +\treturn -1;\n\nMy initial reaction was: should we return comment_line_char instead of -1\nhere? Only after reading how this is called did I realize that the idea is\nto use full command names if there is no abbreviation. Not sure whether\nthis is worth a code comment. What do you think?\n\n> +}\n> +\n> static int is_noop(const enum todo_command command)\n> {\n> \treturn TODO_NOOP <= command;\n> @@ -1242,15 +1249,16 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)\n> \t\treturn 0;\n> \t}\n> \n> -\tfor (i = 0; i < TODO_COMMENT; i++)\n> +\tfor (i = 0; i < TODO_COMMENT; i++) {\n> \t\tif (skip_prefix(bol, todo_command_info[i].str, &bol)) {\n> \t\t\titem->command = i;\n> \t\t\tbreak;\n> -\t\t} else if (bol[1] == ' ' && *bol == todo_command_info[i].c) {\n> +\t\t} else if (bol[1] == ' ' && *bol == command_to_char(i)) {\n> \t\t\tbol++;\n> \t\t\titem->command = i;\n> \t\t\tbreak;\n> \t\t}\n> +\t}\n> \tif (i >= TODO_COMMENT)\n> \t\treturn -1;\n> \n\nI would prefer this hunk to be skipped, it does not really do anything if\nI understand correctly.\n\n> @@ -2443,8 +2451,8 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)\n> \tstrbuf_release(&sob);\n> }\n> \n> -int sequencer_make_script(int keep_empty, FILE *out,\n> -\t\tint argc, const char **argv)\n> +int sequencer_make_script(int keep_empty, int abbreviate_commands, FILE *out,\n> +\t\t\t int argc, const char **argv)\n> {\n> \tchar *format = NULL;\n> \tstruct pretty_print_context pp = {0};\n> @@ -2483,7 +2491,9 @@ int sequencer_make_script(int keep_empty, FILE *out,\n> \t\tstrbuf_reset(&buf);\n> \t\tif (!keep_empty && is_original_commit_empty(commit))\n> \t\t\tstrbuf_addf(&buf, \"%c \", comment_line_char);\n> -\t\tstrbuf_addf(&buf, \"pick %s \", oid_to_hex(&commit->object.oid));\n> +\t\tstrbuf_addf(&buf, \"%s %s \",\n> +\t\t\t abbreviate_commands ? \"p\" : \"pick\",\n> +\t\t\t oid_to_hex(&commit->object.oid));\n\nI guess the compiler will optimize this code so that the conditional is\nevaluated only once. Not that this is performance critical ;-)\n\n> \t\tpretty_print_commit(&pp, commit, &buf);\n> \t\tstrbuf_addch(&buf, '\\n');\n> \t\tfputs(buf.buf, out);\n> @@ -2539,7 +2549,7 @@ int add_exec_commands(const char *command)\n> \treturn 0;\n> }\n> \n> -int transform_todo_ids(int shorten_ids)\n> +int transform_todo_ids(int shorten_ids, int abbreviate_commands)\n> {\n> \tconst char *todo_file = rebase_path_todo();\n> \tstruct todo_list todo_list = TODO_LIST_INIT;\n> @@ -2575,19 +2585,33 @@ int transform_todo_ids(int shorten_ids)\n> \t\t\ttodo_list.items[i + 1].offset_in_buf :\n> \t\t\ttodo_list.buf.len;\n> \n> -\t\tif (item->command >= TODO_EXEC && item->command != TODO_DROP)\n> -\t\t\tfwrite(p, eol - bol, 1, out);\n> -\t\telse {\n> +\t\tif (item->command >= TODO_EXEC && item->command != TODO_DROP) {\n> +\t\t\tif (!abbreviate_commands || command_to_char(item->command) < 0) {\n> +\t\t\t\tfwrite(p, eol - bol, 1, out);\n> +\t\t\t} else {\n> +\t\t\t\tconst char *end_of_line = strchrnul(p, '\\n');\n> +\t\t\t\tp += strspn(p, \" \\t\"); /* skip whitespace */\n> +\t\t\t\tp += strcspn(p, \" \\t\"); /* skip command */\n> +\t\t\t\tfprintf(out, \"%c%.*s\\n\",\n> +\t\t\t\t\tcommand_to_char(item->command),\n> +\t\t\t\t\t(int)(end_of_line - p), p);\n> +\t\t\t}\n> +\t\t} else {\n> \t\t\tconst char *id = shorten_ids ?\n> \t\t\t\tshort_commit_name(item->commit) :\n> \t\t\t\toid_to_hex(&item->commit->object.oid);\n> -\t\t\tint len;\n> \n> -\t\t\tp += strspn(p, \" \\t\"); /* left-trim command */\n> -\t\t\tlen = strcspn(p, \" \\t\"); /* length of command */\n> -\n> -\t\t\tfprintf(out, \"%.*s %s %.*s\\n\",\n> -\t\t\t\tlen, p, id, item->arg_len, item->arg);\n> +\t\t\tif (abbreviate_commands) {\n> +\t\t\t\tfprintf(out, \"%c %s %.*s\\n\",\n> +\t\t\t\t\tcommand_to_char(item->command),\n> +\t\t\t\t\tid, item->arg_len, item->arg);\n> +\t\t\t} else {\n> +\t\t\t\tint len;\n> +\t\t\t\tp += strspn(p, \" \\t\"); /* left-trim command */\n> +\t\t\t\tlen = strcspn(p, \" \\t\"); /* length of command */\n> +\t\t\t\tfprintf(out, \"%.*s %s %.*s\\n\",\n> +\t\t\t\t\tlen, p, id, item->arg_len, item->arg);\n> +\t\t\t}\n\nThis hunk changes indentation quite a bit, therefore it is a bit harder to\nread than necessary (and the resulting code, too, as it is more smooshed\nagainst the 80-column boundary on the right).\n\nHow about this instead:\n\n-\t\tif (item->command >= TODO_EXEC && item->command != TODO_DROP)\n+\t\tif (abbreviate_commands && command_to_char(item->command)) {\n+\t\t\tconst char *id = shorten_ids ?\n+\t\t\t\tshort_commit_name(item->commit) :\n+\t\t\t\toid_to_hex(&item->commit->object.oid);\n+\t\t\tfprintf(out, \"%c %s %.*s\\n\",\n+\t\t\t\tcommand_to_char(item->command),\n+\t\t\t\tid, item->arg_len, item->arg);\n+\t\t} else if (item->command >= TODO_EXEC &&\n+\t\t\t item->command != TODO_DROP)\n\ni.e. test first for the short and sweet case that we want (and can)\nabbreviate the command, otherwise keep the code as before?\n\nCiao,\nDscho\n"},{"id":"333648","messageId":"20171127231131.GB29636@sigill.intra.peff.net","threadId":"47327","inReplyTo":"alpine.DEB.2.21.1.1711272344290.6482@virtualbox","subject":"Re: [PATCH 4/5] rebase -i: learn to abbreviate command names","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-27T23:11:31Z","receivedAt":"2017-11-27T23:11:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 28, 2017 at 12:04:45AM +0100, Johannes Schindelin wrote:\n\n> > +rebase.abbreviateCommands::\n> > +\tIf set to true, `git rebase` will use abbreviated command names in the\n> > +\ttodo list resulting in something like this:\n> > +\n> > +-------------------------------------------\n> > +\tp deadbee The oneline of the commit\n> > +\tp fa1afe1 The oneline of the next commit\n> > +\t...\n> > +-------------------------------------------\n> \n> I *think* that AsciiDoc will render this in a different way from what we\n> want, but I am not an AsciiDoc expert. In my hands, I always had to add a\n> single + in an otherwise empty line to start a new indented paragraph *and\n> then continue with non-indented lines*.\n\nGood catch. Interestingly enough, my asciidoc seems to render this\nas desired for the docbook/roff version, but has screwed-up indentation\nfor the HTML version.\n\nFixing it as you suggest makes it look good in both (and I think you can\nnever go wrong with \"+\"-continuation, aside from making the source a bit\nuglier).\n\nSquashable patch below for convenience, since I did try it.\n\n-Peff\n\ndiff --git a/Documentation/rebase-config.txt b/Documentation/rebase-config.txt\nindex 0820b60f6e..42e1ba7575 100644\n--- a/Documentation/rebase-config.txt\n+++ b/Documentation/rebase-config.txt\n@@ -34,18 +34,19 @@ rebase.instructionFormat::\n rebase.abbreviateCommands::\n \tIf set to true, `git rebase` will use abbreviated command names in the\n \ttodo list resulting in something like this:\n-\n++\n -------------------------------------------\n \tp deadbee The oneline of the commit\n \tp fa1afe1 The oneline of the next commit\n \t...\n -------------------------------------------\n-\n-\tinstead of:\n-\n++\n+instead of:\n++\n -------------------------------------------\n \tpick deadbee The oneline of the commit\n \tpick fa1afe1 The oneline of the next commit\n \t...\n -------------------------------------------\n-\tDefaults to false.\n++\n+Defaults to false.\n"},{"id":"333649","messageId":"alpine.DEB.2.21.1.1711280007160.6482@virtualbox","threadId":"47327","inReplyTo":"20171127045514.25647-6-liambeguin@gmail.com","subject":"Re: [PATCH 5/5] t3404: add test case for abbreviated commands","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-27T23:16:02Z","receivedAt":"2017-11-27T23:16:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Liam,\n\nOn Sun, 26 Nov 2017, Liam Beguin wrote:\n\n> Make sure the todo list ends up using single-letter command\n> abbreviations when the rebase.abbreviateCommands is enabled.\n> This configuration options should not change anything else.\n\nMakes sense. As to the diff:\n\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 6a82d1ed876d..e460ebde3393 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -1260,6 +1260,38 @@ test_expect_success 'rebase -i respects rebase.missingCommitsCheck = error' '\n> \ttest B = $(git cat-file commit HEAD^ | sed -ne \\$p)\n> '\n> \n> +test_expect_success 'prepare rebase.abbreviateCommands' '\n> +\treset_rebase &&\n> +\tgit checkout -b abbrevcmd master &&\n> +\ttest_commit \"first\" file1.txt \"first line\" first &&\n> +\ttest_commit \"second\" file1.txt \"another line\" second &&\n> +\ttest_commit \"fixup! first\" file2.txt \"first line again\" first_fixup &&\n> +\ttest_commit \"squash! second\" file1.txt \"another line here\" second_squash\n> +'\n\nIn addition to Junio's suggestion to include the \"expected\" block in the\nnext test case, I would be in favor of combining all the new code in a\nsingle test case.\n\nAlso, I think that the test_commit calls can be simplified to:\n\n\ttest_commit first &&\n\ttest_commit second &&\n\ttest_commit \"fixup! first\" first A dummy1 &&\n\ttest_commit \"squash! second\" second B dummy2 &&\n\n> +cat >expected < +p $(git rev-list --abbrev-commit -1 first) first\n\nMaybe $(git rev-parse --short HEAD~3)?\n\n> +f $(git rev-list --abbrev-commit -1 first_fixup) fixup! first\n> +x git show HEAD\n> +p $(git rev-list --abbrev-commit -1 second) second\n> +s $(git rev-list --abbrev-commit -1 second_squash) squash! second\n> +x git show HEAD\n> +EOF\n> +\n> +test_expect_success 'respects rebase.abbreviateCommands with fixup, squash and exec' '\n> +\ttest_when_finished \"\n> +\t\tgit checkout master &&\n> +\t\ttest_might_fail git branch -D abbrevcmd &&\n> +\t\ttest_might_fail git rebase --abort\n> +\t\" &&\n> +\tgit checkout abbrevcmd &&\n> +\tset_cat_todo_editor &&\n> +\ttest_config rebase.abbreviateCommands true &&\n> +\ttest_must_fail git rebase -i --exec \"git show HEAD\" \\\n> +\t\t--autosquash master >actual &&\n> +\ttest_cmp expected actual\n> +'\n\nOtherwise, it looks good!\n\nThank you for staying on the ball and getting this patch series updated.\n\nCiao,\nDscho\n"},{"id":"333656","messageId":"xmqqk1yb9tgu.fsf@gitster.mtv.corp.google.com","threadId":"47327","inReplyTo":"alpine.DEB.2.21.1.1711272230490.6482@virtualbox","subject":"Re: [PATCH 3/5] rebase -i: add exec commands via the rebase--helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-27T23:45:21Z","receivedAt":"2017-11-27T23:45:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> As the name of a public function, it does not feel that this hints\n>> it strongly enough that it is from and a part of sequencer.c API.\n>\n> How about a \"yes, and\" instead? As in:\n>\n> To further improve this patch, let's use the name\n> sequencer_add_exec_commands() for this function because it is defined\n> globally now.\n\nI would do so when I have a single \"this is strictly better\"\nsuggestion. In this case, I didn't, but somebody who does not have\na \"better suggestion\" can still have a good sense of smell to tell\nsomething is \"not right\".\n\n>> > +\tconst char *todo_file = rebase_path_todo();\n>> > +\tstruct todo_list todo_list = TODO_LIST_INIT;\n>> > +\tint fd, res, i, first = 1;\n>> > +\tFILE *out;\n>> \n>> Having had to scan backwards while trying to see what the loop that\n>> uses this variable is doing and if it gets affected by things that\n>> happened before we entered the loop, I'd rather not to see 'first'\n>> initialized here, left unused for quite some time until the loop is\n>> entered. It would make it a lot easier to follow if it is declared\n>> and left uninitilized here, and set to 1 immediately before the\n>> for() loop that uses it.\n>\n> Funny, I would have assumed it the other way round: since \"first\" always\n> has to be initialized with 1, I would have been surprised to see an\n> explicit assignment much later than it is declared.\n\nUnfortunately that would force readers to see what happens before\nthe loop to see if there are cases where first is incremented, and\nin this case there is not any.\n"},{"id":"333657","messageId":"xmqqfu8z9tbi.fsf@gitster.mtv.corp.google.com","threadId":"47327","inReplyTo":"alpine.DEB.2.21.1.1711272241590.6482@virtualbox","subject":"Re: [PATCH 3/5] rebase -i: add exec commands via the rebase--helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-27T23:48:33Z","receivedAt":"2017-11-27T23:48:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> could I ask for a favor? I'd like the oneline to start with\n>\n> \trebase -i -x: ...\n>\n> (this would help future me to realize what this commit touches already\n> from the concise graph output I favor).\n\nExcellent.\n\n>> Recent work on `git-rebase--interactive` aim to convert shell code to C.\n>> Even if this is most likely not a big performance enhacement, let's\n>> convert it too since a comming change to abbreviate command names requires\n>> it to be updated.\n>\n> Since Junio did not comment on the commit message: could you replace\n> `aim` by `aims`, `enhacement` by `enhancement` and `comming` by `coming`?\n\nYes, I noticed them but don't mind me ;-) The above are all good fixes.\n\nAll suggestions in the remainder looked sensible. Thanks for a\nreview.\n"},{"id":"333757","messageId":"02d5cf10-7c3d-c5f6-fa9a-6b440c6a60c6@gmail.com","threadId":"47327","inReplyTo":"xmqqwp2ccn2i.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/5] rebase -i: add config to abbreviate command names","fromName":"liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-11-29T01:56:11Z","receivedAt":"2017-11-29T01:56:19Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Hi Junio,\n\nOn 27/11/17 12:23 AM, Junio C Hamano wrote:\n> Liam Beguin writes:\n> \n>> Liam Beguin (5):\n>> Documentation: move rebase.* configs to new file\n>> Documentation: use preferred name for the 'todo list' script\n>> rebase -i: add exec commands via the rebase--helper\n>> rebase -i: learn to abbreviate command names\n>> t3404: add test case for abbreviated commands\n> \n> I didn't send any comment on [1&2/5] but they both looked good.\n> \n\nThanks for reviewing this. I'll go through your comments and post a\nv2 shortly.\n\nThanks,\nLiam\n\nPS: I'm very sorry if someone received a few copies of this, I'm\nhaving issues with my MUA! Hopefully, I've got it right this time...\n"},{"id":"333758","messageId":"46cf2ed9-ea95-5ba9-e0f1-3ed7b524279e@gmail.com","threadId":"47327","inReplyTo":"xmqq609we20v.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/5] rebase -i: add exec commands via the rebase--helper","fromName":"liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-11-29T02:01:25Z","receivedAt":"2017-11-29T02:01:39Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Hi Junio,\n\nOn 27/11/17 12:14 AM, Junio C Hamano wrote:\n> Liam Beguin writes:\n> \n>> diff --git a/sequencer.c b/sequencer.c\n>> index fa94ed652d2c..810b7850748e 100644\n>> --- a/sequencer.c\n>> +++ b/sequencer.c\n>> @@ -2492,6 +2492,52 @@ int sequencer_make_script(int keep_empty, FILE *out,\n>> \treturn 0;\n>> }\n>> \n>> +int add_exec_commands(const char *command)\n>> +{\n> \n> As the name of a public function, it does not feel that this hints\n> it strongly enough that it is from and a part of sequencer.c API.\n> \n>> +\tconst char *todo_file = rebase_path_todo();\n>> +\tstruct todo_list todo_list = TODO_LIST_INIT;\n>> +\tint fd, res, i, first = 1;\n>> +\tFILE *out;\n> \n> Having had to scan backwards while trying to see what the loop that\n> uses this variable is doing and if it gets affected by things that\n> happened before we entered the loop, I'd rather not to see 'first'\n> initialized here, left unused for quite some time until the loop is\n> entered. It would make it a lot easier to follow if it is declared\n> and left uninitilized here, and set to 1 immediately before the\n> for() loop that uses it.\n> \n\nI agree that moving 'first = 1' just above the for() loop makes it\nmore obvious. I'm not quite fond of how this is implemented, I just\n'translated' the shell code and was hoping on maybe a few comments\non how to improve it.\n\n>> +\n>> +\tstrbuf_reset(&todo_list.buf);\n>> +\tfd = open(todo_file, O_RDONLY);\n>> +\tif (fd < 0)\n>> +\t\treturn error_errno(_(\"could not open '%s'\"), todo_file);\n>> +\tif (strbuf_read(&todo_list.buf, fd, 0) < 0) {\n>> +\t\tclose(fd);\n>> +\t\treturn error(_(\"could not read '%s'.\"), todo_file);\n>> +\t}\n>> +\tclose(fd);\n> \n> Is this strbuf_read_file() written in longhand?\n\nThanks for pointing this out! I'll update. And as Johannes pointed out,\nI've copied this from surrounding functions, I'll add a preparatory path\nto update those too.\n\n> \n>> +\tres = parse_insn_buffer(todo_list.buf.buf, &todo_list);\n>> +\tif (res) {\n>> +\t\ttodo_list_release(&todo_list);\n>> +\t\treturn error(_(\"unusable todo list: '%s'\"), todo_file);\n>> +\t}\n>> +\n>> +\tout = fopen(todo_file, \"w\");\n>> +\tif (!out) {\n>> +\t\ttodo_list_release(&todo_list);\n>> +\t\treturn error(_(\"unable to open '%s' for writing\"), todo_file);\n>> +\t}\n>> +\tfor (i = 0; i < todo_list.nr; i++) {\n>> +\t\tstruct todo_item *item = todo_list.items + i;\n>> +\t\tint bol = item->offset_in_buf;\n>> +\t\tconst char *p = todo_list.buf.buf + bol;\n>> +\t\tint eol = i + 1 < todo_list.nr ?\n>> +\t\t\ttodo_list.items[i + 1].offset_in_buf :\n>> +\t\t\ttodo_list.buf.len;\n> \n> Should bol and eol be of type size_t instead? The values that get\n> assigned to them from other structures are.\n> \n\nWill do.\nThanks, \n\nLiam\n"},{"id":"333760","messageId":"6b4e8352-0583-11c2-43ac-ec4ab33cc554@gmail.com","threadId":"47327","inReplyTo":"alpine.DEB.2.21.1.1711272241590.6482@virtualbox","subject":"Re: [PATCH 3/5] rebase -i: add exec commands via the rebase--helper","fromName":"liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-11-29T02:06:53Z","receivedAt":"2017-11-29T02:07:02Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Hi Johannes,\n\nThanks for taking the time to review this.\n\nOn 27/11/17 05:42 PM, Johannes Schindelin wrote:\n> Hi Liam,\n> \n> could I ask for a favor? I'd like the oneline to start with\n> \n> \trebase -i -x: ...\n> \n> (this would help future me to realize what this commit touches already\n> from the concise graph output I favor).\n\nSure, I'll update the commit subject.\n\n> \n> On Sun, 26 Nov 2017, Liam Beguin wrote:\n> \n>> Recent work on `git-rebase--interactive` aim to convert shell code to C.\n>> Even if this is most likely not a big performance enhacement, let's\n>> convert it too since a comming change to abbreviate command names requires\n>> it to be updated.\n> \n> Since Junio did not comment on the commit message: could you replace\n> `aim` by `aims`, `enhacement` by `enhancement` and `comming` by `coming`?\n\nOw.. sorry about that! I'll fix those and make sure to proofread better next time!\n\n> \n>> @@ -36,6 +37,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n>> \t\t\tN_(\"skip unnecessary picks\"), SKIP_UNNECESSARY_PICKS),\n>> \t\tOPT_CMDMODE(0, \"rearrange-squash\", &command,\n>> \t\t\tN_(\"rearrange fixup/squash lines\"), REARRANGE_SQUASH),\n>> +\t\tOPT_CMDMODE(0, \"add-exec\", &command,\n>> +\t\t\tN_(\"insert exec commands in todo list\"), ADD_EXEC),\n> \n> Maybe `add-exec-commands`? I know it is longer to type, but these options do\n> not need to be typed interactively and the longer name would be consistent\n> with the function name.\n\nMakes sense. It'll also be more consistent with the rest of the commands above.\n\n> \n>> diff --git a/sequencer.c b/sequencer.c\n>> index fa94ed652d2c..810b7850748e 100644\n>> --- a/sequencer.c\n>> +++ b/sequencer.c\n>> @@ -2492,6 +2492,52 @@ int sequencer_make_script(int keep_empty, FILE *out,\n>> \treturn 0;\n>> }\n>> \n> \n> As the code in add_exec_commands() may appear convoluted (why not simply\n> append the command after any pick?), the original comment would be really\n> nice here:\n> \n> \t/*\n> \t * Add commands after pick and (series of) squash/fixup commands\n> \t * in the todo list.\n> \t */\n> \n\nI'll make sure to include that comment.\nThe code is a bit convoluted as you say... I wanted to send it \"as is\" first\nto get comments and update based on feedback from the list.\n\nI just realized we could maybe add exec instructions only after pick commands\nif we do add-exec-commands before rearrange-squash. I'll test it out.\n\n>> +int add_exec_commands(const char *command)\n>> +{\n>> +\tconst char *todo_file = rebase_path_todo();\n>> +\tstruct todo_list todo_list = TODO_LIST_INIT;\n>> +\tint fd, res, i, first = 1;\n>> +\tFILE *out;\n>> +\n>> +\tstrbuf_reset(&todo_list.buf);\n> \n> The todo_list.buf has been initialized already (via TODO_LIST_INIT), no\n> need to reset it again.\n> \n>> +\tfd = open(todo_file, O_RDONLY);\n>> +\tif (fd < 0)\n>> +\t\treturn error_errno(_(\"could not open '%s'\"), todo_file);\n>> +\tif (strbuf_read(&todo_list.buf, fd, 0) < 0) {\n>> +\t\tclose(fd);\n>> +\t\treturn error(_(\"could not read '%s'.\"), todo_file);\n>> +\t}\n>> +\tclose(fd);\n> \n> As Junio pointed out so gently: there is a helper function that does this\n> all very conveniently for us:\n> \n> \tif (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)\n> \t\treturn error_errno(_(\"could not read '%s'\"), todo_file);\n> \n> And as I realized looking at the surrounding code: you probably just\n> inherited my inelegant code by copy-editing from another function in\n> sequencer.c. Should you decide to add a preparatory patch to your patch\n> series that converts these other callers, or even refactors all that code\n> that reads the git-rebase-todo file and then parses it, I would be quite\n> happy... :-) (although I would understand if you deemed this outside the\n> purpose of your patch series).\n> \n\nYou guessed well, I mostly did copy-editing... I thought I found this code\na little confusing because I'm not used to as much pointer gymnastics but\nit reassures me a bit to read this :-). I'll see if I can come up with a\nbetter solution.\n\n>> +\tres = parse_insn_buffer(todo_list.buf.buf, &todo_list);\n>> +\tif (res) {\n>> +\t\ttodo_list_release(&todo_list);\n>> +\t\treturn error(_(\"unusable todo list: '%s'\"), todo_file);\n>> +\t}\n> \n> The variable `res` is not really used here. Let's just put the\n> parse_insn_buffer() call inside the if ().\n> \n\nWill do.\n\n>> +\tout = fopen(todo_file, \"w\");\n>> +\tif (!out) {\n>> +\t\ttodo_list_release(&todo_list);\n>> +\t\treturn error(_(\"unable to open '%s' for writing\"), todo_file);\n>> +\t}\n>> +\tfor (i = 0; i < todo_list.nr; i++) {\n>> +\t\tstruct todo_item *item = todo_list.items + i;\n>> +\t\tint bol = item->offset_in_buf;\n>> +\t\tconst char *p = todo_list.buf.buf + bol;\n>> +\t\tint eol = i + 1 < todo_list.nr ?\n>> +\t\t\ttodo_list.items[i + 1].offset_in_buf :\n>> +\t\t\ttodo_list.buf.len;\n> \n> This smells like another copy-edited snippet that originated from my\n> brain, and I am not at all proud by the complexity I used there.\n> \n> The function should also check for errors during writing. So how about\n> something like this instead?\n> \n> \tstruct strbuf *buf = &todo_list.buf;\n> \tsize_t offset = 0, command_len = strlen(command);\n> \tint first = 1, i;\n> \tstruct todo_item *item;\n> \n> \t...\n> \n> \t/* insert before every pick except the first one */\n> \tfor (item = todo_list.items, i = 0; i < todo_list.nr; i++, item++)\n> \t\tif (item->command == TODO_PICK) {\n> \t\t\tif (first)\n> \t\t\t\tfirst = 0;\n> \t\t\telse {\n> \t\t\t\tstrbuf_splice(buf,\n> \t\t\t\t\t item->offset_in_buf + offset, 0,\n> \t\t\t\t\t command, command_len);\n> \t\t\t\toffset += command_len;\n> \t\t\t}\n> \t\t}\n> \n> \t/* append a final */\n> \tstrbuf_complete_list(buf);\n> \tstrbuf_add(buf, command, command_len);\n> \n> \ti = write_message(buf->buf, buf->len, todo_file, 0);\n> \ttodo_list_release(&todo_list);\n> \treturn i;\n> \n\nI'll see how I can include this if calling add-exec-commands before\nrearrange-squash works. But it definitely is lighter to read.\n\n> Ciao,\n> Dscho\n> \n\nThanks again,\n\nLiam\n"},{"id":"333761","messageId":"edecde30-dfde-89a7-3110-c791f4ee3a38@gmail.com","threadId":"47327","inReplyTo":"xmqq1skke1so.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 4/5] rebase -i: learn to abbreviate command names","fromName":"liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-11-29T02:08:52Z","receivedAt":"2017-11-29T02:08:58Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Hi Junio,\n\nOn 27/11/17 12:19 AM, Junio C Hamano wrote:\n> Liam Beguin writes:\n> \n>> \tif (command == MAKE_SCRIPT && argc > 1)\n>> -\t\treturn !!sequencer_make_script(keep_empty, stdout, argc, argv);\n>> +\t\treturn !!sequencer_make_script(keep_empty, abbreviate_commands,\n>> +\t\t\t\t\t stdout, argc, argv);\n> \n> This suggests that a preliminary clean-up to update the parameter\n> list of sequencer_make_script() is in order just before this step.\n> How about making it like so, perhaps:\n> \n> int sequencer_make_script(FILE *out, int ac, char **av, unsigned flags)\n> \n> where keep_empty becomes just one bit in that flags word. Then another\n> bit in the same flags word can be used for this option.\n> \n> Otherwise, every time somebody comes up with a new and shiny feature\n> for the function, we'd end up adding more to its parameter list.\n> \n\nWill do.\nThanks, \n\nLiam\n"},{"id":"333762","messageId":"b4331bb3-db5d-e4f5-54db-f04d77385ae7@gmail.com","threadId":"47327","inReplyTo":"alpine.DEB.2.21.1.1711272344290.6482@virtualbox","subject":"Re: [PATCH 4/5] rebase -i: learn to abbreviate command names","fromName":"liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-11-29T02:10:47Z","receivedAt":"2017-11-29T02:10:54Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Hi Johannes,\n\nOn 27/11/17 06:04 PM, Johannes Schindelin wrote:\n> Hi Liam,\n> \n> On Sun, 26 Nov 2017, Liam Beguin wrote:\n> \n>> diff --git a/Documentation/rebase-config.txt b/Documentation/rebase-config.txt\n>> index 30ae08cb5a4b..0820b60f6e12 100644\n>> --- a/Documentation/rebase-config.txt\n>> +++ b/Documentation/rebase-config.txt\n>> @@ -30,3 +30,22 @@ rebase.instructionFormat::\n>> \tA format string, as specified in linkgit:git-log[1], to be used for the\n>> \ttodo list during an interactive rebase. The format will\n>> \tautomatically have the long commit hash prepended to the format.\n>> +\n>> +rebase.abbreviateCommands::\n>> +\tIf set to true, `git rebase` will use abbreviated command names in the\n>> +\ttodo list resulting in something like this:\n>> +\n>> +-------------------------------------------\n>> +\tp deadbee The oneline of the commit\n>> +\tp fa1afe1 The oneline of the next commit\n>> +\t...\n>> +-------------------------------------------\n> \n> I *think* that AsciiDoc will render this in a different way from what we\n> want, but I am not an AsciiDoc expert. In my hands, I always had to add a\n> single + in an otherwise empty line to start a new indented paragraph *and\n> then continue with non-indented lines*.\n> \n>> diff --git a/sequencer.c b/sequencer.c\n>> index 810b7850748e..aa01e8bd9280 100644\n>> --- a/sequencer.c\n>> +++ b/sequencer.c\n>> @@ -795,6 +795,13 @@ static const char *command_to_string(const enum todo_command command)\n>> \tdie(\"Unknown command: %d\", command);\n>> }\n>> \n>> +static const char command_to_char(const enum todo_command command)\n>> +{\n>> +\tif (command < TODO_COMMENT && todo_command_info[command].c)\n>> +\t\treturn todo_command_info[command].c;\n>> +\treturn -1;\n> \n> My initial reaction was: should we return comment_line_char instead of -1\n> here? Only after reading how this is called did I realize that the idea is\n> to use full command names if there is no abbreviation. Not sure whether\n> this is worth a code comment. What do you think?\n> \n\nI guess it probably deserves a comment!\n\n>> +}\n>> +\n>> static int is_noop(const enum todo_command command)\n>> {\n>> \treturn TODO_NOOP <= command;\n>> @@ -1242,15 +1249,16 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)\n>> \t\treturn 0;\n>> \t}\n>> \n>> -\tfor (i = 0; i < TODO_COMMENT; i++)\n>> +\tfor (i = 0; i < TODO_COMMENT; i++) {\n>> \t\tif (skip_prefix(bol, todo_command_info[i].str, &bol)) {\n>> \t\t\titem->command = i;\n>> \t\t\tbreak;\n>> -\t\t} else if (bol[1] == ' ' && *bol == todo_command_info[i].c) {\n>> +\t\t} else if (bol[1] == ' ' && *bol == command_to_char(i)) {\n>> \t\t\tbol++;\n>> \t\t\titem->command = i;\n>> \t\t\tbreak;\n>> \t\t}\n>> +\t}\n>> \tif (i >= TODO_COMMENT)\n>> \t\treturn -1;\n>> \n> \n> I would prefer this hunk to be skipped, it does not really do anything if\n> I understand correctly.\n\nOk, I was not so sure about this but thought it was probably worth it.\nWill remove.\n\n> \n>> @@ -2443,8 +2451,8 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)\n>> \tstrbuf_release(&sob);\n>> }\n>> \n>> -int sequencer_make_script(int keep_empty, FILE *out,\n>> -\t\tint argc, const char **argv)\n>> +int sequencer_make_script(int keep_empty, int abbreviate_commands, FILE *out,\n>> +\t\t\t int argc, const char **argv)\n>> {\n>> \tchar *format = NULL;\n>> \tstruct pretty_print_context pp = {0};\n>> @@ -2483,7 +2491,9 @@ int sequencer_make_script(int keep_empty, FILE *out,\n>> \t\tstrbuf_reset(&buf);\n>> \t\tif (!keep_empty && is_original_commit_empty(commit))\n>> \t\t\tstrbuf_addf(&buf, \"%c \", comment_line_char);\n>> -\t\tstrbuf_addf(&buf, \"pick %s \", oid_to_hex(&commit->object.oid));\n>> +\t\tstrbuf_addf(&buf, \"%s %s \",\n>> +\t\t\t abbreviate_commands ? \"p\" : \"pick\",\n>> +\t\t\t oid_to_hex(&commit->object.oid));\n> \n> I guess the compiler will optimize this code so that the conditional is\n> evaluated only once. Not that this is performance critical ;-)\n\nIs your guess enough? :-) If not, how could I make sure this is optimized?\nShould I do that check before the while() loop?\n\n> \n>> \t\tpretty_print_commit(&pp, commit, &buf);\n>> \t\tstrbuf_addch(&buf, '\\n');\n>> \t\tfputs(buf.buf, out);\n>> @@ -2539,7 +2549,7 @@ int add_exec_commands(const char *command)\n>> \treturn 0;\n>> }\n>> \n>> -int transform_todo_ids(int shorten_ids)\n>> +int transform_todo_ids(int shorten_ids, int abbreviate_commands)\n>> {\n>> \tconst char *todo_file = rebase_path_todo();\n>> \tstruct todo_list todo_list = TODO_LIST_INIT;\n>> @@ -2575,19 +2585,33 @@ int transform_todo_ids(int shorten_ids)\n>> \t\t\ttodo_list.items[i + 1].offset_in_buf :\n>> \t\t\ttodo_list.buf.len;\n>> \n>> -\t\tif (item->command >= TODO_EXEC && item->command != TODO_DROP)\n>> -\t\t\tfwrite(p, eol - bol, 1, out);\n>> -\t\telse {\n>> +\t\tif (item->command >= TODO_EXEC && item->command != TODO_DROP) {\n>> +\t\t\tif (!abbreviate_commands || command_to_char(item->command) < 0) {\n>> +\t\t\t\tfwrite(p, eol - bol, 1, out);\n>> +\t\t\t} else {\n>> +\t\t\t\tconst char *end_of_line = strchrnul(p, '\\n');\n>> +\t\t\t\tp += strspn(p, \" \\t\"); /* skip whitespace */\n>> +\t\t\t\tp += strcspn(p, \" \\t\"); /* skip command */\n>> +\t\t\t\tfprintf(out, \"%c%.*s\\n\",\n>> +\t\t\t\t\tcommand_to_char(item->command),\n>> +\t\t\t\t\t(int)(end_of_line - p), p);\n>> +\t\t\t}\n>> +\t\t} else {\n>> \t\t\tconst char *id = shorten_ids ?\n>> \t\t\t\tshort_commit_name(item->commit) :\n>> \t\t\t\toid_to_hex(&item->commit->object.oid);\n>> -\t\t\tint len;\n>> \n>> -\t\t\tp += strspn(p, \" \\t\"); /* left-trim command */\n>> -\t\t\tlen = strcspn(p, \" \\t\"); /* length of command */\n>> -\n>> -\t\t\tfprintf(out, \"%.*s %s %.*s\\n\",\n>> -\t\t\t\tlen, p, id, item->arg_len, item->arg);\n>> +\t\t\tif (abbreviate_commands) {\n>> +\t\t\t\tfprintf(out, \"%c %s %.*s\\n\",\n>> +\t\t\t\t\tcommand_to_char(item->command),\n>> +\t\t\t\t\tid, item->arg_len, item->arg);\n>> +\t\t\t} else {\n>> +\t\t\t\tint len;\n>> +\t\t\t\tp += strspn(p, \" \\t\"); /* left-trim command */\n>> +\t\t\t\tlen = strcspn(p, \" \\t\"); /* length of command */\n>> +\t\t\t\tfprintf(out, \"%.*s %s %.*s\\n\",\n>> +\t\t\t\t\tlen, p, id, item->arg_len, item->arg);\n>> +\t\t\t}\n> \n> This hunk changes indentation quite a bit, therefore it is a bit harder to\n> read than necessary (and the resulting code, too, as it is more smooshed\n> against the 80-column boundary on the right).\n> \n> How about this instead:\n> \n> -\t\tif (item->command >= TODO_EXEC && item->command != TODO_DROP)\n> +\t\tif (abbreviate_commands && command_to_char(item->command)) {\n> +\t\t\tconst char *id = shorten_ids ?\n> +\t\t\t\tshort_commit_name(item->commit) :\n> +\t\t\t\toid_to_hex(&item->commit->object.oid);\n> +\t\t\tfprintf(out, \"%c %s %.*s\\n\",\n> +\t\t\t\tcommand_to_char(item->command),\n> +\t\t\t\tid, item->arg_len, item->arg);\n> +\t\t} else if (item->command >= TODO_EXEC &&\n> +\t\t\t item->command != TODO_DROP)\n> \n> i.e. test first for the short and sweet case that we want (and can)\n> abbreviate the command, otherwise keep the code as before?\n\nThat looks quite better! I'll update.\n\n> \n> Ciao,\n> Dscho\n> \n\nThanks,\nLiam\n"},{"id":"333763","messageId":"79720807-9e00-af34-b42a-03fd65c58a9e@gmail.com","threadId":"47327","inReplyTo":"20171127231131.GB29636@sigill.intra.peff.net","subject":"Re: [PATCH 4/5] rebase -i: learn to abbreviate command names","fromName":"liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-11-29T02:11:35Z","receivedAt":"2017-11-29T02:11:41Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Hi Peff,\n\nThanks for taking the time to test this, I'll squash that patch in v2.\n\nOn 27/11/17 06:11 PM, Jeff King wrote:\n> On Tue, Nov 28, 2017 at 12:04:45AM +0100, Johannes Schindelin wrote:\n> \n>>> +rebase.abbreviateCommands::\n>>> +\tIf set to true, `git rebase` will use abbreviated command names in the\n>>> +\ttodo list resulting in something like this:\n>>> +\n>>> +-------------------------------------------\n>>> +\tp deadbee The oneline of the commit\n>>> +\tp fa1afe1 The oneline of the next commit\n>>> +\t...\n>>> +-------------------------------------------\n>>\n>> I *think* that AsciiDoc will render this in a different way from what we\n>> want, but I am not an AsciiDoc expert. In my hands, I always had to add a\n>> single + in an otherwise empty line to start a new indented paragraph *and\n>> then continue with non-indented lines*.\n> \n> Good catch. Interestingly enough, my asciidoc seems to render this\n> as desired for the docbook/roff version, but has screwed-up indentation\n> for the HTML version.\n> \n> Fixing it as you suggest makes it look good in both (and I think you can\n> never go wrong with \"+\"-continuation, aside from making the source a bit\n> uglier).\n> \n> Squashable patch below for convenience, since I did try it.\n> \n> -Peff\n> \n> diff --git a/Documentation/rebase-config.txt b/Documentation/rebase-config.txt\n> index 0820b60f6e..42e1ba7575 100644\n> --- a/Documentation/rebase-config.txt\n> +++ b/Documentation/rebase-config.txt\n> @@ -34,18 +34,19 @@ rebase.instructionFormat::\n> rebase.abbreviateCommands::\n> \tIf set to true, `git rebase` will use abbreviated command names in the\n> \ttodo list resulting in something like this:\n> -\n> ++\n> -------------------------------------------\n> \tp deadbee The oneline of the commit\n> \tp fa1afe1 The oneline of the next commit\n> \t...\n> -------------------------------------------\n> -\n> -\tinstead of:\n> -\n> ++\n> +instead of:\n> ++\n> -------------------------------------------\n> \tpick deadbee The oneline of the commit\n> \tpick fa1afe1 The oneline of the next commit\n> \t...\n> -------------------------------------------\n> -\tDefaults to false.\n> ++\n> +Defaults to false.\n> \n\nLiam\n"},{"id":"333814","messageId":"alpine.DEB.2.21.1.1711292234140.6482@virtualbox","threadId":"47327","inReplyTo":"6b4e8352-0583-11c2-43ac-ec4ab33cc554@gmail.com","subject":"Re: [PATCH 3/5] rebase -i: add exec commands via the rebase--helper","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-29T21:35:44Z","receivedAt":"2017-11-29T21:35:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Liam,\n\nOn Tue, 28 Nov 2017, liam Beguin wrote:\n\n> I just realized we could maybe add exec instructions only after pick\n> commands if we do add-exec-commands before rearrange-squash.\n\nThat won't work, because the squash/fixup commands are pick commands\nbefore rearrange-squash. So you'd add one unwanted exec per\nsquash/fixup...\n\nCiao,\nDscho\n"},{"id":"333815","messageId":"alpine.DEB.2.21.1.1711292236010.6482@virtualbox","threadId":"47327","inReplyTo":"b4331bb3-db5d-e4f5-54db-f04d77385ae7@gmail.com","subject":"Re: [PATCH 4/5] rebase -i: learn to abbreviate command names","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-29T21:40:52Z","receivedAt":"2017-11-29T21:41:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Liam,\n\nOn Tue, 28 Nov 2017, liam Beguin wrote:\n\n> On 27/11/17 06:04 PM, Johannes Schindelin wrote:\n> > \n> > On Sun, 26 Nov 2017, Liam Beguin wrote:\n> > \n> >> @@ -2483,7 +2491,9 @@ int sequencer_make_script(int keep_empty, FILE *out,\n> >> \t\tstrbuf_reset(&buf);\n> >> \t\tif (!keep_empty && is_original_commit_empty(commit))\n> >> \t\t\tstrbuf_addf(&buf, \"%c \", comment_line_char);\n> >> -\t\tstrbuf_addf(&buf, \"pick %s \", oid_to_hex(&commit->object.oid));\n> >> +\t\tstrbuf_addf(&buf, \"%s %s \",\n> >> +\t\t\t abbreviate_commands ? \"p\" : \"pick\",\n> >> +\t\t\t oid_to_hex(&commit->object.oid));\n> > \n> > I guess the compiler will optimize this code so that the conditional\n> > is evaluated only once. Not that this is performance critical ;-)\n> \n> Is your guess enough? :-) If not, how could I make sure this is\n> optimized? Should I do that check before the while() loop?\n\nI am a fan of not relying too heavily on compiler optimization and e.g.\nextract code from loops when it does not need to be evaluated every single\niteration. In this case:\n\n\tconst char *pick = abbreviate_commands ? \"p\" : \"pick\";\n\t...\n\t\tstrbuf_addf(&buf, \"%s %s \", pick,\n\t\t\t oid_to_hex(&commit->object.oid));\n\nBut given Junio's comment that the assignment of `first` was too far away\nfrom the line where it is used for his taste, I guess he will argue (once\nagain) the exact opposite of me.\n\nCiao,\nDscho\n"},{"id":"333979","messageId":"xmqqk1y4libp.fsf@gitster.mtv.corp.google.com","threadId":"47327","inReplyTo":"alpine.DEB.2.21.1.1711292236010.6482@virtualbox","subject":"Re: [PATCH 4/5] rebase -i: learn to abbreviate command names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-03T01:18:50Z","receivedAt":"2017-12-03T01:18:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> I am a fan of not relying too heavily on compiler optimization and e.g.\n> extract code from loops when it does not need to be evaluated every single\n> iteration. In this case:\n>\n> \tconst char *pick = abbreviate_commands ? \"p\" : \"pick\";\n> \t...\n> \t\tstrbuf_addf(&buf, \"%s %s \", pick,\n> \t\t\t oid_to_hex(&commit->object.oid));\n\nI would have called that variable \"pick_cmd\", not just \"pick\"; this\npreference is minor enough that I would probably reject a patch to\nrename from one to the other if the above were already part of the\nexisting codebase.\n\nI find that the code suggested above easier to follow, simply\nbecause it expresses clearly the flow of thought and that flow of\nthought matches how I personally think: we decide how this command\nis spelled in the output upfront, and then use that same spelling\nconsistently throughout the loop.\n\nI do not think it matters performance-wise either way, but I value\nhow easy it is to follow the code for humans, and it matters much\nmore in the longer run. If a compiler does a poor job, we can\neventually notice and help it to produce better code that still does\nwhat we wanted it to do (or it may not be performance critical and\nwe may not even notice). If a code is hard to follow, on the other\nhand, what we wanted it to do in the first place becomes harder to\nfigure out.\n"},{"id":"334014","messageId":"20171203221721.16462-1-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171127045514.25647-1-liambeguin@gmail.com","subject":"[PATCH v2 0/9] rebase -i: add config to abbreviate command names","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-03T22:17:12Z","receivedAt":"2017-12-03T22:18:04Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Hi everyone,\n\nThis series will add the 'rebase.abbreviateCommands' configuration\noption to allow `git rebase -i` to default to the single-letter command\nnames when generating the todo list.\n\nUsing single-letter command names can present two benefits. First, it\nmakes it easier to change the action since you only need to replace a\nsingle character (i.e.: in vim \"r\" instead of\n\"ciw\"). Second, using this with a large enough value of\n'core.abbrev' enables the lines of the todo list to remain aligned\nmaking the files easier to read.\n\nChanges in V2:\n- Refactor and rename 'transform_todo_ids'\n- Replace SHA-1 by OID in rebase--helper.c\n- Update todo list related functions to take a generic 'flags' parameter\n- Rename 'add_exec_commands' function to 'sequencer_add_exec_commands'\n- Rename 'add-exec' option to 'add-exec-commands'\n- Use 'strbur_read_file' instead of rewriting it\n- Make 'command_to_char' return 'comment_char_line' if no single-letter\n command name is defined\n- Combine both tests into a single test case\n- Update commit messages\n\nLiam Beguin (9):\n Documentation: move rebase.* configs to new file\n Documentation: use preferred name for the 'todo list' script\n rebase -i: set commit to null in exec commands\n rebase -i: refactor transform_todo_ids\n rebase -i: replace reference to sha1 with oid\n rebase -i: update functions to use a flags parameter\n rebase -i -x: add exec commands via the rebase--helper\n rebase -i: learn to abbreviate command names\n t3404: add test case for abbreviated commands\n\n Documentation/config.txt | 31 +-------\n Documentation/git-rebase.txt | 19 +----\n Documentation/rebase-config.txt | 52 +++++++++++++\n builtin/rebase--helper.c | 29 +++++---\n git-rebase--interactive.sh | 23 +-----\n sequencer.c | 126 +++++++++++++++++++++-----------\n sequencer.h | 10 ++-\n t/t3404-rebase-interactive.sh | 22 ++++++\n 8 files changed, 186 insertions(+), 126 deletions(-)\n create mode 100644 Documentation/rebase-config.txt\n\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334015","messageId":"20171203221721.16462-2-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171203221721.16462-1-liambeguin@gmail.com","subject":"[PATCH v2 1/9] Documentation: move rebase.* configs to new file","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-03T22:17:13Z","receivedAt":"2017-12-03T22:18:06Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Move all rebase.* configuration variables to a separate file in order to\nremove duplicates, and include it in config.txt and git-rebase.txt. The\nnew descriptions are mostly taken from config.txt as they are more\nverbose.\n\nSigned-off-by: Liam Beguin \n---\n Documentation/config.txt | 31 +------------------------------\n Documentation/git-rebase.txt | 19 +------------------\n Documentation/rebase-config.txt | 32 ++++++++++++++++++++++++++++++++\n 3 files changed, 34 insertions(+), 48 deletions(-)\n create mode 100644 Documentation/rebase-config.txt\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 531649cb40ea..e424b7de90b5 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2691,36 +2691,7 @@ push.recurseSubmodules::\n \tis retained. You may override this configuration at time of push by\n \tspecifying '--recurse-submodules=check|on-demand|no'.\n \n-rebase.stat::\n-\tWhether to show a diffstat of what changed upstream since the last\n-\trebase. False by default.\n-\n-rebase.autoSquash::\n-\tIf set to true enable `--autosquash` option by default.\n-\n-rebase.autoStash::\n-\tWhen set to true, automatically create a temporary stash entry\n-\tbefore the operation begins, and apply it after the operation\n-\tends. This means that you can run rebase on a dirty worktree.\n-\tHowever, use with care: the final stash application after a\n-\tsuccessful rebase might result in non-trivial conflicts.\n-\tDefaults to false.\n-\n-rebase.missingCommitsCheck::\n-\tIf set to \"warn\", git rebase -i will print a warning if some\n-\tcommits are removed (e.g. a line was deleted), however the\n-\trebase will still proceed. If set to \"error\", it will print\n-\tthe previous warning and stop the rebase, 'git rebase\n-\t--edit-todo' can then be used to correct the error. If set to\n-\t\"ignore\", no checking is done.\n-\tTo drop a commit without warning or error, use the `drop`\n-\tcommand in the todo-list.\n-\tDefaults to \"ignore\".\n-\n-rebase.instructionFormat::\n-\tA format string, as specified in linkgit:git-log[1], to be used for\n-\tthe instruction list during an interactive rebase. The format will automatically\n-\thave the long commit hash prepended to the format.\n+include::rebase-config.txt[]\n \n receive.advertiseAtomic::\n \tBy default, git-receive-pack will advertise the atomic push\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 3cedfb0fd22b..8a861c1e0d69 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -203,24 +203,7 @@ Alternatively, you can undo the 'git rebase' with\n CONFIGURATION\n -------------\n \n-rebase.stat::\n-\tWhether to show a diffstat of what changed upstream since the last\n-\trebase. False by default.\n-\n-rebase.autoSquash::\n-\tIf set to true enable `--autosquash` option by default.\n-\n-rebase.autoStash::\n-\tIf set to true enable `--autostash` option by default.\n-\n-rebase.missingCommitsCheck::\n-\tIf set to \"warn\", print warnings about removed commits in\n-\tinteractive mode. If set to \"error\", print the warnings and\n-\tstop the rebase. If set to \"ignore\", no checking is\n-\tdone. \"ignore\" by default.\n-\n-rebase.instructionFormat::\n-\tCustom commit list format to use during an `--interactive` rebase.\n+include::rebase-config.txt[]\n \n OPTIONS\n -------\ndiff --git a/Documentation/rebase-config.txt b/Documentation/rebase-config.txt\nnew file mode 100644\nindex 000000000000..dba088d7c68f\n--- /dev/null\n+++ b/Documentation/rebase-config.txt\n@@ -0,0 +1,32 @@\n+rebase.stat::\n+\tWhether to show a diffstat of what changed upstream since the last\n+\trebase. False by default.\n+\n+rebase.autoSquash::\n+\tIf set to true enable `--autosquash` option by default.\n+\n+rebase.autoStash::\n+\tWhen set to true, automatically create a temporary stash entry\n+\tbefore the operation begins, and apply it after the operation\n+\tends. This means that you can run rebase on a dirty worktree.\n+\tHowever, use with care: the final stash application after a\n+\tsuccessful rebase might result in non-trivial conflicts.\n+\tThis option can be overridden by the `--no-autostash` and\n+\t`--autostash` options of linkgit:git-rebase[1].\n+\tDefaults to false.\n+\n+rebase.missingCommitsCheck::\n+\tIf set to \"warn\", git rebase -i will print a warning if some\n+\tcommits are removed (e.g. a line was deleted), however the\n+\trebase will still proceed. If set to \"error\", it will print\n+\tthe previous warning and stop the rebase, 'git rebase\n+\t--edit-todo' can then be used to correct the error. If set to\n+\t\"ignore\", no checking is done.\n+\tTo drop a commit without warning or error, use the `drop`\n+\tcommand in the todo-list.\n+\tDefaults to \"ignore\".\n+\n+rebase.instructionFormat::\n+\tA format string, as specified in linkgit:git-log[1], to be used for the\n+\tinstruction list during an interactive rebase. The format will\n+\tautomatically have the long commit hash prepended to the format.\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334016","messageId":"20171203221721.16462-3-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171203221721.16462-1-liambeguin@gmail.com","subject":"[PATCH v2 2/9] Documentation: use preferred name for the 'todo list' script","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-03T22:17:14Z","receivedAt":"2017-12-03T22:18:09Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Use \"todo list\" instead of \"instruction list\" or \"todo-list\" to\nreduce further confusion regarding the name of this script.\n\nSigned-off-by: Liam Beguin \n---\n Documentation/rebase-config.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/rebase-config.txt b/Documentation/rebase-config.txt\nindex dba088d7c68f..30ae08cb5a4b 100644\n--- a/Documentation/rebase-config.txt\n+++ b/Documentation/rebase-config.txt\n@@ -23,10 +23,10 @@ rebase.missingCommitsCheck::\n \t--edit-todo' can then be used to correct the error. If set to\n \t\"ignore\", no checking is done.\n \tTo drop a commit without warning or error, use the `drop`\n-\tcommand in the todo-list.\n+\tcommand in the todo list.\n \tDefaults to \"ignore\".\n \n rebase.instructionFormat::\n \tA format string, as specified in linkgit:git-log[1], to be used for the\n-\tinstruction list during an interactive rebase. The format will\n+\ttodo list during an interactive rebase. The format will\n \tautomatically have the long commit hash prepended to the format.\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334017","messageId":"20171203221721.16462-6-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171203221721.16462-1-liambeguin@gmail.com","subject":"[PATCH v2 5/9] rebase -i: replace reference to sha1 with oid","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-03T22:17:17Z","receivedAt":"2017-12-03T22:18:13Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Since we are trying to abstract the hash function name elsewhere in the\ncode base, lets use OID instead of SHA-1 in the rebase--helper too.\n\nSigned-off-by: Liam Beguin \n---\n builtin/rebase--helper.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex 7c06a27de821..af0f91164fd0 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -14,7 +14,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \tstruct replay_opts opts = REPLAY_OPTS_INIT;\n \tint keep_empty = 0;\n \tenum {\n-\t\tCONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S,\n+\t\tCONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,\n \t\tCHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH\n \t} command = 0;\n \tstruct option options[] = {\n@@ -27,9 +27,9 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\tOPT_CMDMODE(0, \"make-script\", &command,\n \t\t\tN_(\"make rebase script\"), MAKE_SCRIPT),\n \t\tOPT_CMDMODE(0, \"shorten-ids\", &command,\n-\t\t\tN_(\"shorten SHA-1s in the todo list\"), SHORTEN_SHA1S),\n+\t\t\tN_(\"shorten commit ids in the todo list\"), SHORTEN_OIDS),\n \t\tOPT_CMDMODE(0, \"expand-ids\", &command,\n-\t\t\tN_(\"expand SHA-1s in the todo list\"), EXPAND_SHA1S),\n+\t\t\tN_(\"expand commit ids in the todo list\"), EXPAND_OIDS),\n \t\tOPT_CMDMODE(0, \"check-todo-list\", &command,\n \t\t\tN_(\"check the todo list\"), CHECK_TODO_LIST),\n \t\tOPT_CMDMODE(0, \"skip-unnecessary-picks\", &command,\n@@ -54,9 +54,9 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\treturn !!sequencer_remove_state(&opts);\n \tif (command == MAKE_SCRIPT && argc > 1)\n \t\treturn !!sequencer_make_script(keep_empty, stdout, argc, argv);\n-\tif (command == SHORTEN_SHA1S && argc == 1)\n+\tif (command == SHORTEN_OIDS && argc == 1)\n \t\treturn !!transform_todo_insn(1);\n-\tif (command == EXPAND_SHA1S && argc == 1)\n+\tif (command == EXPAND_OIDS && argc == 1)\n \t\treturn !!transform_todo_insn(0);\n \tif (command == CHECK_TODO_LIST && argc == 1)\n \t\treturn !!check_todo_list();\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334018","messageId":"20171203221721.16462-5-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171203221721.16462-1-liambeguin@gmail.com","subject":"[PATCH v2 4/9] rebase -i: refactor transform_todo_ids","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-03T22:17:16Z","receivedAt":"2017-12-03T22:18:16Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"The transform_todo_ids function is a little hard to read. Lets try\nto make it easier by using more of the strbuf API. Also, since we'll\nsoon be adding command abbreviations, let's rename the function so\nit's name reflects that change.\n\nSigned-off-by: Liam Beguin \n---\n builtin/rebase--helper.c | 4 +--\n sequencer.c | 69 ++++++++++++++++++++----------------------------\n sequencer.h | 2 +-\n 3 files changed, 31 insertions(+), 44 deletions(-)\n\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex f8519363a393..7c06a27de821 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -55,9 +55,9 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \tif (command == MAKE_SCRIPT && argc > 1)\n \t\treturn !!sequencer_make_script(keep_empty, stdout, argc, argv);\n \tif (command == SHORTEN_SHA1S && argc == 1)\n-\t\treturn !!transform_todo_ids(1);\n+\t\treturn !!transform_todo_insn(1);\n \tif (command == EXPAND_SHA1S && argc == 1)\n-\t\treturn !!transform_todo_ids(0);\n+\t\treturn !!transform_todo_insn(0);\n \tif (command == CHECK_TODO_LIST && argc == 1)\n \t\treturn !!check_todo_list();\n \tif (command == SKIP_UNNECESSARY_PICKS && argc == 1)\ndiff --git a/sequencer.c b/sequencer.c\nindex 5033b049d995..0ff3c90e44bf 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2494,60 +2494,47 @@ int sequencer_make_script(int keep_empty, FILE *out,\n }\n \n \n-int transform_todo_ids(int shorten_ids)\n+int transform_todo_insn(int shorten_ids)\n {\n \tconst char *todo_file = rebase_path_todo();\n \tstruct todo_list todo_list = TODO_LIST_INIT;\n-\tint fd, res, i;\n-\tFILE *out;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct todo_item *item;\n+\tint i;\n \n-\tstrbuf_reset(&todo_list.buf);\n-\tfd = open(todo_file, O_RDONLY);\n-\tif (fd < 0)\n-\t\treturn error_errno(_(\"could not open '%s'\"), todo_file);\n-\tif (strbuf_read(&todo_list.buf, fd, 0) < 0) {\n-\t\tclose(fd);\n+\tif (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)\n \t\treturn error(_(\"could not read '%s'.\"), todo_file);\n-\t}\n-\tclose(fd);\n \n-\tres = parse_insn_buffer(todo_list.buf.buf, &todo_list);\n-\tif (res) {\n+\tif (parse_insn_buffer(todo_list.buf.buf, &todo_list)) {\n \t\ttodo_list_release(&todo_list);\n \t\treturn error(_(\"unusable todo list: '%s'\"), todo_file);\n \t}\n \n-\tout = fopen(todo_file, \"w\");\n-\tif (!out) {\n-\t\ttodo_list_release(&todo_list);\n-\t\treturn error(_(\"unable to open '%s' for writing\"), todo_file);\n-\t}\n-\tfor (i = 0; i < todo_list.nr; i++) {\n-\t\tstruct todo_item *item = todo_list.items + i;\n-\t\tint bol = item->offset_in_buf;\n-\t\tconst char *p = todo_list.buf.buf + bol;\n-\t\tint eol = i + 1 < todo_list.nr ?\n-\t\t\ttodo_list.items[i + 1].offset_in_buf :\n-\t\t\ttodo_list.buf.len;\n-\n-\t\tif (item->command >= TODO_EXEC && item->command != TODO_DROP)\n-\t\t\tfwrite(p, eol - bol, 1, out);\n-\t\telse {\n-\t\t\tconst char *id = shorten_ids ?\n-\t\t\t\tshort_commit_name(item->commit) :\n-\t\t\t\toid_to_hex(&item->commit->object.oid);\n-\t\t\tint len;\n-\n-\t\t\tp += strspn(p, \" \\t\"); /* left-trim command */\n-\t\t\tlen = strcspn(p, \" \\t\"); /* length of command */\n-\n-\t\t\tfprintf(out, \"%.*s %s %.*s\\n\",\n-\t\t\t\tlen, p, id, item->arg_len, item->arg);\n+\tfor (item = todo_list.items, i = 0; i < todo_list.nr; i++, item++) {\n+\t\t/* if the item is not a command write it and continue */\n+\t\tif (item->command >= TODO_COMMENT) {\n+\t\t\tstrbuf_addf(&buf, \"%.*s\\n\", item->arg_len, item->arg);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/* add command to the buffer */\n+\t\tstrbuf_addstr(&buf, command_to_string(item->command));\n+\n+\t\t/* add commit id */\n+\t\tif (item->commit) {\n+\t\t\tconst char *oid = shorten_ids ?\n+\t\t\t\t\t short_commit_name(item->commit) :\n+\t\t\t\t\t oid_to_hex(&item->commit->object.oid);\n+\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}\n-\tfclose(out);\n+\n+\ti = write_message(buf.buf, buf.len, todo_file, 0);\n \ttodo_list_release(&todo_list);\n-\treturn 0;\n+\treturn i;\n }\n \n enum check_level {\ndiff --git a/sequencer.h b/sequencer.h\nindex 6f3d3df82c0a..4e444e3bf1c4 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -48,7 +48,7 @@ int sequencer_remove_state(struct replay_opts *opts);\n int sequencer_make_script(int keep_empty, FILE *out,\n \t\tint argc, const char **argv);\n \n-int transform_todo_ids(int shorten_ids);\n+int transform_todo_insn(int shorten_ids);\n int check_todo_list(void);\n int skip_unnecessary_picks(void);\n int rearrange_squash(void);\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334019","messageId":"20171203221721.16462-8-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171203221721.16462-1-liambeguin@gmail.com","subject":"[PATCH v2 7/9] rebase -i -x: add exec commands via the rebase--helper","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-03T22:17:19Z","receivedAt":"2017-12-03T22:18:18Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Recent work on `git-rebase--interactive` aims to convert shell code to\nC. Even if this is most likely not a big performance enhancement, let's\nconvert it too since a coming change to abbreviate command names\nrequires it to be updated.\n\nSigned-off-by: Liam Beguin \n---\n builtin/rebase--helper.c | 7 ++++++-\n git-rebase--interactive.sh | 23 +----------------------\n sequencer.c | 39 +++++++++++++++++++++++++++++++++++++++\n sequencer.h | 1 +\n 4 files changed, 47 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex fe814bf7229e..03337e1484a2 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -15,7 +15,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \tunsigned flags = 0, keep_empty = 0;\n \tenum {\n \t\tCONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,\n-\t\tCHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH\n+\t\tCHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,\n+\t\tADD_EXEC\n \t} command = 0;\n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"ff\", &opts.allow_ff, N_(\"allow fast-forward\")),\n@@ -36,6 +37,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\t\tN_(\"skip unnecessary picks\"), SKIP_UNNECESSARY_PICKS),\n \t\tOPT_CMDMODE(0, \"rearrange-squash\", &command,\n \t\t\tN_(\"rearrange fixup/squash lines\"), REARRANGE_SQUASH),\n+\t\tOPT_CMDMODE(0, \"add-exec-commands\", &command,\n+\t\t\tN_(\"insert exec commands in todo list\"), ADD_EXEC),\n \t\tOPT_END()\n \t};\n \n@@ -65,5 +68,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\treturn !!skip_unnecessary_picks();\n \tif (command == REARRANGE_SQUASH && argc == 1)\n \t\treturn !!rearrange_squash();\n+\tif (command == ADD_EXEC && argc == 2)\n+\t\treturn !!sequencer_add_exec_commands(argv[1]);\n \tusage_with_options(builtin_rebase_helper_usage, options);\n }\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 437815669f00..e3f5a0abf3c7 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -722,27 +722,6 @@ collapse_todo_ids() {\n \tgit rebase--helper --shorten-ids\n }\n \n-# Add commands after a pick or after a squash/fixup series\n-# in the todo list.\n-add_exec_commands () {\n-\t{\n-\t\tfirst=t\n-\t\twhile read -r insn rest\n-\t\tdo\n-\t\t\tcase $insn in\n-\t\t\tpick)\n-\t\t\t\ttest -n \"$first\" ||\n-\t\t\t\tprintf \"%s\" \"$cmd\"\n-\t\t\t\t;;\n-\t\t\tesac\n-\t\t\tprintf \"%s %s\\n\" \"$insn\" \"$rest\"\n-\t\t\tfirst=\n-\t\tdone\n-\t\tprintf \"%s\" \"$cmd\"\n-\t} <\"$1\" >\"$1.new\" &&\n-\tmv \"$1.new\" \"$1\"\n-}\n-\n # Switch to the branch in $into and notify it in the reflog\n checkout_onto () {\n \tGIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $onto_name\"\n@@ -982,7 +961,7 @@ fi\n \n test -s \"$todo\" || echo noop >> \"$todo\"\n test -z \"$autosquash\" || git rebase--helper --rearrange-squash || exit\n-test -n \"$cmd\" && add_exec_commands \"$todo\"\n+test -n \"$cmd\" && git rebase--helper --add-exec-commands \"$cmd\"\n \n todocount=$(git stripspace --strip-comments <\"$todo\" | wc -l)\n todocount=${todocount##* }\ndiff --git a/sequencer.c b/sequencer.c\nindex 7d712811e9d1..bd047737082d 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2494,6 +2494,45 @@ int sequencer_make_script(FILE *out, int argc, const char **argv,\n \treturn 0;\n }\n \n+/*\n+ * Add commands after pick and (series of) squash/fixup commands\n+ * in the todo list.\n+ */\n+int sequencer_add_exec_commands(const char *commands)\n+{\n+\tconst char *todo_file = rebase_path_todo();\n+\tstruct todo_list todo_list = TODO_LIST_INIT;\n+\tstruct todo_item *item;\n+\tstruct strbuf *buf = &todo_list.buf;\n+\tsize_t offset = 0, commands_len = strlen(commands);\n+\tint i, first;\n+\n+\tif (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)\n+\t\treturn error(_(\"could not read '%s'.\"), todo_file);\n+\n+\tif (parse_insn_buffer(todo_list.buf.buf, &todo_list)) {\n+\t\ttodo_list_release(&todo_list);\n+\t\treturn error(_(\"unusable todo list: '%s'\"), todo_file);\n+\t}\n+\n+\tfirst = 1;\n+\t/* insert before every pick except the first one */\n+\tfor (item = todo_list.items, i = 0; i < todo_list.nr; i++, item++) {\n+\t\tif (item->command == TODO_PICK && !first) {\n+\t\t\tstrbuf_insert(buf, item->offset_in_buf + offset,\n+\t\t\t\t commands, commands_len);\n+\t\t\toffset += commands_len;\n+\t\t}\n+\t\tfirst = 0;\n+\t}\n+\n+\t/* append final */\n+\tstrbuf_add(buf, commands, commands_len);\n+\n+\ti = write_message(buf->buf, buf->len, todo_file, 0);\n+\ttodo_list_release(&todo_list);\n+\treturn i;\n+}\n \n int transform_todo_insn(unsigned flags)\n {\ndiff --git a/sequencer.h b/sequencer.h\nindex 3bb6b0658192..e4a9d2419883 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -50,6 +50,7 @@ int sequencer_remove_state(struct replay_opts *opts);\n int sequencer_make_script(FILE *out, int argc, const char **argv,\n \t\t\t unsigned flags);\n \n+int sequencer_add_exec_commands(const char *command);\n int transform_todo_insn(unsigned flags);\n int check_todo_list(void);\n int skip_unnecessary_picks(void);\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334020","messageId":"20171203221721.16462-7-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171203221721.16462-1-liambeguin@gmail.com","subject":"[PATCH v2 6/9] rebase -i: update functions to use a flags parameter","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-03T22:17:18Z","receivedAt":"2017-12-03T22:18:22Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Update functions used in the rebase--helper so that they take a generic\n'flags' parameter instead of a growing list of options.\n\nSigned-off-by: Liam Beguin \n---\n builtin/rebase--helper.c | 13 +++++++------\n sequencer.c | 9 +++++----\n sequencer.h | 8 +++++---\n 3 files changed, 17 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex af0f91164fd0..fe814bf7229e 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -12,7 +12,7 @@ static const char * const builtin_rebase_helper_usage[] = {\n int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n {\n \tstruct replay_opts opts = REPLAY_OPTS_INIT;\n-\tint keep_empty = 0;\n+\tunsigned flags = 0, keep_empty = 0;\n \tenum {\n \t\tCONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,\n \t\tCHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH\n@@ -48,16 +48,17 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, NULL, options,\n \t\t\tbuiltin_rebase_helper_usage, PARSE_OPT_KEEP_ARGV0);\n \n+\tflags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0;\n+\tflags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTED_IDS : 0;\n+\n \tif (command == CONTINUE && argc == 1)\n \t\treturn !!sequencer_continue(&opts);\n \tif (command == ABORT && argc == 1)\n \t\treturn !!sequencer_remove_state(&opts);\n \tif (command == MAKE_SCRIPT && argc > 1)\n-\t\treturn !!sequencer_make_script(keep_empty, stdout, argc, argv);\n-\tif (command == SHORTEN_OIDS && argc == 1)\n-\t\treturn !!transform_todo_insn(1);\n-\tif (command == EXPAND_OIDS && argc == 1)\n-\t\treturn !!transform_todo_insn(0);\n+\t\treturn !!sequencer_make_script(stdout, argc, argv, flags);\n+\tif ((command == SHORTEN_OIDS || command == EXPAND_OIDS) && argc == 1)\n+\t\treturn !!transform_todo_insn(flags);\n \tif (command == CHECK_TODO_LIST && argc == 1)\n \t\treturn !!check_todo_list();\n \tif (command == SKIP_UNNECESSARY_PICKS && argc == 1)\ndiff --git a/sequencer.c b/sequencer.c\nindex 0ff3c90e44bf..7d712811e9d1 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2444,14 +2444,15 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)\n \tstrbuf_release(&sob);\n }\n \n-int sequencer_make_script(int keep_empty, FILE *out,\n-\t\tint argc, const char **argv)\n+int sequencer_make_script(FILE *out, int argc, const char **argv,\n+\t\t\t unsigned flags)\n {\n \tchar *format = NULL;\n \tstruct pretty_print_context pp = {0};\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct rev_info revs;\n \tstruct commit *commit;\n+\tint keep_empty = flags & TODO_LIST_KEEP_EMPTY;\n \n \tinit_revisions(&revs, NULL);\n \trevs.verbose_header = 1;\n@@ -2494,7 +2495,7 @@ int sequencer_make_script(int keep_empty, FILE *out,\n }\n \n \n-int transform_todo_insn(int shorten_ids)\n+int transform_todo_insn(unsigned flags)\n {\n \tconst char *todo_file = rebase_path_todo();\n \tstruct todo_list todo_list = TODO_LIST_INIT;\n@@ -2522,7 +2523,7 @@ int transform_todo_insn(int shorten_ids)\n \n \t\t/* add commit id */\n \t\tif (item->commit) {\n-\t\t\tconst char *oid = shorten_ids ?\n+\t\t\tconst char *oid = flags & TODO_LIST_SHORTED_IDS ?\n \t\t\t\t\t short_commit_name(item->commit) :\n \t\t\t\t\t oid_to_hex(&item->commit->object.oid);\n \ndiff --git a/sequencer.h b/sequencer.h\nindex 4e444e3bf1c4..3bb6b0658192 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -45,10 +45,12 @@ int sequencer_continue(struct replay_opts *opts);\n int sequencer_rollback(struct replay_opts *opts);\n int sequencer_remove_state(struct replay_opts *opts);\n \n-int sequencer_make_script(int keep_empty, FILE *out,\n-\t\tint argc, const char **argv);\n+#define TODO_LIST_KEEP_EMPTY (1U << 0)\n+#define TODO_LIST_SHORTED_IDS (1U << 1)\n+int sequencer_make_script(FILE *out, int argc, const char **argv,\n+\t\t\t unsigned flags);\n \n-int transform_todo_insn(int shorten_ids);\n+int transform_todo_insn(unsigned flags);\n int check_todo_list(void);\n int skip_unnecessary_picks(void);\n int rearrange_squash(void);\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334021","messageId":"20171203221721.16462-4-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171203221721.16462-1-liambeguin@gmail.com","subject":"[PATCH v2 3/9] rebase -i: set commit to null in exec commands","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-03T22:17:15Z","receivedAt":"2017-12-03T22:18:24Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Make sure commit is set to NULL when parsing exec instructions\nfrom the todo list. If not, we may try to access an uninitialized\naddress later while updating the todo list.\n\nSigned-off-by: Liam Beguin \n---\n sequencer.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex fa94ed652d2c..5033b049d995 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1268,6 +1268,7 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)\n \tbol += padding;\n \n \tif (item->command == TODO_EXEC) {\n+\t\titem->commit = NULL;\n \t\titem->arg = bol;\n \t\titem->arg_len = (int)(eol - bol);\n \t\treturn 0;\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334022","messageId":"20171203221721.16462-10-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171203221721.16462-1-liambeguin@gmail.com","subject":"[PATCH v2 9/9] t3404: add test case for abbreviated commands","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-03T22:17:21Z","receivedAt":"2017-12-03T22:18:28Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Make sure the todo list ends up using single-letter command\nabbreviations when the rebase.abbreviateCommands is enabled.\nThis configuration option should not change anything else.\n\nSigned-off-by: Liam Beguin \n---\n t/t3404-rebase-interactive.sh | 22 ++++++++++++++++++++++\n 1 file changed, 22 insertions(+)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 6a82d1ed876d..481a3500900d 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1260,6 +1260,28 @@ test_expect_success 'rebase -i respects rebase.missingCommitsCheck = error' '\n \ttest B = $(git cat-file commit HEAD^ | sed -ne \\$p)\n '\n \n+test_expect_success 'respects rebase.abbreviateCommands with fixup, squash and exec' '\n+\trebase_setup_and_clean abbrevcmd &&\n+\ttest_commit \"first\" file1.txt \"first line\" first &&\n+\ttest_commit \"second\" file1.txt \"another line\" second &&\n+\ttest_commit \"fixup! first\" file2.txt \"first line again\" first_fixup &&\n+\ttest_commit \"squash! second\" file1.txt \"another line here\" second_squash &&\n+\tcat >expected <<-EOF &&\n+\tp $(git rev-list --abbrev-commit -1 first) first\n+\tf $(git rev-list --abbrev-commit -1 first_fixup) fixup! first\n+\tx git show HEAD\n+\tp $(git rev-list --abbrev-commit -1 second) second\n+\ts $(git rev-list --abbrev-commit -1 second_squash) squash! second\n+\tx git show HEAD\n+\tEOF\n+\tgit checkout abbrevcmd &&\n+\tset_cat_todo_editor &&\n+\ttest_config rebase.abbreviateCommands true &&\n+\ttest_must_fail git rebase -i --exec \"git show HEAD\" \\\n+\t\t--autosquash master >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'static check of bad command' '\n \trebase_setup_and_clean bad-cmd &&\n \tset_fake_editor &&\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334023","messageId":"20171203221721.16462-9-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171203221721.16462-1-liambeguin@gmail.com","subject":"[PATCH v2 8/9] rebase -i: learn to abbreviate command names","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-03T22:17:20Z","receivedAt":"2017-12-03T22:18:30Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"`git rebase -i` already know how to interpret single-letter command\nnames. Teach it to generate the todo list with these same abbreviated\nnames.\n\nBased-on-patch-by: Johannes Schindelin \nSigned-off-by: Liam Beguin \n---\n Documentation/rebase-config.txt | 20 ++++++++++++++++++++\n builtin/rebase--helper.c | 3 +++\n sequencer.c | 16 ++++++++++++++--\n sequencer.h | 1 +\n 4 files changed, 38 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/rebase-config.txt b/Documentation/rebase-config.txt\nindex 30ae08cb5a4b..42e1ba757564 100644\n--- a/Documentation/rebase-config.txt\n+++ b/Documentation/rebase-config.txt\n@@ -30,3 +30,23 @@ rebase.instructionFormat::\n \tA format string, as specified in linkgit:git-log[1], to be used for the\n \ttodo list during an interactive rebase. The format will\n \tautomatically have the long commit hash prepended to the format.\n+\n+rebase.abbreviateCommands::\n+\tIf set to true, `git rebase` will use abbreviated command names in the\n+\ttodo list resulting in something like this:\n++\n+-------------------------------------------\n+\tp deadbee The oneline of the commit\n+\tp fa1afe1 The oneline of the next commit\n+\t...\n+-------------------------------------------\n++\n+instead of:\n++\n+-------------------------------------------\n+\tpick deadbee The oneline of the commit\n+\tpick fa1afe1 The oneline of the next commit\n+\t...\n+-------------------------------------------\n++\n+Defaults to false.\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex 03337e1484a2..2c51ddcfd3dd 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -13,6 +13,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n {\n \tstruct replay_opts opts = REPLAY_OPTS_INIT;\n \tunsigned flags = 0, keep_empty = 0;\n+\tint abbreviate_commands = 0;\n \tenum {\n \t\tCONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,\n \t\tCHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,\n@@ -43,6 +44,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t};\n \n \tgit_config(git_default_config, NULL);\n+\tgit_config_get_bool(\"rebase.abbreviatecommands\", &abbreviate_commands);\n \n \topts.action = REPLAY_INTERACTIVE_REBASE;\n \topts.allow_ff = 1;\n@@ -52,6 +54,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\t\tbuiltin_rebase_helper_usage, PARSE_OPT_KEEP_ARGV0);\n \n \tflags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0;\n+\tflags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;\n \tflags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTED_IDS : 0;\n \n \tif (command == CONTINUE && argc == 1)\ndiff --git a/sequencer.c b/sequencer.c\nindex bd047737082d..b752dcc52982 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -795,6 +795,13 @@ static const char *command_to_string(const enum todo_command command)\n \tdie(\"Unknown command: %d\", command);\n }\n \n+static const char command_to_char(const enum todo_command command)\n+{\n+\tif (command < TODO_COMMENT && todo_command_info[command].c)\n+\t\treturn todo_command_info[command].c;\n+\treturn comment_line_char;\n+}\n+\n static int is_noop(const enum todo_command command)\n {\n \treturn TODO_NOOP <= command;\n@@ -2453,6 +2460,7 @@ int sequencer_make_script(FILE *out, int argc, const char **argv,\n \tstruct rev_info revs;\n \tstruct commit *commit;\n \tint keep_empty = flags & TODO_LIST_KEEP_EMPTY;\n+\tconst char *insn = flags & TODO_LIST_ABBREVIATE_CMDS ? \"p\" : \"pick\";\n \n \tinit_revisions(&revs, NULL);\n \trevs.verbose_header = 1;\n@@ -2485,7 +2493,8 @@ int sequencer_make_script(FILE *out, int argc, const char **argv,\n \t\tstrbuf_reset(&buf);\n \t\tif (!keep_empty && is_original_commit_empty(commit))\n \t\t\tstrbuf_addf(&buf, \"%c \", comment_line_char);\n-\t\tstrbuf_addf(&buf, \"pick %s \", oid_to_hex(&commit->object.oid));\n+\t\tstrbuf_addf(&buf, \"%s %s \", insn,\n+\t\t\t oid_to_hex(&commit->object.oid));\n \t\tpretty_print_commit(&pp, commit, &buf);\n \t\tstrbuf_addch(&buf, '\\n');\n \t\tfputs(buf.buf, out);\n@@ -2558,7 +2567,10 @@ int transform_todo_insn(unsigned flags)\n \t\t}\n \n \t\t/* add command to the buffer */\n-\t\tstrbuf_addstr(&buf, command_to_string(item->command));\n+\t\tif (flags & TODO_LIST_ABBREVIATE_CMDS)\n+\t\t\tstrbuf_addch(&buf, command_to_char(item->command));\n+\t\telse\n+\t\t\tstrbuf_addstr(&buf, command_to_string(item->command));\n \n \t\t/* add commit id */\n \t\tif (item->commit) {\ndiff --git a/sequencer.h b/sequencer.h\nindex e4a9d2419883..468ee79fb72d 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -47,6 +47,7 @@ int sequencer_remove_state(struct replay_opts *opts);\n \n #define TODO_LIST_KEEP_EMPTY (1U << 0)\n #define TODO_LIST_SHORTED_IDS (1U << 1)\n+#define TODO_LIST_ABBREVIATE_CMDS (1U << 2)\n int sequencer_make_script(FILE *out, int argc, const char **argv,\n \t\t\t unsigned flags);\n \n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334053","messageId":"alpine.DEB.2.21.1.1712041541000.98586@virtualbox","threadId":"47327","inReplyTo":"20171203221721.16462-5-liambeguin@gmail.com","subject":"Re: [PATCH v2 4/9] rebase -i: refactor transform_todo_ids","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-12-04T14:42:11Z","receivedAt":"2017-12-04T14:43:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Liam,\n\nOn Sun, 3 Dec 2017, Liam Beguin wrote:\n\n> The transform_todo_ids function is a little hard to read. Lets try\n> to make it easier by using more of the strbuf API. Also, since we'll\n> soon be adding command abbreviations, let's rename the function so\n> it's name reflects that change.\n\nI am not really a fan of the new name, and would prefer the old one, but\nthat's only a nit, not a reason to reject the patch.\n\nThe rest of it makes the code reads a lot nicer than before. Thank you,\nJohannes\n"},{"id":"334055","messageId":"alpine.DEB.2.21.1.1712041643250.98586@virtualbox","threadId":"47327","inReplyTo":"20171203221721.16462-7-liambeguin@gmail.com","subject":"Re: [PATCH v2 6/9] rebase -i: update functions to use a flags parameter","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-12-04T15:46:50Z","receivedAt":"2017-12-04T15:47:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Liam,\n\nOn Sun, 3 Dec 2017, Liam Beguin wrote:\n\n> diff --git a/sequencer.h b/sequencer.h\n> index 4e444e3bf1c4..3bb6b0658192 100644\n> --- a/sequencer.h\n> +++ b/sequencer.h\n> @@ -45,10 +45,12 @@ int sequencer_continue(struct replay_opts *opts);\n> int sequencer_rollback(struct replay_opts *opts);\n> int sequencer_remove_state(struct replay_opts *opts);\n> \n> -int sequencer_make_script(int keep_empty, FILE *out,\n> -\t\tint argc, const char **argv);\n> +#define TODO_LIST_KEEP_EMPTY (1U << 0)\n> +#define TODO_LIST_SHORTED_IDS (1U << 1)\n\nMaybe SHORTEN_IDs? And either revert back to transform_todo_ids() or use\nSHORTEN_INSNS...\n\nMaybe also TRANSFORM_TODO_LIST_* and maybe move the #define's above the\ntransform_todo_ids() function, i.e. one line further down?\n\n> +int sequencer_make_script(FILE *out, int argc, const char **argv,\n> +\t\t\t unsigned flags);\n> \n> -int transform_todo_insn(int shorten_ids);\n> +int transform_todo_insn(unsigned flags);\n> int check_todo_list(void);\n> int skip_unnecessary_picks(void);\n> int rearrange_squash(void);\n\nCiao,\nJohannes\n"},{"id":"334058","messageId":"alpine.DEB.2.21.1.1712041707050.98586@virtualbox","threadId":"47327","inReplyTo":"20171203221721.16462-1-liambeguin@gmail.com","subject":"Re: [PATCH v2 0/9] rebase -i: add config to abbreviate command names","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-12-04T16:07:39Z","receivedAt":"2017-12-04T16:08:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Liam,\n\nOn Sun, 3 Dec 2017, Liam Beguin wrote:\n\n> This series will add the 'rebase.abbreviateCommands' configuration\n> option to allow `git rebase -i` to default to the single-letter command\n> names when generating the todo list.\n> \n> Using single-letter command names can present two benefits. First, it\n> makes it easier to change the action since you only need to replace a\n> single character (i.e.: in vim \"r\" instead of\n> \"ciw\"). Second, using this with a large enough value of\n> 'core.abbrev' enables the lines of the todo list to remain aligned\n> making the files easier to read.\n> \n> Changes in V2:\n> - Refactor and rename 'transform_todo_ids'\n> - Replace SHA-1 by OID in rebase--helper.c\n> - Update todo list related functions to take a generic 'flags' parameter\n> - Rename 'add_exec_commands' function to 'sequencer_add_exec_commands'\n> - Rename 'add-exec' option to 'add-exec-commands'\n> - Use 'strbur_read_file' instead of rewriting it\n> - Make 'command_to_char' return 'comment_char_line' if no single-letter\n> command name is defined\n> - Combine both tests into a single test case\n> - Update commit messages\n\nLooks very nice already! I offered a couple of comments/suggestions, but\nnothing major.\n\nThank you,\nJohannes\n"},{"id":"334059","messageId":"xmqq4lp6o4p4.fsf@gitster.mtv.corp.google.com","threadId":"47327","inReplyTo":"alpine.DEB.2.21.1.1712041541000.98586@virtualbox","subject":"Re: [PATCH v2 4/9] rebase -i: refactor transform_todo_ids","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-04T16:09:27Z","receivedAt":"2017-12-04T16:09:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Sun, 3 Dec 2017, Liam Beguin wrote:\n>\n>> The transform_todo_ids function is a little hard to read. Lets try\n>> to make it easier by using more of the strbuf API. Also, since we'll\n>> soon be adding command abbreviations, let's rename the function so\n>> it's name reflects that change.\n>\n> I am not really a fan of the new name, and would prefer the old one, but\n> that's only a nit, not a reason to reject the patch.\n\nFWIW, I do think the new name goes backwards. The command uses to\nremember what operations are to be carried out in which order using\na thing, and the name of that thing \"todo list\". We also called it\nthe \"instruction sheet\", and \"insn\" was a good term to call one item\non that sheet among other items.\n\nBut recent commits in the area are pushing us to call it \"todo list\"\nconsistently. An element in that list should be called \"todo\".\n\nA \"todo\" consists of two parts, \"what operation is done\" part and\n\"using what commit object\" part. The original implementation of\nthis function affected only the latter part, and in that light, the\noriginal name transform_todo_ids() is understandable. Now you are\nplanning to make it modify both parts, not just \"ids\", so it is\nunderstandable that you would want to rename it. But I do not think\n\"insn\" matches the recent trend. It even risks misunderstanding\n(i.e. xfrm_todo_ids() is about modifying \"using what commit\" part,\nso perhaps xfrm_todo_insns() is about modifying \"what operation is\ndone\" part---but that is different from what you want to do, which\nis to update _both_ halves).\n\nCalling it just transform_todo() would probably be more in line with\nthe reason why you wanted to rename it in the first place.\n\n"},{"id":"334060","messageId":"alpine.DEB.2.21.1.1712041706120.98586@virtualbox","threadId":"47327","inReplyTo":"alpine.DEB.2.21.1.1712041643250.98586@virtualbox","subject":"Re: [PATCH v2 6/9] rebase -i: update functions to use a flags parameter","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-12-04T16:06:52Z","receivedAt":"2017-12-04T16:10:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Liam,\n\nOn Mon, 4 Dec 2017, Johannes Schindelin wrote:\n\n> On Sun, 3 Dec 2017, Liam Beguin wrote:\n> \n> > diff --git a/sequencer.h b/sequencer.h\n> > index 4e444e3bf1c4..3bb6b0658192 100644\n> > --- a/sequencer.h\n> > +++ b/sequencer.h\n> > @@ -45,10 +45,12 @@ int sequencer_continue(struct replay_opts *opts);\n> > int sequencer_rollback(struct replay_opts *opts);\n> > int sequencer_remove_state(struct replay_opts *opts);\n> > \n> > -int sequencer_make_script(int keep_empty, FILE *out,\n> > -\t\tint argc, const char **argv);\n> > +#define TODO_LIST_KEEP_EMPTY (1U << 0)\n> > +#define TODO_LIST_SHORTED_IDS (1U << 1)\n> \n> Maybe SHORTEN_IDs? And either revert back to transform_todo_ids() or use\n> SHORTEN_INSNS...\n> \n> Maybe also TRANSFORM_TODO_LIST_* and maybe move the #define's above the\n> transform_todo_ids() function, i.e. one line further down?\n> \n> > +int sequencer_make_script(FILE *out, int argc, const char **argv,\n> > +\t\t\t unsigned flags);\n\nAh, I just realized that make_script() already takes `flags`. Sorry for\nthe noise,\nJohannes\n"},{"id":"334132","messageId":"ddb4bc14-0598-aaab-af1c-e3a714a6c49b@gmail.com","threadId":"47327","inReplyTo":"xmqq4lp6o4p4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 4/9] rebase -i: refactor transform_todo_ids","fromName":"liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-05T03:36:53Z","receivedAt":"2017-12-05T03:37:01Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Hi Junio,\n\nOn 04/12/17 11:09 AM, Junio C Hamano wrote:\n> Johannes Schindelin writes:\n> \n>> On Sun, 3 Dec 2017, Liam Beguin wrote:\n>>\n>>> The transform_todo_ids function is a little hard to read. Lets try\n>>> to make it easier by using more of the strbuf API. Also, since we'll\n>>> soon be adding command abbreviations, let's rename the function so\n>>> it's name reflects that change.\n>>\n>> I am not really a fan of the new name, and would prefer the old one, but\n>> that's only a nit, not a reason to reject the patch.\n> \n> FWIW, I do think the new name goes backwards. The command uses to\n> remember what operations are to be carried out in which order using\n> a thing, and the name of that thing \"todo list\". We also called it\n> the \"instruction sheet\", and \"insn\" was a good term to call one item\n> on that sheet among other items.\n> \n\nGood point on saying this name change is going backwards.\n\n> But recent commits in the area are pushing us to call it \"todo list\"\n> consistently. An element in that list should be called \"todo\".\n> \n> A \"todo\" consists of two parts, \"what operation is done\" part and\n> \"using what commit object\" part. The original implementation of\n> this function affected only the latter part, and in that light, the\n> original name transform_todo_ids() is understandable. Now you are\n> planning to make it modify both parts, not just \"ids\", so it is\n> understandable that you would want to rename it. But I do not think\n> \"insn\" matches the recent trend. It even risks misunderstanding\n> (i.e. xfrm_todo_ids() is about modifying \"using what commit\" part,\n> so perhaps xfrm_todo_insns() is about modifying \"what operation is\n> done\" part---but that is different from what you want to do, which\n> is to update _both_ halves).\n> \n\nYou're right! We do want the name to reflect that we intend to change\nboth halves and not only the command.\n\n> Calling it just transform_todo() would probably be more in line with\n> the reason why you wanted to rename it in the first place.\n> \n\nGood suggestion. Would transform_todos() work too? I'll send an update\ntomorrow.\nThanks, \n\nLiam\n"},{"id":"334133","messageId":"edf23a51-f08d-6e61-b1a5-af8929c477ab@gmail.com","threadId":"47327","inReplyTo":"alpine.DEB.2.21.1.1712041541000.98586@virtualbox","subject":"Re: [PATCH v2 4/9] rebase -i: refactor transform_todo_ids","fromName":"liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-05T03:39:40Z","receivedAt":"2017-12-05T03:39:48Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Hi Johannes,\n\nOn 04/12/17 09:42 AM, Johannes Schindelin wrote:\n> Hi Liam,\n> \n> On Sun, 3 Dec 2017, Liam Beguin wrote:\n> \n>> The transform_todo_ids function is a little hard to read. Lets try\n>> to make it easier by using more of the strbuf API. Also, since we'll\n>> soon be adding command abbreviations, let's rename the function so\n>> it's name reflects that change.\n> \n> I am not really a fan of the new name, and would prefer the old one, but\n> that's only a nit, not a reason to reject the patch.\n> \n\nYou're right, it's probably not the best name. I'll change it to\ntransform_todos() as we want the function name to reflect that it changes\nboth parts of the todo.\n\n> The rest of it makes the code reads a lot nicer than before. Thank you,\n> Johannes\n> \n\nThanks,\nLiam\n"},{"id":"334134","messageId":"22f665eb-0ed1-27d4-7184-e6063ea5b47e@gmail.com","threadId":"47327","inReplyTo":"alpine.DEB.2.21.1.1712041643250.98586@virtualbox","subject":"Re: [PATCH v2 6/9] rebase -i: update functions to use a flags parameter","fromName":"liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-05T03:42:43Z","receivedAt":"2017-12-05T03:42:50Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Hi Johannes,\n\nOn 04/12/17 10:46 AM, Johannes Schindelin wrote:\n> Hi Liam,\n> \n> On Sun, 3 Dec 2017, Liam Beguin wrote:\n> \n>> diff --git a/sequencer.h b/sequencer.h\n>> index 4e444e3bf1c4..3bb6b0658192 100644\n>> --- a/sequencer.h\n>> +++ b/sequencer.h\n>> @@ -45,10 +45,12 @@ int sequencer_continue(struct replay_opts *opts);\n>> int sequencer_rollback(struct replay_opts *opts);\n>> int sequencer_remove_state(struct replay_opts *opts);\n>> \n>> -int sequencer_make_script(int keep_empty, FILE *out,\n>> -\t\tint argc, const char **argv);\n>> +#define TODO_LIST_KEEP_EMPTY (1U << 0)\n>> +#define TODO_LIST_SHORTED_IDS (1U << 1)\n> \n> Maybe SHORTEN_IDs? And either revert back to transform_todo_ids() or use\n> SHORTEN_INSNS...\n> \n\nI'll change it to TODO_LIST_SHORTED_IDs. TODO_LIST_SHORTED_INSNS would\nsuggest the flag changes both parts of the todo.\n\n> Maybe also TRANSFORM_TODO_LIST_* and maybe move the #define's above the\n> transform_todo_ids() function, i.e. one line further down?\n> \n>> +int sequencer_make_script(FILE *out, int argc, const char **argv,\n>> +\t\t\t unsigned flags);\n>> \n>> -int transform_todo_insn(int shorten_ids);\n>> +int transform_todo_insn(unsigned flags);\n>> int check_todo_list(void);\n>> int skip_unnecessary_picks(void);\n>> int rearrange_squash(void);\n> \n> Ciao,\n> Johannes\n> \n\nThanks, \nLiam\n"},{"id":"334148","messageId":"xmqq1sk9l5dp.fsf@gitster.mtv.corp.google.com","threadId":"47327","inReplyTo":"ddb4bc14-0598-aaab-af1c-e3a714a6c49b@gmail.com","subject":"Re: [PATCH v2 4/9] rebase -i: refactor transform_todo_ids","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-05T12:35:14Z","receivedAt":"2017-12-05T12:35:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"liam Beguin writes:\n\n> Good suggestion. Would transform_todos() work too?\n\nIf the function is about munging multiple of them, then todo\"s\"\nwould work well; I wasn't focusing on singular vs plural, as I\nthought the choice between them needs much less thought to make\ncorrectly.\n\n"},{"id":"334149","messageId":"xmqqwp21jqpl.fsf@gitster.mtv.corp.google.com","threadId":"47327","inReplyTo":"22f665eb-0ed1-27d4-7184-e6063ea5b47e@gmail.com","subject":"Re: [PATCH v2 6/9] rebase -i: update functions to use a flags parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-05T12:37:26Z","receivedAt":"2017-12-05T12:37:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"liam Beguin writes:\n\n> I'll change it to TODO_LIST_SHORTED_IDs. TODO_LIST_SHORTED_INSNS would\n> suggest the flag changes both parts of the todo.\n\nI am not a native speaker, but SHORTED does not sound like a right\nphrase. When you make something shorter, that thing is \"shortened\",\nnot \"shorted\".\n\n"},{"id":"334150","messageId":"HE1PR0201MB19938A1581F799CB9D48AE999C3D0@HE1PR0201MB1993.eurprd02.prod.outlook.com","threadId":"47327","inReplyTo":"xmqqwp21jqpl.fsf@gitster.mtv.corp.google.com","subject":"RE: [PATCH v2 6/9] rebase -i: update functions to use a flags parameter","fromName":"Kerry, Richard","fromEmail":"richard.kerry@atos.net","sentAt":"2017-12-05T12:41:03Z","receivedAt":"2017-12-05T12:47:44Z","isPatch":true,"sender":{"key":"richard.kerry@atos.net","avatar":null},"body":"\n\"Shorted\" is what happens when you put a piece of wire across the terminals of a battery ... (bang, smoke, etc).\nIt's short for \"short-circuited\".\nYes, I think you mean \"shortened\" in this case.\n\nRegards,\nRichard.\n\n\n\nRichard Kerry\nBNCS Engineer, SI SOL Telco & Media Vertical Practice\n\nT: +44 (0)20 3618 2669\nM: +44 (0)7812 325518\nLync: +44 (0) 20 3618 0778\nRoom G300, Stadium House, Wood Lane, London, W12 7TA\nrichard.kerry@atos.net\n\n\n\n\n> -----Original Message-----\n> From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On\n> Behalf Of Junio C Hamano\n> Sent: Tuesday, December 05, 2017 12:37 PM\n> To: liam Beguin \n> Cc: Johannes Schindelin ;\n> git@vger.kernel.org; peff@peff.net\n> Subject: Re: [PATCH v2 6/9] rebase -i: update functions to use a flags\n> parameter\n>\n> liam Beguin writes:\n>\n> > I'll change it to TODO_LIST_SHORTED_IDs. TODO_LIST_SHORTED_INSNS\n> would\n> > suggest the flag changes both parts of the todo.\n>\n> I am not a native speaker, but SHORTED does not sound like a right phrase.\n> When you make something shorter, that thing is \"shortened\", not \"shorted\".\n\nAtos, Atos Consulting, Worldline and Canopy The Open Cloud Company are trading names used by the Atos group. The following trading entities are registered in England and Wales: Atos IT Services UK Limited (registered number 01245534), Atos Consulting Limited (registered number 04312380), Atos Worldline UK Limited (registered number 08514184) and Canopy The Open Cloud Company Limited (registration number 08011902). The registered office for each is at 4 Triton Square, Regent’s Place, London, NW1 3HG.The VAT No. for each is: GB232327983.\n\nThis e-mail and the documents attached are confidential and intended solely for the addressee, and may contain confidential or privileged information. If you receive this e-mail in error, you are not authorised to copy, disclose, use or retain it. Please notify the sender immediately and delete this email from your systems. As emails may be intercepted, amended or lost, they are not secure. Atos therefore can accept no liability for any errors or their content. Although Atos endeavours to maintain a virus-free network, we do not warrant that this transmission is virus-free and can accept no liability for any damages resulting from any virus transmitted. The risks are deemed to be accepted by everyone who communicates with Atos by email.\n"},{"id":"334155","messageId":"a80f0166-3c3f-85aa-9961-67b1032a96b0@gmail.com","threadId":"47327","inReplyTo":"HE1PR0201MB19938A1581F799CB9D48AE999C3D0@HE1PR0201MB1993.eurprd02.prod.outlook.com","subject":"Re: [PATCH v2 6/9] rebase -i: update functions to use a flags parameter","fromName":"liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-05T14:42:28Z","receivedAt":"2017-12-05T14:42:38Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Hi,\n\nOn 05/12/17 07:41 AM, Kerry, Richard wrote:\n> \n> \"Shorted\" is what happens when you put a piece of wire across the terminals of a battery ... (bang, smoke, etc).\n> It's short for \"short-circuited\".\n> Yes, I think you mean \"shortened\" in this case.\n> \n\nThanks for the explanation.\nSorry, my eyes stopped at the lowercase 's' in Johannes message.\nWill fix.\n\n> Regards,\n> Richard.\n> \n> \n> \n> Richard Kerry\n> BNCS Engineer, SI SOL Telco & Media Vertical Practice\n> \n> T: +44 (0)20 3618 2669\n> M: +44 (0)7812 325518\n> Lync: +44 (0) 20 3618 0778\n> Room G300, Stadium House, Wood Lane, London, W12 7TA\n> richard.kerry@atos.net\n> \n> \n\n[...]\n\nThanks,\nLiam\n"},{"id":"334161","messageId":"xmqq4lp5jh2n.fsf@gitster.mtv.corp.google.com","threadId":"47327","inReplyTo":"HE1PR0201MB19938A1581F799CB9D48AE999C3D0@HE1PR0201MB1993.eurprd02.prod.outlook.com","subject":"Re: [PATCH v2 6/9] rebase -i: update functions to use a flags parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-05T16:05:36Z","receivedAt":"2017-12-05T16:05:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kerry, Richard\" writes:\n\n> \"Shorted\" is what happens when you put a piece of wire across the terminals of a battery ... (bang, smoke, etc).\n> It's short for \"short-circuited\".\n\nOr it is what you do to something that you sell and that you yet do\nnot own, expecting that you can later buy it cheaper, allowing you\nto pocket the difference ;-).\n\n\n"},{"id":"334162","messageId":"HE1PR0201MB1993E5FBA6241CFA7EF2A1EF9C3D0@HE1PR0201MB1993.eurprd02.prod.outlook.com","threadId":"47327","inReplyTo":"xmqq4lp5jh2n.fsf@gitster.mtv.corp.google.com","subject":"RE: [PATCH v2 6/9] rebase -i: update functions to use a flags parameter","fromName":"Kerry, Richard","fromEmail":"richard.kerry@atos.net","sentAt":"2017-12-05T16:14:53Z","receivedAt":"2017-12-05T16:15:10Z","isPatch":true,"sender":{"key":"richard.kerry@atos.net","avatar":null},"body":"Indeed so.\nIn which case it is short for \"selling short\", or possibly \"short selling\".\n\nOf course a little searching shows that \"shorted\" could mean some other things, including possibly the meaning originally suggested.\nNevertheless it seems to me that \"shortened\" is the most appropriate word in modern English.\n\nR.\n\n\nRichard Kerry\nBNCS Engineer, SI SOL Telco & Media Vertical Practice\n\nT: +44 (0)20 3618 2669\nM: +44 (0)7812 325518\nLync: +44 (0) 20 3618 0778\nRoom G300, Stadium House, Wood Lane, London, W12 7TA\nrichard.kerry@atos.net\n\n\n\n\n> -----Original Message-----\n> From: Junio C Hamano [mailto:gitster@pobox.com]\n> Sent: Tuesday, December 05, 2017 4:06 PM\n> To: Kerry, Richard \n> Cc: git@vger.kernel.org; Johannes Schindelin\n> ; peff@peff.net; liam Beguin\n> \n> Subject: Re: [PATCH v2 6/9] rebase -i: update functions to use a flags\n> parameter\n>\n> \"Kerry, Richard\" writes:\n>\n> > \"Shorted\" is what happens when you put a piece of wire across the\n> terminals of a battery ... (bang, smoke, etc).\n> > It's short for \"short-circuited\".\n>\n> Or it is what you do to something that you sell and that you yet do not own,\n> expecting that you can later buy it cheaper, allowing you to pocket the\n> difference ;-).\n>\n\nAtos, Atos Consulting, Worldline and Canopy The Open Cloud Company are trading names used by the Atos group. The following trading entities are registered in England and Wales: Atos IT Services UK Limited (registered number 01245534), Atos Consulting Limited (registered number 04312380), Atos Worldline UK Limited (registered number 08514184) and Canopy The Open Cloud Company Limited (registration number 08011902). The registered office for each is at 4 Triton Square, Regent’s Place, London, NW1 3HG.The VAT No. for each is: GB232327983.\n\nThis e-mail and the documents attached are confidential and intended solely for the addressee, and may contain confidential or privileged information. If you receive this e-mail in error, you are not authorised to copy, disclose, use or retain it. Please notify the sender immediately and delete this email from your systems. As emails may be intercepted, amended or lost, they are not secure. Atos therefore can accept no liability for any errors or their content. Although Atos endeavours to maintain a virus-free network, we do not warrant that this transmission is virus-free and can accept no liability for any damages resulting from any virus transmitted. The risks are deemed to be accepted by everyone who communicates with Atos by email.\n"},{"id":"334198","messageId":"20171205175235.32319-1-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171127045514.25647-1-liambeguin@gmail.com","subject":"[PATCH v2 0/9] rebase -i: add config to abbreviate command names","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-05T17:52:26Z","receivedAt":"2017-12-05T17:52:55Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Hi everyone,\n\nThis series will add the 'rebase.abbreviateCommands' configuration\noption to allow `git rebase -i` to default to the single-letter command\nnames when generating the todo list.\n\nUsing single-letter command names can present two benefits. First, it\nmakes it easier to change the action since you only need to replace a\nsingle character (i.e.: in vim \"r\" instead of\n\"ciw\"). Second, using this with a large enough value of\n'core.abbrev' enables the lines of the todo list to remain aligned\nmaking the files easier to read.\n\nChanges in V2:\n- Refactor and rename 'transform_todo_ids'\n- Replace SHA-1 by OID in rebase--helper.c\n- Update todo list related functions to take a generic 'flags' parameter\n- Rename 'add_exec_commands' function to 'sequencer_add_exec_commands'\n- Rename 'add-exec' option to 'add-exec-commands'\n- Use 'strbur_read_file' instead of rewriting it\n- Make 'command_to_char' return 'comment_char_line' if no single-letter\n command name is defined\n- Combine both tests into a single test case\n- Update commit messages\n\nChanges in V2:\n- Rename 'transform_todo_insn' to 'transform_todos'\n- Fix flag name TODO_LIST_SHORTE{D,N}_IDS\n\nLiam Beguin (9):\n Documentation: move rebase.* configs to new file\n Documentation: use preferred name for the 'todo list' script\n rebase -i: set commit to null in exec commands\n rebase -i: refactor transform_todo_ids\n rebase -i: replace reference to sha1 with oid\n rebase -i: update functions to use a flags parameter\n rebase -i -x: add exec commands via the rebase--helper\n rebase -i: learn to abbreviate command names\n t3404: add test case for abbreviated commands\n\n Documentation/config.txt | 31 +-------\n Documentation/git-rebase.txt | 19 +----\n Documentation/rebase-config.txt | 52 +++++++++++++\n builtin/rebase--helper.c | 29 +++++---\n git-rebase--interactive.sh | 23 +-----\n sequencer.c | 126 +++++++++++++++++++++-----------\n sequencer.h | 10 ++-\n t/t3404-rebase-interactive.sh | 22 ++++++\n 8 files changed, 186 insertions(+), 126 deletions(-)\n create mode 100644 Documentation/rebase-config.txt\n\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334199","messageId":"20171205175235.32319-2-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171205175235.32319-1-liambeguin@gmail.com","subject":"[PATCH v3 1/9] Documentation: move rebase.* configs to new file","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-05T17:52:27Z","receivedAt":"2017-12-05T17:52:58Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Move all rebase.* configuration variables to a separate file in order to\nremove duplicates, and include it in config.txt and git-rebase.txt. The\nnew descriptions are mostly taken from config.txt as they are more\nverbose.\n\nSigned-off-by: Liam Beguin \n---\n Documentation/config.txt | 31 +------------------------------\n Documentation/git-rebase.txt | 19 +------------------\n Documentation/rebase-config.txt | 32 ++++++++++++++++++++++++++++++++\n 3 files changed, 34 insertions(+), 48 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 531649cb40ea..e424b7de90b5 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2691,36 +2691,7 @@ push.recurseSubmodules::\n \tis retained. You may override this configuration at time of push by\n \tspecifying '--recurse-submodules=check|on-demand|no'.\n \n-rebase.stat::\n-\tWhether to show a diffstat of what changed upstream since the last\n-\trebase. False by default.\n-\n-rebase.autoSquash::\n-\tIf set to true enable `--autosquash` option by default.\n-\n-rebase.autoStash::\n-\tWhen set to true, automatically create a temporary stash entry\n-\tbefore the operation begins, and apply it after the operation\n-\tends. This means that you can run rebase on a dirty worktree.\n-\tHowever, use with care: the final stash application after a\n-\tsuccessful rebase might result in non-trivial conflicts.\n-\tDefaults to false.\n-\n-rebase.missingCommitsCheck::\n-\tIf set to \"warn\", git rebase -i will print a warning if some\n-\tcommits are removed (e.g. a line was deleted), however the\n-\trebase will still proceed. If set to \"error\", it will print\n-\tthe previous warning and stop the rebase, 'git rebase\n-\t--edit-todo' can then be used to correct the error. If set to\n-\t\"ignore\", no checking is done.\n-\tTo drop a commit without warning or error, use the `drop`\n-\tcommand in the todo-list.\n-\tDefaults to \"ignore\".\n-\n-rebase.instructionFormat::\n-\tA format string, as specified in linkgit:git-log[1], to be used for\n-\tthe instruction list during an interactive rebase. The format will automatically\n-\thave the long commit hash prepended to the format.\n+include::rebase-config.txt[]\n \n receive.advertiseAtomic::\n \tBy default, git-receive-pack will advertise the atomic push\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 3cedfb0fd22b..8a861c1e0d69 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -203,24 +203,7 @@ Alternatively, you can undo the 'git rebase' with\n CONFIGURATION\n -------------\n \n-rebase.stat::\n-\tWhether to show a diffstat of what changed upstream since the last\n-\trebase. False by default.\n-\n-rebase.autoSquash::\n-\tIf set to true enable `--autosquash` option by default.\n-\n-rebase.autoStash::\n-\tIf set to true enable `--autostash` option by default.\n-\n-rebase.missingCommitsCheck::\n-\tIf set to \"warn\", print warnings about removed commits in\n-\tinteractive mode. If set to \"error\", print the warnings and\n-\tstop the rebase. If set to \"ignore\", no checking is\n-\tdone. \"ignore\" by default.\n-\n-rebase.instructionFormat::\n-\tCustom commit list format to use during an `--interactive` rebase.\n+include::rebase-config.txt[]\n \n OPTIONS\n -------\ndiff --git a/Documentation/rebase-config.txt b/Documentation/rebase-config.txt\nnew file mode 100644\nindex 000000000000..dba088d7c68f\n--- /dev/null\n+++ b/Documentation/rebase-config.txt\n@@ -0,0 +1,32 @@\n+rebase.stat::\n+\tWhether to show a diffstat of what changed upstream since the last\n+\trebase. False by default.\n+\n+rebase.autoSquash::\n+\tIf set to true enable `--autosquash` option by default.\n+\n+rebase.autoStash::\n+\tWhen set to true, automatically create a temporary stash entry\n+\tbefore the operation begins, and apply it after the operation\n+\tends. This means that you can run rebase on a dirty worktree.\n+\tHowever, use with care: the final stash application after a\n+\tsuccessful rebase might result in non-trivial conflicts.\n+\tThis option can be overridden by the `--no-autostash` and\n+\t`--autostash` options of linkgit:git-rebase[1].\n+\tDefaults to false.\n+\n+rebase.missingCommitsCheck::\n+\tIf set to \"warn\", git rebase -i will print a warning if some\n+\tcommits are removed (e.g. a line was deleted), however the\n+\trebase will still proceed. If set to \"error\", it will print\n+\tthe previous warning and stop the rebase, 'git rebase\n+\t--edit-todo' can then be used to correct the error. If set to\n+\t\"ignore\", no checking is done.\n+\tTo drop a commit without warning or error, use the `drop`\n+\tcommand in the todo-list.\n+\tDefaults to \"ignore\".\n+\n+rebase.instructionFormat::\n+\tA format string, as specified in linkgit:git-log[1], to be used for the\n+\tinstruction list during an interactive rebase. The format will\n+\tautomatically have the long commit hash prepended to the format.\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334200","messageId":"20171205175235.32319-3-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171205175235.32319-1-liambeguin@gmail.com","subject":"[PATCH v3 2/9] Documentation: use preferred name for the 'todo list' script","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-05T17:52:28Z","receivedAt":"2017-12-05T17:53:02Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Use \"todo list\" instead of \"instruction list\" or \"todo-list\" to\nreduce further confusion regarding the name of this script.\n\nSigned-off-by: Liam Beguin \n---\n Documentation/rebase-config.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/rebase-config.txt b/Documentation/rebase-config.txt\nindex dba088d7c68f..30ae08cb5a4b 100644\n--- a/Documentation/rebase-config.txt\n+++ b/Documentation/rebase-config.txt\n@@ -23,10 +23,10 @@ rebase.missingCommitsCheck::\n \t--edit-todo' can then be used to correct the error. If set to\n \t\"ignore\", no checking is done.\n \tTo drop a commit without warning or error, use the `drop`\n-\tcommand in the todo-list.\n+\tcommand in the todo list.\n \tDefaults to \"ignore\".\n \n rebase.instructionFormat::\n \tA format string, as specified in linkgit:git-log[1], to be used for the\n-\tinstruction list during an interactive rebase. The format will\n+\ttodo list during an interactive rebase. The format will\n \tautomatically have the long commit hash prepended to the format.\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334201","messageId":"20171205175235.32319-4-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171205175235.32319-1-liambeguin@gmail.com","subject":"[PATCH v3 3/9] rebase -i: set commit to null in exec commands","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-05T17:52:29Z","receivedAt":"2017-12-05T17:53:04Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Make sure commit is set to NULL when parsing exec instructions\nfrom the todo list. If not, we may try to access an uninitialized\naddress later while updating the todo list.\n\nSigned-off-by: Liam Beguin \n---\n sequencer.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex fa94ed652d2c..5033b049d995 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1268,6 +1268,7 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)\n \tbol += padding;\n \n \tif (item->command == TODO_EXEC) {\n+\t\titem->commit = NULL;\n \t\titem->arg = bol;\n \t\titem->arg_len = (int)(eol - bol);\n \t\treturn 0;\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334202","messageId":"20171205175235.32319-5-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171205175235.32319-1-liambeguin@gmail.com","subject":"[PATCH v3 4/9] rebase -i: refactor transform_todo_ids","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-05T17:52:30Z","receivedAt":"2017-12-05T17:53:09Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"The transform_todo_ids function is a little hard to read. Lets try\nto make it easier by using more of the strbuf API. Also, since we'll\nsoon be adding command abbreviations, let's rename the function so\nit's name reflects that change.\n\nSigned-off-by: Liam Beguin \n---\n builtin/rebase--helper.c | 4 +--\n sequencer.c | 69 ++++++++++++++++------------------------\n sequencer.h | 2 +-\n 3 files changed, 31 insertions(+), 44 deletions(-)\n\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex f8519363a393..8ad4779d1650 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -55,9 +55,9 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \tif (command == MAKE_SCRIPT && argc > 1)\n \t\treturn !!sequencer_make_script(keep_empty, stdout, argc, argv);\n \tif (command == SHORTEN_SHA1S && argc == 1)\n-\t\treturn !!transform_todo_ids(1);\n+\t\treturn !!transform_todos(1);\n \tif (command == EXPAND_SHA1S && argc == 1)\n-\t\treturn !!transform_todo_ids(0);\n+\t\treturn !!transform_todos(0);\n \tif (command == CHECK_TODO_LIST && argc == 1)\n \t\treturn !!check_todo_list();\n \tif (command == SKIP_UNNECESSARY_PICKS && argc == 1)\ndiff --git a/sequencer.c b/sequencer.c\nindex 5033b049d995..c9a661a8c4bd 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2494,60 +2494,47 @@ int sequencer_make_script(int keep_empty, FILE *out,\n }\n \n \n-int transform_todo_ids(int shorten_ids)\n+int transform_todos(int shorten_ids)\n {\n \tconst char *todo_file = rebase_path_todo();\n \tstruct todo_list todo_list = TODO_LIST_INIT;\n-\tint fd, res, i;\n-\tFILE *out;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct todo_item *item;\n+\tint i;\n \n-\tstrbuf_reset(&todo_list.buf);\n-\tfd = open(todo_file, O_RDONLY);\n-\tif (fd < 0)\n-\t\treturn error_errno(_(\"could not open '%s'\"), todo_file);\n-\tif (strbuf_read(&todo_list.buf, fd, 0) < 0) {\n-\t\tclose(fd);\n+\tif (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)\n \t\treturn error(_(\"could not read '%s'.\"), todo_file);\n-\t}\n-\tclose(fd);\n \n-\tres = parse_insn_buffer(todo_list.buf.buf, &todo_list);\n-\tif (res) {\n+\tif (parse_insn_buffer(todo_list.buf.buf, &todo_list)) {\n \t\ttodo_list_release(&todo_list);\n \t\treturn error(_(\"unusable todo list: '%s'\"), todo_file);\n \t}\n \n-\tout = fopen(todo_file, \"w\");\n-\tif (!out) {\n-\t\ttodo_list_release(&todo_list);\n-\t\treturn error(_(\"unable to open '%s' for writing\"), todo_file);\n-\t}\n-\tfor (i = 0; i < todo_list.nr; i++) {\n-\t\tstruct todo_item *item = todo_list.items + i;\n-\t\tint bol = item->offset_in_buf;\n-\t\tconst char *p = todo_list.buf.buf + bol;\n-\t\tint eol = i + 1 < todo_list.nr ?\n-\t\t\ttodo_list.items[i + 1].offset_in_buf :\n-\t\t\ttodo_list.buf.len;\n-\n-\t\tif (item->command >= TODO_EXEC && item->command != TODO_DROP)\n-\t\t\tfwrite(p, eol - bol, 1, out);\n-\t\telse {\n-\t\t\tconst char *id = shorten_ids ?\n-\t\t\t\tshort_commit_name(item->commit) :\n-\t\t\t\toid_to_hex(&item->commit->object.oid);\n-\t\t\tint len;\n-\n-\t\t\tp += strspn(p, \" \\t\"); /* left-trim command */\n-\t\t\tlen = strcspn(p, \" \\t\"); /* length of command */\n-\n-\t\t\tfprintf(out, \"%.*s %s %.*s\\n\",\n-\t\t\t\tlen, p, id, item->arg_len, item->arg);\n+\tfor (item = todo_list.items, i = 0; i < todo_list.nr; i++, item++) {\n+\t\t/* if the item is not a command write it and continue */\n+\t\tif (item->command >= TODO_COMMENT) {\n+\t\t\tstrbuf_addf(&buf, \"%.*s\\n\", item->arg_len, item->arg);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/* add command to the buffer */\n+\t\tstrbuf_addstr(&buf, command_to_string(item->command));\n+\n+\t\t/* add commit id */\n+\t\tif (item->commit) {\n+\t\t\tconst char *oid = shorten_ids ?\n+\t\t\t\t\t short_commit_name(item->commit) :\n+\t\t\t\t\t oid_to_hex(&item->commit->object.oid);\n+\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}\n-\tfclose(out);\n+\n+\ti = write_message(buf.buf, buf.len, todo_file, 0);\n \ttodo_list_release(&todo_list);\n-\treturn 0;\n+\treturn i;\n }\n \n enum check_level {\ndiff --git a/sequencer.h b/sequencer.h\nindex 6f3d3df82c0a..4f7f2c93f83e 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -48,7 +48,7 @@ int sequencer_remove_state(struct replay_opts *opts);\n int sequencer_make_script(int keep_empty, FILE *out,\n \t\tint argc, const char **argv);\n \n-int transform_todo_ids(int shorten_ids);\n+int transform_todos(int shorten_ids);\n int check_todo_list(void);\n int skip_unnecessary_picks(void);\n int rearrange_squash(void);\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334203","messageId":"20171205175235.32319-6-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171205175235.32319-1-liambeguin@gmail.com","subject":"[PATCH v3 5/9] rebase -i: replace reference to sha1 with oid","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-05T17:52:31Z","receivedAt":"2017-12-05T17:53:12Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Since we are trying to abstract the hash function name elsewhere in the\ncode base, lets use OID instead of SHA-1 in the rebase--helper too.\n\nSigned-off-by: Liam Beguin \n---\n builtin/rebase--helper.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex 8ad4779d1650..c3b8e4d401f8 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -14,7 +14,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \tstruct replay_opts opts = REPLAY_OPTS_INIT;\n \tint keep_empty = 0;\n \tenum {\n-\t\tCONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S,\n+\t\tCONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,\n \t\tCHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH\n \t} command = 0;\n \tstruct option options[] = {\n@@ -27,9 +27,9 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\tOPT_CMDMODE(0, \"make-script\", &command,\n \t\t\tN_(\"make rebase script\"), MAKE_SCRIPT),\n \t\tOPT_CMDMODE(0, \"shorten-ids\", &command,\n-\t\t\tN_(\"shorten SHA-1s in the todo list\"), SHORTEN_SHA1S),\n+\t\t\tN_(\"shorten commit ids in the todo list\"), SHORTEN_OIDS),\n \t\tOPT_CMDMODE(0, \"expand-ids\", &command,\n-\t\t\tN_(\"expand SHA-1s in the todo list\"), EXPAND_SHA1S),\n+\t\t\tN_(\"expand commit ids in the todo list\"), EXPAND_OIDS),\n \t\tOPT_CMDMODE(0, \"check-todo-list\", &command,\n \t\t\tN_(\"check the todo list\"), CHECK_TODO_LIST),\n \t\tOPT_CMDMODE(0, \"skip-unnecessary-picks\", &command,\n@@ -54,9 +54,9 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\treturn !!sequencer_remove_state(&opts);\n \tif (command == MAKE_SCRIPT && argc > 1)\n \t\treturn !!sequencer_make_script(keep_empty, stdout, argc, argv);\n-\tif (command == SHORTEN_SHA1S && argc == 1)\n+\tif (command == SHORTEN_OIDS && argc == 1)\n \t\treturn !!transform_todos(1);\n-\tif (command == EXPAND_SHA1S && argc == 1)\n+\tif (command == EXPAND_OIDS && argc == 1)\n \t\treturn !!transform_todos(0);\n \tif (command == CHECK_TODO_LIST && argc == 1)\n \t\treturn !!check_todo_list();\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334204","messageId":"20171205175235.32319-8-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171205175235.32319-1-liambeguin@gmail.com","subject":"[PATCH v3 7/9] rebase -i -x: add exec commands via the rebase--helper","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-05T17:52:33Z","receivedAt":"2017-12-05T17:53:15Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Recent work on `git-rebase--interactive` aims to convert shell code to\nC. Even if this is most likely not a big performance enhancement, let's\nconvert it too since a coming change to abbreviate command names\nrequires it to be updated.\n\nSigned-off-by: Liam Beguin \n---\n builtin/rebase--helper.c | 7 ++++++-\n git-rebase--interactive.sh | 23 +---------------------\n sequencer.c | 39 ++++++++++++++++++++++++++++++++++++++\n sequencer.h | 1 +\n 4 files changed, 47 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex 1102ecb43b67..4229ea0dc122 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -15,7 +15,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \tunsigned flags = 0, keep_empty = 0;\n \tenum {\n \t\tCONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,\n-\t\tCHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH\n+\t\tCHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,\n+\t\tADD_EXEC\n \t} command = 0;\n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"ff\", &opts.allow_ff, N_(\"allow fast-forward\")),\n@@ -36,6 +37,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\t\tN_(\"skip unnecessary picks\"), SKIP_UNNECESSARY_PICKS),\n \t\tOPT_CMDMODE(0, \"rearrange-squash\", &command,\n \t\t\tN_(\"rearrange fixup/squash lines\"), REARRANGE_SQUASH),\n+\t\tOPT_CMDMODE(0, \"add-exec-commands\", &command,\n+\t\t\tN_(\"insert exec commands in todo list\"), ADD_EXEC),\n \t\tOPT_END()\n \t};\n \n@@ -65,5 +68,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\treturn !!skip_unnecessary_picks();\n \tif (command == REARRANGE_SQUASH && argc == 1)\n \t\treturn !!rearrange_squash();\n+\tif (command == ADD_EXEC && argc == 2)\n+\t\treturn !!sequencer_add_exec_commands(argv[1]);\n \tusage_with_options(builtin_rebase_helper_usage, options);\n }\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 437815669f00..e3f5a0abf3c7 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -722,27 +722,6 @@ collapse_todo_ids() {\n \tgit rebase--helper --shorten-ids\n }\n \n-# Add commands after a pick or after a squash/fixup series\n-# in the todo list.\n-add_exec_commands () {\n-\t{\n-\t\tfirst=t\n-\t\twhile read -r insn rest\n-\t\tdo\n-\t\t\tcase $insn in\n-\t\t\tpick)\n-\t\t\t\ttest -n \"$first\" ||\n-\t\t\t\tprintf \"%s\" \"$cmd\"\n-\t\t\t\t;;\n-\t\t\tesac\n-\t\t\tprintf \"%s %s\\n\" \"$insn\" \"$rest\"\n-\t\t\tfirst=\n-\t\tdone\n-\t\tprintf \"%s\" \"$cmd\"\n-\t} <\"$1\" >\"$1.new\" &&\n-\tmv \"$1.new\" \"$1\"\n-}\n-\n # Switch to the branch in $into and notify it in the reflog\n checkout_onto () {\n \tGIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $onto_name\"\n@@ -982,7 +961,7 @@ fi\n \n test -s \"$todo\" || echo noop >> \"$todo\"\n test -z \"$autosquash\" || git rebase--helper --rearrange-squash || exit\n-test -n \"$cmd\" && add_exec_commands \"$todo\"\n+test -n \"$cmd\" && git rebase--helper --add-exec-commands \"$cmd\"\n \n todocount=$(git stripspace --strip-comments <\"$todo\" | wc -l)\n todocount=${todocount##* }\ndiff --git a/sequencer.c b/sequencer.c\nindex 8b0dd610c881..892d242f6966 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2494,6 +2494,45 @@ int sequencer_make_script(FILE *out, int argc, const char **argv,\n \treturn 0;\n }\n \n+/*\n+ * Add commands after pick and (series of) squash/fixup commands\n+ * in the todo list.\n+ */\n+int sequencer_add_exec_commands(const char *commands)\n+{\n+\tconst char *todo_file = rebase_path_todo();\n+\tstruct todo_list todo_list = TODO_LIST_INIT;\n+\tstruct todo_item *item;\n+\tstruct strbuf *buf = &todo_list.buf;\n+\tsize_t offset = 0, commands_len = strlen(commands);\n+\tint i, first;\n+\n+\tif (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)\n+\t\treturn error(_(\"could not read '%s'.\"), todo_file);\n+\n+\tif (parse_insn_buffer(todo_list.buf.buf, &todo_list)) {\n+\t\ttodo_list_release(&todo_list);\n+\t\treturn error(_(\"unusable todo list: '%s'\"), todo_file);\n+\t}\n+\n+\tfirst = 1;\n+\t/* insert before every pick except the first one */\n+\tfor (item = todo_list.items, i = 0; i < todo_list.nr; i++, item++) {\n+\t\tif (item->command == TODO_PICK && !first) {\n+\t\t\tstrbuf_insert(buf, item->offset_in_buf + offset,\n+\t\t\t\t commands, commands_len);\n+\t\t\toffset += commands_len;\n+\t\t}\n+\t\tfirst = 0;\n+\t}\n+\n+\t/* append final */\n+\tstrbuf_add(buf, commands, commands_len);\n+\n+\ti = write_message(buf->buf, buf->len, todo_file, 0);\n+\ttodo_list_release(&todo_list);\n+\treturn i;\n+}\n \n int transform_todos(unsigned flags)\n {\ndiff --git a/sequencer.h b/sequencer.h\nindex 68284e9762c8..212426c44548 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -50,6 +50,7 @@ int sequencer_remove_state(struct replay_opts *opts);\n int sequencer_make_script(FILE *out, int argc, const char **argv,\n \t\t\t unsigned flags);\n \n+int sequencer_add_exec_commands(const char *command);\n int transform_todos(unsigned flags);\n int check_todo_list(void);\n int skip_unnecessary_picks(void);\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334205","messageId":"20171205175235.32319-9-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171205175235.32319-1-liambeguin@gmail.com","subject":"[PATCH v3 8/9] rebase -i: learn to abbreviate command names","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-05T17:52:34Z","receivedAt":"2017-12-05T17:53:18Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"`git rebase -i` already know how to interpret single-letter command\nnames. Teach it to generate the todo list with these same abbreviated\nnames.\n\nBased-on-patch-by: Johannes Schindelin \nSigned-off-by: Liam Beguin \n---\n Documentation/rebase-config.txt | 20 ++++++++++++++++++++\n builtin/rebase--helper.c | 3 +++\n sequencer.c | 16 ++++++++++++++--\n sequencer.h | 1 +\n 4 files changed, 38 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/rebase-config.txt b/Documentation/rebase-config.txt\nindex 30ae08cb5a4b..42e1ba757564 100644\n--- a/Documentation/rebase-config.txt\n+++ b/Documentation/rebase-config.txt\n@@ -30,3 +30,23 @@ rebase.instructionFormat::\n \tA format string, as specified in linkgit:git-log[1], to be used for the\n \ttodo list during an interactive rebase. The format will\n \tautomatically have the long commit hash prepended to the format.\n+\n+rebase.abbreviateCommands::\n+\tIf set to true, `git rebase` will use abbreviated command names in the\n+\ttodo list resulting in something like this:\n++\n+-------------------------------------------\n+\tp deadbee The oneline of the commit\n+\tp fa1afe1 The oneline of the next commit\n+\t...\n+-------------------------------------------\n++\n+instead of:\n++\n+-------------------------------------------\n+\tpick deadbee The oneline of the commit\n+\tpick fa1afe1 The oneline of the next commit\n+\t...\n+-------------------------------------------\n++\n+Defaults to false.\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex 4229ea0dc122..7daee544b7b4 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -13,6 +13,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n {\n \tstruct replay_opts opts = REPLAY_OPTS_INIT;\n \tunsigned flags = 0, keep_empty = 0;\n+\tint abbreviate_commands = 0;\n \tenum {\n \t\tCONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,\n \t\tCHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,\n@@ -43,6 +44,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t};\n \n \tgit_config(git_default_config, NULL);\n+\tgit_config_get_bool(\"rebase.abbreviatecommands\", &abbreviate_commands);\n \n \topts.action = REPLAY_INTERACTIVE_REBASE;\n \topts.allow_ff = 1;\n@@ -52,6 +54,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\t\tbuiltin_rebase_helper_usage, PARSE_OPT_KEEP_ARGV0);\n \n \tflags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0;\n+\tflags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;\n \tflags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;\n \n \tif (command == CONTINUE && argc == 1)\ndiff --git a/sequencer.c b/sequencer.c\nindex 892d242f6966..115085d39ca8 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -795,6 +795,13 @@ static const char *command_to_string(const enum todo_command command)\n \tdie(\"Unknown command: %d\", command);\n }\n \n+static const char command_to_char(const enum todo_command command)\n+{\n+\tif (command < TODO_COMMENT && todo_command_info[command].c)\n+\t\treturn todo_command_info[command].c;\n+\treturn comment_line_char;\n+}\n+\n static int is_noop(const enum todo_command command)\n {\n \treturn TODO_NOOP <= command;\n@@ -2453,6 +2460,7 @@ int sequencer_make_script(FILE *out, int argc, const char **argv,\n \tstruct rev_info revs;\n \tstruct commit *commit;\n \tint keep_empty = flags & TODO_LIST_KEEP_EMPTY;\n+\tconst char *insn = flags & TODO_LIST_ABBREVIATE_CMDS ? \"p\" : \"pick\";\n \n \tinit_revisions(&revs, NULL);\n \trevs.verbose_header = 1;\n@@ -2485,7 +2493,8 @@ int sequencer_make_script(FILE *out, int argc, const char **argv,\n \t\tstrbuf_reset(&buf);\n \t\tif (!keep_empty && is_original_commit_empty(commit))\n \t\t\tstrbuf_addf(&buf, \"%c \", comment_line_char);\n-\t\tstrbuf_addf(&buf, \"pick %s \", oid_to_hex(&commit->object.oid));\n+\t\tstrbuf_addf(&buf, \"%s %s \", insn,\n+\t\t\t oid_to_hex(&commit->object.oid));\n \t\tpretty_print_commit(&pp, commit, &buf);\n \t\tstrbuf_addch(&buf, '\\n');\n \t\tfputs(buf.buf, out);\n@@ -2558,7 +2567,10 @@ int transform_todos(unsigned flags)\n \t\t}\n \n \t\t/* add command to the buffer */\n-\t\tstrbuf_addstr(&buf, command_to_string(item->command));\n+\t\tif (flags & TODO_LIST_ABBREVIATE_CMDS)\n+\t\t\tstrbuf_addch(&buf, command_to_char(item->command));\n+\t\telse\n+\t\t\tstrbuf_addstr(&buf, command_to_string(item->command));\n \n \t\t/* add commit id */\n \t\tif (item->commit) {\ndiff --git a/sequencer.h b/sequencer.h\nindex 212426c44548..81f6d7d393fd 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -47,6 +47,7 @@ int sequencer_remove_state(struct replay_opts *opts);\n \n #define TODO_LIST_KEEP_EMPTY (1U << 0)\n #define TODO_LIST_SHORTEN_IDS (1U << 1)\n+#define TODO_LIST_ABBREVIATE_CMDS (1U << 2)\n int sequencer_make_script(FILE *out, int argc, const char **argv,\n \t\t\t unsigned flags);\n \n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334206","messageId":"20171205175235.32319-7-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171205175235.32319-1-liambeguin@gmail.com","subject":"[PATCH v3 6/9] rebase -i: update functions to use a flags parameter","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-05T17:52:32Z","receivedAt":"2017-12-05T17:53:24Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Update functions used in the rebase--helper so that they take a generic\n'flags' parameter instead of a growing list of options.\n\nSigned-off-by: Liam Beguin \n---\n builtin/rebase--helper.c | 13 +++++++------\n sequencer.c | 9 +++++----\n sequencer.h | 8 +++++---\n 3 files changed, 17 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex c3b8e4d401f8..1102ecb43b67 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -12,7 +12,7 @@ static const char * const builtin_rebase_helper_usage[] = {\n int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n {\n \tstruct replay_opts opts = REPLAY_OPTS_INIT;\n-\tint keep_empty = 0;\n+\tunsigned flags = 0, keep_empty = 0;\n \tenum {\n \t\tCONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,\n \t\tCHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH\n@@ -48,16 +48,17 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, NULL, options,\n \t\t\tbuiltin_rebase_helper_usage, PARSE_OPT_KEEP_ARGV0);\n \n+\tflags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0;\n+\tflags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;\n+\n \tif (command == CONTINUE && argc == 1)\n \t\treturn !!sequencer_continue(&opts);\n \tif (command == ABORT && argc == 1)\n \t\treturn !!sequencer_remove_state(&opts);\n \tif (command == MAKE_SCRIPT && argc > 1)\n-\t\treturn !!sequencer_make_script(keep_empty, stdout, argc, argv);\n-\tif (command == SHORTEN_OIDS && argc == 1)\n-\t\treturn !!transform_todos(1);\n-\tif (command == EXPAND_OIDS && argc == 1)\n-\t\treturn !!transform_todos(0);\n+\t\treturn !!sequencer_make_script(stdout, argc, argv, flags);\n+\tif ((command == SHORTEN_OIDS || command == EXPAND_OIDS) && argc == 1)\n+\t\treturn !!transform_todos(flags);\n \tif (command == CHECK_TODO_LIST && argc == 1)\n \t\treturn !!check_todo_list();\n \tif (command == SKIP_UNNECESSARY_PICKS && argc == 1)\ndiff --git a/sequencer.c b/sequencer.c\nindex c9a661a8c4bd..8b0dd610c881 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2444,14 +2444,15 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)\n \tstrbuf_release(&sob);\n }\n \n-int sequencer_make_script(int keep_empty, FILE *out,\n-\t\tint argc, const char **argv)\n+int sequencer_make_script(FILE *out, int argc, const char **argv,\n+\t\t\t unsigned flags)\n {\n \tchar *format = NULL;\n \tstruct pretty_print_context pp = {0};\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct rev_info revs;\n \tstruct commit *commit;\n+\tint keep_empty = flags & TODO_LIST_KEEP_EMPTY;\n \n \tinit_revisions(&revs, NULL);\n \trevs.verbose_header = 1;\n@@ -2494,7 +2495,7 @@ int sequencer_make_script(int keep_empty, FILE *out,\n }\n \n \n-int transform_todos(int shorten_ids)\n+int transform_todos(unsigned flags)\n {\n \tconst char *todo_file = rebase_path_todo();\n \tstruct todo_list todo_list = TODO_LIST_INIT;\n@@ -2522,7 +2523,7 @@ int transform_todos(int shorten_ids)\n \n \t\t/* add commit id */\n \t\tif (item->commit) {\n-\t\t\tconst char *oid = shorten_ids ?\n+\t\t\tconst char *oid = flags & TODO_LIST_SHORTEN_IDS ?\n \t\t\t\t\t short_commit_name(item->commit) :\n \t\t\t\t\t oid_to_hex(&item->commit->object.oid);\n \ndiff --git a/sequencer.h b/sequencer.h\nindex 4f7f2c93f83e..68284e9762c8 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -45,10 +45,12 @@ int sequencer_continue(struct replay_opts *opts);\n int sequencer_rollback(struct replay_opts *opts);\n int sequencer_remove_state(struct replay_opts *opts);\n \n-int sequencer_make_script(int keep_empty, FILE *out,\n-\t\tint argc, const char **argv);\n+#define TODO_LIST_KEEP_EMPTY (1U << 0)\n+#define TODO_LIST_SHORTEN_IDS (1U << 1)\n+int sequencer_make_script(FILE *out, int argc, const char **argv,\n+\t\t\t unsigned flags);\n \n-int transform_todos(int shorten_ids);\n+int transform_todos(unsigned flags);\n int check_todo_list(void);\n int skip_unnecessary_picks(void);\n int rearrange_squash(void);\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334207","messageId":"20171205175235.32319-10-liambeguin@gmail.com","threadId":"47327","inReplyTo":"20171205175235.32319-1-liambeguin@gmail.com","subject":"[PATCH v3 9/9] t3404: add test case for abbreviated commands","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-05T17:52:35Z","receivedAt":"2017-12-05T17:53:27Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Make sure the todo list ends up using single-letter command\nabbreviations when the rebase.abbreviateCommands is enabled.\nThis configuration option should not change anything else.\n\nSigned-off-by: Liam Beguin \n---\n t/t3404-rebase-interactive.sh | 22 ++++++++++++++++++++++\n 1 file changed, 22 insertions(+)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 6a82d1ed876d..481a3500900d 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1260,6 +1260,28 @@ test_expect_success 'rebase -i respects rebase.missingCommitsCheck = error' '\n \ttest B = $(git cat-file commit HEAD^ | sed -ne \\$p)\n '\n \n+test_expect_success 'respects rebase.abbreviateCommands with fixup, squash and exec' '\n+\trebase_setup_and_clean abbrevcmd &&\n+\ttest_commit \"first\" file1.txt \"first line\" first &&\n+\ttest_commit \"second\" file1.txt \"another line\" second &&\n+\ttest_commit \"fixup! first\" file2.txt \"first line again\" first_fixup &&\n+\ttest_commit \"squash! second\" file1.txt \"another line here\" second_squash &&\n+\tcat >expected <<-EOF &&\n+\tp $(git rev-list --abbrev-commit -1 first) first\n+\tf $(git rev-list --abbrev-commit -1 first_fixup) fixup! first\n+\tx git show HEAD\n+\tp $(git rev-list --abbrev-commit -1 second) second\n+\ts $(git rev-list --abbrev-commit -1 second_squash) squash! second\n+\tx git show HEAD\n+\tEOF\n+\tgit checkout abbrevcmd &&\n+\tset_cat_todo_editor &&\n+\ttest_config rebase.abbreviateCommands true &&\n+\ttest_must_fail git rebase -i --exec \"git show HEAD\" \\\n+\t\t--autosquash master >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'static check of bad command' '\n \trebase_setup_and_clean bad-cmd &&\n \tset_fake_editor &&\n-- \n2.15.1.280.g10402c1f5b5c\n\n"},{"id":"334227","messageId":"xmqq374oizov.fsf@gitster.mtv.corp.google.com","threadId":"47327","inReplyTo":"20171205175235.32319-1-liambeguin@gmail.com","subject":"Re: [PATCH v2 0/9] rebase -i: add config to abbreviate command names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-05T22:21:04Z","receivedAt":"2017-12-05T22:21:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Liam Beguin writes:\n\n> This series will add the 'rebase.abbreviateCommands' configuration\n> option to allow `git rebase -i` to default to the single-letter command\n> names when generating the todo list.\n>\n> Using single-letter command names can present two benefits. First, it\n> makes it easier to change the action since you only need to replace a\n> single character (i.e.: in vim \"r\" instead of\n> \"ciw\"). Second, using this with a large enough value of\n> 'core.abbrev' enables the lines of the todo list to remain aligned\n> making the files easier to read.\n>\n> Changes in V2:\n> - Refactor and rename 'transform_todo_ids'\n> - Replace SHA-1 by OID in rebase--helper.c\n> - Update todo list related functions to take a generic 'flags' parameter\n> - Rename 'add_exec_commands' function to 'sequencer_add_exec_commands'\n> - Rename 'add-exec' option to 'add-exec-commands'\n> - Use 'strbur_read_file' instead of rewriting it\n> - Make 'command_to_char' return 'comment_char_line' if no single-letter\n> command name is defined\n> - Combine both tests into a single test case\n> - Update commit messages\n>\n> Changes in V2:\n> - Rename 'transform_todo_insn' to 'transform_todos'\n> - Fix flag name TODO_LIST_SHORTE{D,N}_IDS\n\nI've replaced this series and pushed out the result.\n\nThanks.\n"},{"id":"334240","messageId":"b240d881-8729-ff04-7d42-1a101d063ec6@gmail.com","threadId":"47327","inReplyTo":"xmqq374oizov.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 0/9] rebase -i: add config to abbreviate command names","fromName":"liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-06T02:42:52Z","receivedAt":"2017-12-06T02:43:01Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"\n\nOn 05/12/17 05:21 PM, Junio C Hamano wrote:\n> Liam Beguin writes:\n> \n>> This series will add the 'rebase.abbreviateCommands' configuration\n>> option to allow `git rebase -i` to default to the single-letter command\n>> names when generating the todo list.\n>>\n>> Using single-letter command names can present two benefits. First, it\n>> makes it easier to change the action since you only need to replace a\n>> single character (i.e.: in vim \"r\" instead of\n>> \"ciw\"). Second, using this with a large enough value of\n>> 'core.abbrev' enables the lines of the todo list to remain aligned\n>> making the files easier to read.\n>>\n>> Changes in V2:\n>> - Refactor and rename 'transform_todo_ids'\n>> - Replace SHA-1 by OID in rebase--helper.c\n>> - Update todo list related functions to take a generic 'flags' parameter\n>> - Rename 'add_exec_commands' function to 'sequencer_add_exec_commands'\n>> - Rename 'add-exec' option to 'add-exec-commands'\n>> - Use 'strbur_read_file' instead of rewriting it\n>> - Make 'command_to_char' return 'comment_char_line' if no single-letter\n>> command name is defined\n>> - Combine both tests into a single test case\n>> - Update commit messages\n>>\n>> Changes in V2:\n>> - Rename 'transform_todo_insn' to 'transform_todos'\n>> - Fix flag name TODO_LIST_SHORTE{D,N}_IDS\n> \n> I've replaced this series and pushed out the result.\n\nGreat! Thanks again,\n\n> \n> Thanks.\n> \n\nLiam\n"},{"id":"335284","messageId":"CACsJy8B3U0_sJeEt+gLy9HJKszO5-uRZsssL3ZFdkKbSM9yWDg@mail.gmail.com","threadId":"47327","inReplyTo":"20171203221721.16462-9-liambeguin@gmail.com","subject":"Re: [PATCH v2 8/9] rebase -i: learn to abbreviate command names","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-25T12:48:00Z","receivedAt":"2017-12-25T12:48:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Dec 4, 2017 at 5:17 AM, Liam Beguin wrote:\n> +static const char command_to_char(const enum todo_command command)\n> +{\n> + if (command < TODO_COMMENT && todo_command_info[command].c)\n> + return todo_command_info[command].c;\n> + return comment_line_char;\n> +}\n\n CC sequencer.o\nsequencer.c:798:19: error: type qualifiers ignored on function return\ntype [-Werror=ignored-qualifiers]\n static const char command_to_char(const enum todo_command command)\n ^\n\nMaybe drop the first const.\n-- \nDuy\n"},{"id":"335285","messageId":"CAKm4OoWgB7tz0HJorDzL9Xy+fX0LVE1eOVngrfMTMDQQUTDk4Q@mail.gmail.com","threadId":"47327","inReplyTo":"CACsJy8B3U0_sJeEt+gLy9HJKszO5-uRZsssL3ZFdkKbSM9yWDg@mail.gmail.com","subject":"Re: [PATCH v2 8/9] rebase -i: learn to abbreviate command names","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-25T15:39:54Z","receivedAt":"2017-12-25T15:40:21Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Hi Duy,\n\nOn Mon, 25 Dec 2017 at 07:48 Duy Nguyen wrote:\n>\n> On Mon, Dec 4, 2017 at 5:17 AM, Liam Beguin wrote:\n> > +static const char command_to_char(const enum todo_command command)\n> > +{\n> > + if (command < TODO_COMMENT && todo_command_info[command].c)\n> > + return todo_command_info[command].c;\n> > + return comment_line_char;\n> > +}\n>\n> CC sequencer.o\n> sequencer.c:798:19: error: type qualifiers ignored on function return\n> type [-Werror=ignored-qualifiers]\n> static const char command_to_char(const enum todo_command command)\n> ^\n>\n> Maybe drop the first const.\n\nSorry, that's another copy-edit error I made that slipped through...\nI'm curious, how did you build to get this error to show?\nI tried with the DEVELOPER 'flag' but nothing showed and -Wextra gave\nway too much messages...\nDid you just add -Wignored-qualifiers to CFLAGS?\n\n> --\n> Duy\n\nThanks,\nLiam\n"},{"id":"335304","messageId":"CACsJy8CS8Zub=XyzU5VJtVZqcttN0v8TKShbH6cgWxN03y33ew@mail.gmail.com","threadId":"47327","inReplyTo":"CAKm4OoWgB7tz0HJorDzL9Xy+fX0LVE1eOVngrfMTMDQQUTDk4Q@mail.gmail.com","subject":"Re: [PATCH v2 8/9] rebase -i: learn to abbreviate command names","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-25T23:58:03Z","receivedAt":"2017-12-26T00:06:37Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Dec 25, 2017 at 10:39 PM, Liam Beguin wrote:\n> I'm curious, how did you build to get this error to show?\n> I tried with the DEVELOPER 'flag' but nothing showed and -Wextra gave\n> way too much messages...\n> Did you just add -Wignored-qualifiers to CFLAGS?\n\nI have a custom CFLAGS, created before DEVELOPER flag was added, which\nis -Wextra -Werror plus about 5 -Wno-xxx to shut gcc up.\n-- \nDuy\n"},{"id":"335385","messageId":"xmqqmv24m186.fsf@gitster.mtv.corp.google.com","threadId":"47327","inReplyTo":"CACsJy8B3U0_sJeEt+gLy9HJKszO5-uRZsssL3ZFdkKbSM9yWDg@mail.gmail.com","subject":"Re: [PATCH v2 8/9] rebase -i: learn to abbreviate command names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-27T19:15:21Z","receivedAt":"2017-12-27T19:15:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Mon, Dec 4, 2017 at 5:17 AM, Liam Beguin wrote:\n>> +static const char command_to_char(const enum todo_command command)\n>> +{\n>> + if (command < TODO_COMMENT && todo_command_info[command].c)\n>> + return todo_command_info[command].c;\n>> + return comment_line_char;\n>> +}\n>\n> CC sequencer.o\n> sequencer.c:798:19: error: type qualifiers ignored on function return\n> type [-Werror=ignored-qualifiers]\n> static const char command_to_char(const enum todo_command command)\n> ^\n>\n> Maybe drop the first const.\n\nThanks. This topic has been in 'next' for quite some time and I\nwanted to merge it down to 'master' soonish, so I've added the\nfollowing before doing so.\n\n-- >8 --\nFrom: Junio C Hamano \nDate: Wed, 27 Dec 2017 11:12:45 -0800\nSubject: [PATCH] sequencer.c: drop 'const' from function return type\n\nWith -Werror=ignored-qualifiers, a function that claims to return\n\"const char\" gets this error:\n\n CC sequencer.o\nsequencer.c:798:19: error: type qualifiers ignored on function return\ntype [-Werror=ignored-qualifiers]\n static const char command_to_char(const enum todo_command command)\n ^\n\nReported-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: Junio C Hamano \n---\n sequencer.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 115085d39c..2a407cbe54 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -795,7 +795,7 @@ static const char *command_to_string(const enum todo_command command)\n \tdie(\"Unknown command: %d\", command);\n }\n \n-static const char command_to_char(const enum todo_command command)\n+static char command_to_char(const enum todo_command command)\n {\n \tif (command < TODO_COMMENT && todo_command_info[command].c)\n \t\treturn todo_command_info[command].c;\n-- \n2.15.1-597-g62d91a8972\n\n"},{"id":"335397","messageId":"CAKm4OoVMRd-ZkaB9Z8Mxnavfy077=LifJq9OEeg-mvjEGz4K6A@mail.gmail.com","threadId":"47327","inReplyTo":"xmqqmv24m186.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 8/9] rebase -i: learn to abbreviate command names","fromName":"Liam Beguin","fromEmail":"liambeguin@gmail.com","sentAt":"2017-12-27T21:58:19Z","receivedAt":"2017-12-27T21:58:46Z","isPatch":true,"sender":{"key":"liambeguin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3811160?v=4"},"body":"Hi Junio,\n\n\nOn 27 December 2017 at 20:15, Junio C Hamano wrote:\n> Duy Nguyen writes:\n>\n>> On Mon, Dec 4, 2017 at 5:17 AM, Liam Beguin wrote:\n>>> +static const char command_to_char(const enum todo_command command)\n>>> +{\n>>> + if (command < TODO_COMMENT && todo_command_info[command].c)\n>>> + return todo_command_info[command].c;\n>>> + return comment_line_char;\n>>> +}\n>>\n>> CC sequencer.o\n>> sequencer.c:798:19: error: type qualifiers ignored on function return\n>> type [-Werror=ignored-qualifiers]\n>> static const char command_to_char(const enum todo_command command)\n>> ^\n>>\n>> Maybe drop the first const.\n>\n> Thanks. This topic has been in 'next' for quite some time and I\n> wanted to merge it down to 'master' soonish, so I've added the\n> following before doing so.\n\nThanks for taking the time. I had prepared the patch but was waiting to get\nhome to send it.\nOnly comment I have, maybe s/sequencer.c/rebase -i/ in the subject line\nso it matches with the rest.\n\nSince this came up, would it be a good thing to add -Wignored-qualifiers\nto the DEVELOPER flags?\n\n>\n> -- >8 --\n> From: Junio C Hamano \n> Date: Wed, 27 Dec 2017 11:12:45 -0800\n> Subject: [PATCH] sequencer.c: drop 'const' from function return type\n>\n> With -Werror=ignored-qualifiers, a function that claims to return\n> \"const char\" gets this error:\n>\n> CC sequencer.o\n> sequencer.c:798:19: error: type qualifiers ignored on function return\n> type [-Werror=ignored-qualifiers]\n> static const char command_to_char(const enum todo_command command)\n> ^\n>\n> Reported-by: Nguyễn Thái Ngọc Duy \n> Signed-off-by: Junio C Hamano \n> ---\n> sequencer.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index 115085d39c..2a407cbe54 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -795,7 +795,7 @@ static const char *command_to_string(const enum todo_command command)\n> die(\"Unknown command: %d\", command);\n> }\n>\n> -static const char command_to_char(const enum todo_command command)\n> +static char command_to_char(const enum todo_command command)\n> {\n> if (command < TODO_COMMENT && todo_command_info[command].c)\n> return todo_command_info[command].c;\n> --\n> 2.15.1-597-g62d91a8972\n>\n\nThanks,\nLiam\n"},{"id":"335474","messageId":"xmqqpo6ylm27.fsf@gitster.mtv.corp.google.com","threadId":"47327","inReplyTo":"CAKm4OoVMRd-ZkaB9Z8Mxnavfy077=LifJq9OEeg-mvjEGz4K6A@mail.gmail.com","subject":"Re: [PATCH v2 8/9] rebase -i: learn to abbreviate command names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-28T18:55:12Z","receivedAt":"2017-12-28T18:55:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Liam Beguin writes:\n\n> Since this came up, would it be a good thing to add -Wignored-qualifiers\n> to the DEVELOPER flags?\n\nQuite frankly, I am not sure if catching that particular warning\nviolation buys us much. As a return value from a function is never\nan lvalue, what triggers the warning may certainly be an indication\nof a sloppy coding, but otherwise I do not see it as diagnosing a\npotential error. \"The programmer thought that the returned value\nwill only be assigned to a const variable and will never be\nmodified, but the language does not guarantee such a behaviour out\nof the caller\"---does such an incorrect expectation lead to an error\nin the codepath that involves such a function?\n"}]}