{"thread":{"id":"48688","subject":"[GSoC][PATCH 0/1] rebase -i: rewrite the edit-todo functionality in C","startedAt":"2018-06-11T13:58:17Z","lastAt":"2018-06-26T21:47:30Z","messageCount":19,"participants":["Alban Gruin","Phillip Wood","Elijah Newren","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"349894","messageId":"20180611135714.29378-1-alban.gruin@gmail.com","threadId":"48688","inReplyTo":null,"subject":"[GSoC][PATCH 0/1] rebase -i: rewrite the edit-todo functionality in C","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2018-06-11T13:57:13Z","receivedAt":"2018-06-11T13:58:17Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"This patch rewrites the edit-todo functionality from shell to C. This is\npart of the effort to rewrite interactive rebase in C.\n\nAlban Gruin (1):\n  rebase--interactive: rewrite the edit-todo functionality in C\n\n builtin/rebase--helper.c   | 13 ++++++++-----\n git-rebase--interactive.sh | 11 +----------\n sequencer.c                | 31 +++++++++++++++++++++++++++++++\n sequencer.h                |  1 +\n 4 files changed, 41 insertions(+), 15 deletions(-)\n\n-- \n2.16.4\n\n"},{"id":"349895","messageId":"20180611135714.29378-2-alban.gruin@gmail.com","threadId":"48688","inReplyTo":"20180611135714.29378-1-alban.gruin@gmail.com","subject":"[GSoC][PATCH 1/1] rebase--interactive: rewrite the edit-todo functionality in C","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2018-06-11T13:57:14Z","receivedAt":"2018-06-11T13:58:23Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"This rewrites the edit-todo functionality from shell to C.\n\nTo achieve that, a new command mode, `edit-todo`, is added, and the\n`write-edit-todo` flag is removed, as the shell script does not need to\nwrite the edit todo help message to the todo list anymore.\n\nThe shell version is then stripped in favour of a call to the helper.\n\nSigned-off-by: Alban Gruin <alban.gruin@gmail.com>\n---\n builtin/rebase--helper.c   | 13 ++++++++-----\n git-rebase--interactive.sh | 11 +----------\n sequencer.c                | 31 +++++++++++++++++++++++++++++++\n sequencer.h                |  1 +\n 4 files changed, 41 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex ded5e291d..d2990b210 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -12,12 +12,12 @@ 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-\tunsigned flags = 0, keep_empty = 0, rebase_merges = 0, write_edit_todo = 0;\n+\tunsigned flags = 0, keep_empty = 0, rebase_merges = 0;\n \tint abbreviate_commands = 0, rebase_cousins = -1;\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\tADD_EXEC, APPEND_TODO_HELP\n+\t\tADD_EXEC, APPEND_TODO_HELP, EDIT_TODO\n \t} command = 0;\n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"ff\", &opts.allow_ff, N_(\"allow fast-forward\")),\n@@ -27,8 +27,6 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"rebase-merges\", &rebase_merges, N_(\"rebase merge commits\")),\n \t\tOPT_BOOL(0, \"rebase-cousins\", &rebase_cousins,\n \t\t\t N_(\"keep original branch points of cousins\")),\n-\t\tOPT_BOOL(0, \"write-edit-todo\", &write_edit_todo,\n-\t\t\t N_(\"append the edit-todo message to the todo-list\")),\n \t\tOPT_CMDMODE(0, \"continue\", &command, N_(\"continue rebase\"),\n \t\t\t\tCONTINUE),\n \t\tOPT_CMDMODE(0, \"abort\", &command, N_(\"abort rebase\"),\n@@ -49,6 +47,9 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\t\tN_(\"insert exec commands in todo list\"), ADD_EXEC),\n \t\tOPT_CMDMODE(0, \"append-todo-help\", &command,\n \t\t\t    N_(\"insert the help in the todo list\"), APPEND_TODO_HELP),\n+\t\tOPT_CMDMODE(0, \"edit-todo\", &command,\n+\t\t\t    N_(\"edit the todo list during an interactive rebase\"),\n+\t\t\t    EDIT_TODO),\n \t\tOPT_END()\n \t};\n \n@@ -89,6 +90,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \tif (command == ADD_EXEC && argc == 2)\n \t\treturn !!sequencer_add_exec_commands(argv[1]);\n \tif (command == APPEND_TODO_HELP && argc == 1)\n-\t\treturn !!append_todo_help(write_edit_todo, keep_empty);\n+\t\treturn !!append_todo_help(0, keep_empty);\n+\tif (command == EDIT_TODO && argc == 1)\n+\t\treturn !!edit_todo_list(flags);\n \tusage_with_options(builtin_rebase_helper_usage, options);\n }\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 94c23a7af..2defe607f 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -108,16 +108,7 @@ initiate_action () {\n \t\t     --continue\n \t\t;;\n \tedit-todo)\n-\t\tgit stripspace --strip-comments <\"$todo\" >\"$todo\".new\n-\t\tmv -f \"$todo\".new \"$todo\"\n-\t\tcollapse_todo_ids\n-\t\tgit rebase--helper --append-todo-help --write-edit-todo\n-\n-\t\tgit_sequence_editor \"$todo\" ||\n-\t\t\tdie \"$(gettext \"Could not execute editor\")\"\n-\t\texpand_todo_ids\n-\n-\t\texit\n+\t\texec git rebase--helper --edit-todo\n \t\t;;\n \tshow-current-patch)\n \t\texec git show REBASE_HEAD --\ndiff --git a/sequencer.c b/sequencer.c\nindex 1ffd990f7..1c1799c91 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -4386,6 +4386,37 @@ int append_todo_help(unsigned edit_todo, unsigned keep_empty)\n \treturn ret;\n }\n \n+int edit_todo_list(unsigned flags)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tconst char *todo_file = rebase_path_todo();\n+\tFILE *todo;\n+\n+\tif (strbuf_read_file(&buf, todo_file, 0) < 0)\n+\t\treturn error_errno(_(\"could not read '%s'.\"), todo_file);\n+\n+\tstrbuf_stripspace(&buf, 1);\n+\ttodo = fopen_or_warn(todo_file, \"w\");\n+\tif (!todo) {\n+\t\tstrbuf_release(&buf);\n+\t\treturn 1;\n+\t}\n+\n+\tstrbuf_write(&buf, todo);\n+\tfclose(todo);\n+\tstrbuf_release(&buf);\n+\n+\ttransform_todos(flags | TODO_LIST_SHORTEN_IDS);\n+\tappend_todo_help(1, 0);\n+\n+\tif (launch_editor(todo_file, NULL, NULL))\n+\t\treturn 1;\n+\n+\ttransform_todos(flags & ~(TODO_LIST_SHORTEN_IDS));\n+\n+\treturn 0;\n+}\n+\n static int rewrite_file(const char *path, const char *buf, size_t len)\n {\n \tint rc = 0;\ndiff --git a/sequencer.h b/sequencer.h\nindex e14f6590e..35730b13e 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -81,6 +81,7 @@ int sequencer_add_exec_commands(const char *command);\n int transform_todos(unsigned flags);\n int check_todo_list(void);\n int append_todo_help(unsigned edit_todo, unsigned keep_empty);\n+int edit_todo_list(unsigned flags);\n int skip_unnecessary_picks(void);\n int rearrange_squash(void);\n \n-- \n2.16.4\n\n"},{"id":"349902","messageId":"3bfd3470-4482-fe6a-2cd9-08311a0bbaac@talktalk.net","threadId":"48688","inReplyTo":"20180611135714.29378-2-alban.gruin@gmail.com","subject":"Re: [GSoC][PATCH 1/1] rebase--interactive: rewrite the edit-todo functionality in C","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-06-11T15:32:50Z","receivedAt":"2018-06-11T15:32:57Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Alban, it's great to see you making progress with this.\n\nI don't want to add to your workload but a couple of things that might \nbe nice to have in the future would be\n\n1) avoid rewriting the todo list and running the editor if \nGIT_SEQUENCE_EDITOR is ':', especially when creating the todo list for \nimplicit interactive rebases.\n\n2) have --edit-todo warn if the user drops commits, in the same way as \nrebase does for the initial edit of the todo list. This should also make \nit easier to use the same code for the initial edit as well as when the \nuser does 'rebase --edit-todo'\n\nI've got a couple of comments on this patch below.\n\nOn 11/06/18 14:57, Alban Gruin wrote:\n> This rewrites the edit-todo functionality from shell to C.\n> \n> To achieve that, a new command mode, `edit-todo`, is added, and the\n> `write-edit-todo` flag is removed, as the shell script does not need to\n> write the edit todo help message to the todo list anymore.\n> \n> The shell version is then stripped in favour of a call to the helper.\n> \n> Signed-off-by: Alban Gruin <alban.gruin@gmail.com>\n> ---\n>   builtin/rebase--helper.c   | 13 ++++++++-----\n>   git-rebase--interactive.sh | 11 +----------\n>   sequencer.c                | 31 +++++++++++++++++++++++++++++++\n>   sequencer.h                |  1 +\n>   4 files changed, 41 insertions(+), 15 deletions(-)\n> \n> diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\n> index ded5e291d..d2990b210 100644\n> --- a/builtin/rebase--helper.c\n> +++ b/builtin/rebase--helper.c\n> @@ -12,12 +12,12 @@ 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> -\tunsigned flags = 0, keep_empty = 0, rebase_merges = 0, write_edit_todo = 0;\n> +\tunsigned flags = 0, keep_empty = 0, rebase_merges = 0;\n>   \tint abbreviate_commands = 0, rebase_cousins = -1;\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\tADD_EXEC, APPEND_TODO_HELP\n> +\t\tADD_EXEC, APPEND_TODO_HELP, EDIT_TODO\n>   \t} command = 0;\n>   \tstruct option options[] = {\n>   \t\tOPT_BOOL(0, \"ff\", &opts.allow_ff, N_(\"allow fast-forward\")),\n> @@ -27,8 +27,6 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n>   \t\tOPT_BOOL(0, \"rebase-merges\", &rebase_merges, N_(\"rebase merge commits\")),\n>   \t\tOPT_BOOL(0, \"rebase-cousins\", &rebase_cousins,\n>   \t\t\t N_(\"keep original branch points of cousins\")),\n> -\t\tOPT_BOOL(0, \"write-edit-todo\", &write_edit_todo,\n> -\t\t\t N_(\"append the edit-todo message to the todo-list\")),\n>   \t\tOPT_CMDMODE(0, \"continue\", &command, N_(\"continue rebase\"),\n>   \t\t\t\tCONTINUE),\n>   \t\tOPT_CMDMODE(0, \"abort\", &command, N_(\"abort rebase\"),\n> @@ -49,6 +47,9 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n>   \t\t\tN_(\"insert exec commands in todo list\"), ADD_EXEC),\n>   \t\tOPT_CMDMODE(0, \"append-todo-help\", &command,\n>   \t\t\t    N_(\"insert the help in the todo list\"), APPEND_TODO_HELP),\n> +\t\tOPT_CMDMODE(0, \"edit-todo\", &command,\n> +\t\t\t    N_(\"edit the todo list during an interactive rebase\"),\n> +\t\t\t    EDIT_TODO),\n>   \t\tOPT_END()\n>   \t};\n>   \n> @@ -89,6 +90,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n>   \tif (command == ADD_EXEC && argc == 2)\n>   \t\treturn !!sequencer_add_exec_commands(argv[1]);\n>   \tif (command == APPEND_TODO_HELP && argc == 1)\n> -\t\treturn !!append_todo_help(write_edit_todo, keep_empty);\n> +\t\treturn !!append_todo_help(0, keep_empty);\n> +\tif (command == EDIT_TODO && argc == 1)\n> +\t\treturn !!edit_todo_list(flags);\n>   \tusage_with_options(builtin_rebase_helper_usage, options);\n>   }\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index 94c23a7af..2defe607f 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -108,16 +108,7 @@ initiate_action () {\n>   \t\t     --continue\n>   \t\t;;\n>   \tedit-todo)\n> -\t\tgit stripspace --strip-comments <\"$todo\" >\"$todo\".new\n> -\t\tmv -f \"$todo\".new \"$todo\"\n> -\t\tcollapse_todo_ids\n> -\t\tgit rebase--helper --append-todo-help --write-edit-todo\n> -\n> -\t\tgit_sequence_editor \"$todo\" ||\n> -\t\t\tdie \"$(gettext \"Could not execute editor\")\"\n> -\t\texpand_todo_ids\n> -\n> -\t\texit\n> +\t\texec git rebase--helper --edit-todo\n>   \t\t;;\n>   \tshow-current-patch)\n>   \t\texec git show REBASE_HEAD --\n> diff --git a/sequencer.c b/sequencer.c\n> index 1ffd990f7..1c1799c91 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -4386,6 +4386,37 @@ int append_todo_help(unsigned edit_todo, unsigned keep_empty)\n>   \treturn ret;\n>   }\n>   \n> +int edit_todo_list(unsigned flags)\n> +{\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\tconst char *todo_file = rebase_path_todo();\n> +\tFILE *todo;\n> +\n> +\tif (strbuf_read_file(&buf, todo_file, 0) < 0)\n> +\t\treturn error_errno(_(\"could not read '%s'.\"), todo_file);\n> +\n> +\tstrbuf_stripspace(&buf, 1);\n> +\ttodo = fopen_or_warn(todo_file, \"w\");\n> +\tif (!todo) {\n> +\t\tstrbuf_release(&buf);\n> +\t\treturn 1;\n> +\t}\n> +\n> +\tstrbuf_write(&buf, todo);\n> +\tfclose(todo);\n> +\tstrbuf_release(&buf);\n> +\n> +\ttransform_todos(flags | TODO_LIST_SHORTEN_IDS);\n> +\tappend_todo_help(1, 0);\n> +\n> +\tif (launch_editor(todo_file, NULL, NULL))\n\nI'm not sure that this will respect GIT_SEQUENCE_EDITOR, it would be \nnice to have a launch_sequence_editor() function that shared as much \ncode as possible with launch_editor()\n\n> +\t\treturn 1;\n> +\n> +\ttransform_todos(flags & ~(TODO_LIST_SHORTEN_IDS));\n> +\n> +\treturn 0;\n> +}\n> +\n>   static int rewrite_file(const char *path, const char *buf, size_t len)\n>   {\n>   \tint rc = 0;\n> diff --git a/sequencer.h b/sequencer.h\n> index e14f6590e..35730b13e 100644\n> --- a/sequencer.h\n> +++ b/sequencer.h\n> @@ -81,6 +81,7 @@ int sequencer_add_exec_commands(const char *command);\n>   int transform_todos(unsigned flags);\n>   int check_todo_list(void);\n>   int append_todo_help(unsigned edit_todo, unsigned keep_empty);\n\nCan this declaration be removed now?\n\n> +int edit_todo_list(unsigned flags);\n>   int skip_unnecessary_picks(void);\n>   int rearrange_squash(void);\n\nBest Wishes\n\nPhillip\n\n"},{"id":"349982","messageId":"801da146-6905-5f7b-12c0-7239108b6207@gmail.com","threadId":"48688","inReplyTo":"3bfd3470-4482-fe6a-2cd9-08311a0bbaac@talktalk.net","subject":"Re: [GSoC][PATCH 1/1] rebase--interactive: rewrite the edit-todo functionality in C","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2018-06-12T12:33:02Z","receivedAt":"2018-06-12T12:33:11Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"Hi Phillip,\n\nLe 11/06/2018 à 17:32, Phillip Wood a écrit :\n>> +    if (launch_editor(todo_file, NULL, NULL))\n> \n> I'm not sure that this will respect GIT_SEQUENCE_EDITOR, it would be\n> nice to have a launch_sequence_editor() function that shared as much\n> code as possible with launch_editor()\n> \n\nIt could be done by making launch_editor() and launch_sequence_editor()\nsome kind of wrapper around a function like launch_specified_editor()\n(or something like that), that would take the editor as a parameter, in\naddition to the path, the buffer and environment variables.  It would be\nalso very trivial to implement your first point above on top of that.\n\n>>   int append_todo_help(unsigned edit_todo, unsigned keep_empty);\n> \n> Can this declaration be removed now?\n\nNo, it’s still used in rebase--helper.c for now.\n\n> \n>> +int edit_todo_list(unsigned flags);\n>>   int skip_unnecessary_picks(void);\n>>   int rearrange_squash(void);\n> \n> Best Wishes\n> \n> Phillip\n> \n\nCheers,\nAlban\n\n"},{"id":"349984","messageId":"CABPp-BHG41ePN1jRxADfb47FnXfKgJ++Jv=DUiTPwpUJUi=C=w@mail.gmail.com","threadId":"48688","inReplyTo":"20180611135714.29378-2-alban.gruin@gmail.com","subject":"Re: [GSoC][PATCH 1/1] rebase--interactive: rewrite the edit-todo functionality in C","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-06-12T15:46:49Z","receivedAt":"2018-06-12T15:46:54Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Alban,\n\nOn Mon, Jun 11, 2018 at 6:57 AM, Alban Gruin <alban.gruin@gmail.com> wrote:\n> This rewrites the edit-todo functionality from shell to C.\n>\n> To achieve that, a new command mode, `edit-todo`, is added, and the\n> `write-edit-todo` flag is removed, as the shell script does not need to\n> write the edit todo help message to the todo list anymore.\n>\n> The shell version is then stripped in favour of a call to the helper.\n\nI looked over the patch and didn't see any problems (though I haven't\nworked with rebase--helper before, or the code for todo list editing),\nbut when I went to apply the patch it failed telling me:\n\nApplying: rebase--interactive: rewrite the edit-todo functionality in C\nerror: sha1 information is lacking or useless (builtin/rebase--helper.c).\nerror: could not build fake ancestor\nPatch failed at 0001 rebase--interactive: rewrite the edit-todo\nfunctionality in C\nUse 'git am --show-current-patch' to see the failed patch\n\nI tried each of master, next, and pu (as of today) to see if it might\napply there.  On which commit is this patch based?  (Do you have other\nlocal commits that this was based on top of?)\n\n\nElijah\n"},{"id":"349986","messageId":"599bb1fd-3a09-8c2d-d319-dbed95012895@gmail.com","threadId":"48688","inReplyTo":"CABPp-BHG41ePN1jRxADfb47FnXfKgJ++Jv=DUiTPwpUJUi=C=w@mail.gmail.com","subject":"Re: [GSoC][PATCH 1/1] rebase--interactive: rewrite the edit-todo functionality in C","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2018-06-12T16:11:45Z","receivedAt":"2018-06-12T16:11:52Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"Hi Elijah,\n\nLe 12/06/2018 à 17:46, Elijah Newren a écrit :\n> Hi Alban,\n> \n> On Mon, Jun 11, 2018 at 6:57 AM, Alban Gruin <alban.gruin@gmail.com> wrote:\n>> This rewrites the edit-todo functionality from shell to C.\n>>\n>> To achieve that, a new command mode, `edit-todo`, is added, and the\n>> `write-edit-todo` flag is removed, as the shell script does not need to\n>> write the edit todo help message to the todo list anymore.\n>>\n>> The shell version is then stripped in favour of a call to the helper.\n> \n> I looked over the patch and didn't see any problems (though I haven't\n> worked with rebase--helper before, or the code for todo list editing),\n> but when I went to apply the patch it failed telling me:\n> \n> Applying: rebase--interactive: rewrite the edit-todo functionality in C\n> error: sha1 information is lacking or useless (builtin/rebase--helper.c).\n> error: could not build fake ancestor\n> Patch failed at 0001 rebase--interactive: rewrite the edit-todo\n> functionality in C\n> Use 'git am --show-current-patch' to see the failed patch\n> \n> I tried each of master, next, and pu (as of today) to see if it might\n> apply there.  On which commit is this patch based?  (Do you have other\n> local commits that this was based on top of?)\n> \n> \n> Elijah\n> \n\nIt can be applied on top of pu with my patch that rewrites\nappend_todo_help() in C\n(https://public-inbox.org/git/20180607103012.22981-1-alban.gruin@gmail.com/)\n\nCheers,\nAlban\n\n"},{"id":"349987","messageId":"7b9134a2-8e23-e089-0fd3-0e67f384eba0@talktalk.net","threadId":"48688","inReplyTo":"801da146-6905-5f7b-12c0-7239108b6207@gmail.com","subject":"Re: [GSoC][PATCH 1/1] rebase--interactive: rewrite the edit-todo functionality in C","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-06-12T16:20:50Z","receivedAt":"2018-06-12T16:20:59Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Alban\n\nOn 12/06/18 13:33, Alban Gruin wrote:\n> Hi Phillip,\n> \n> Le 11/06/2018 à 17:32, Phillip Wood a écrit :\n>>> +    if (launch_editor(todo_file, NULL, NULL))\n>>\n>> I'm not sure that this will respect GIT_SEQUENCE_EDITOR, it would be\n>> nice to have a launch_sequence_editor() function that shared as much\n>> code as possible with launch_editor()\n>>\n> \n> It could be done by making launch_editor() and launch_sequence_editor()\n> some kind of wrapper around a function like launch_specified_editor()\n> (or something like that), that would take the editor as a parameter, in\n> addition to the path, the buffer and environment variables.  It would be\n> also very trivial to implement your first point above on top of that.\n\nThat sounds like a good way forward, launch_sequence_editor() could just \ncall launch_editor() if GIT_SEQUENCE_EDITOR and sequence.editor are not set.\n\n>>>    int append_todo_help(unsigned edit_todo, unsigned keep_empty);\n>>\n>> Can this declaration be removed now?\n> \n> No, it’s still used in rebase--helper.c for now.\n\nAh, sorry I missed that\n\nBest Wishes\n\nPhillip\n\n> \n>>\n>>> +int edit_todo_list(unsigned flags);\n>>>    int skip_unnecessary_picks(void);\n>>>    int rearrange_squash(void);\n>>\n>> Best Wishes\n>>\n>> Phillip\n>>\n> \n> Cheers,\n> Alban\n> \n\n"},{"id":"350059","messageId":"20180613152211.12580-1-alban.gruin@gmail.com","threadId":"48688","inReplyTo":"20180611135714.29378-1-alban.gruin@gmail.com","subject":"[GSoC][PATCH v2 0/2] rebase -i: rewrite the edit-todo functionality in C","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2018-06-13T15:22:09Z","receivedAt":"2018-06-13T15:23:20Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"This patch rewrites the edit-todo functionality from shell to C. This is\npart of the effort to rewrite interactive rebase in C.\n\nChanges since v1:\n\n - Add a new function to launch the sequence editor, as advised by\n   Phillip Wood[0]\n\n[0] https://public-inbox.org/git/3bfd3470-4482-fe6a-2cd9-08311a0bbaac@talktalk.net/\n\nAlban Gruin (2):\n  editor: add a function to launch the sequence editor\n  rebase--interactive: rewrite the edit-todo functionality in C\n\n builtin/rebase--helper.c   | 13 ++++++++-----\n cache.h                    |  1 +\n editor.c                   | 27 +++++++++++++++++++++++++--\n git-rebase--interactive.sh | 11 +----------\n sequencer.c                | 31 +++++++++++++++++++++++++++++++\n sequencer.h                |  1 +\n strbuf.h                   |  2 ++\n 7 files changed, 69 insertions(+), 17 deletions(-)\n\n-- \n2.16.4\n\n"},{"id":"350060","messageId":"20180613152211.12580-2-alban.gruin@gmail.com","threadId":"48688","inReplyTo":"20180613152211.12580-1-alban.gruin@gmail.com","subject":"[GSoC][PATCH v2 1/2] editor: add a function to launch the sequence editor","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2018-06-13T15:22:10Z","receivedAt":"2018-06-13T15:23:25Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"As part of the rewrite of interactive rebase, the sequencer will need to\nopen the sequence editor to allow the user to edit the todo list.\nInstead of duplicating the existing launch_editor() function, this\nrefactors it to a new function, launch_specified_editor(), which takes\nthe editor as a parameter, in addition to the path, the buffer and the\nenvironment variables.  launch_sequence_editor() is then added to launch\nthe sequence editor.\n\nSigned-off-by: Alban Gruin <alban.gruin@gmail.com>\n---\n cache.h  |  1 +\n editor.c | 27 +++++++++++++++++++++++++--\n strbuf.h |  2 ++\n 3 files changed, 28 insertions(+), 2 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 89a107a7f..c124849a1 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1472,6 +1472,7 @@ extern const char *fmt_name(const char *name, const char *email);\n extern const char *ident_default_name(void);\n extern const char *ident_default_email(void);\n extern const char *git_editor(void);\n+extern const char *git_sequence_editor(void);\n extern const char *git_pager(int stdout_is_tty);\n extern int is_terminal_dumb(void);\n extern int git_ident_config(const char *, const char *, void *);\ndiff --git a/editor.c b/editor.c\nindex 9a9b4e12d..c985eee1f 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -1,4 +1,5 @@\n #include \"cache.h\"\n+#include \"config.h\"\n #include \"strbuf.h\"\n #include \"run-command.h\"\n #include \"sigchain.h\"\n@@ -34,10 +35,21 @@ const char *git_editor(void)\n \treturn editor;\n }\n \n-int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)\n+const char *git_sequence_editor(void)\n {\n-\tconst char *editor = git_editor();\n+\tconst char *editor = getenv(\"GIT_SEQUENCE_EDITOR\");\n+\n+\tif (!editor)\n+\t\tgit_config_get_string_const(\"sequence.editor\", &editor);\n+\tif (!editor)\n+\t\teditor = git_editor();\n \n+\treturn editor;\n+}\n+\n+static int launch_specified_editor(const char *editor, const char *path,\n+\t\t\t\t   struct strbuf *buffer, const char *const *env)\n+{\n \tif (!editor)\n \t\treturn error(\"Terminal is dumb, but EDITOR unset\");\n \n@@ -95,3 +107,14 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en\n \t\treturn error_errno(\"could not read file '%s'\", path);\n \treturn 0;\n }\n+\n+int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)\n+{\n+\treturn launch_specified_editor(git_editor(), path, buffer, env);\n+}\n+\n+int launch_sequence_editor(const char *path, struct strbuf *buffer,\n+\t\t\t   const char *const *env)\n+{\n+\treturn launch_specified_editor(git_sequence_editor(), path, buffer, env);\n+}\ndiff --git a/strbuf.h b/strbuf.h\nindex 60a35aef1..66da9822f 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -575,6 +575,8 @@ extern void strbuf_add_unique_abbrev(struct strbuf *sb,\n  * file's contents are not read into the buffer upon completion.\n  */\n extern int launch_editor(const char *path, struct strbuf *buffer, const char *const *env);\n+extern int launch_sequence_editor(const char *path, struct strbuf *buffer,\n+\t\t\t\t  const char *const *env);\n \n extern void strbuf_add_lines(struct strbuf *sb, const char *prefix, const char *buf, size_t size);\n \n-- \n2.16.4\n\n"},{"id":"350061","messageId":"20180613152211.12580-3-alban.gruin@gmail.com","threadId":"48688","inReplyTo":"20180613152211.12580-1-alban.gruin@gmail.com","subject":"[GSoC][PATCH v2 2/2] rebase--interactive: rewrite the edit-todo functionality in C","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2018-06-13T15:22:11Z","receivedAt":"2018-06-13T15:23:27Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"This rewrites the edit-todo functionality from shell to C.\n\nTo achieve that, a new command mode, `edit-todo`, is added, and the\n`write-edit-todo` flag is removed, as the shell script does not need to\nwrite the edit todo help message to the todo list anymore.\n\nThe shell version is then stripped in favour of a call to the helper.\n\nSigned-off-by: Alban Gruin <alban.gruin@gmail.com>\n---\n builtin/rebase--helper.c   | 13 ++++++++-----\n git-rebase--interactive.sh | 11 +----------\n sequencer.c                | 31 +++++++++++++++++++++++++++++++\n sequencer.h                |  1 +\n 4 files changed, 41 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex ded5e291d..d2990b210 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -12,12 +12,12 @@ 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-\tunsigned flags = 0, keep_empty = 0, rebase_merges = 0, write_edit_todo = 0;\n+\tunsigned flags = 0, keep_empty = 0, rebase_merges = 0;\n \tint abbreviate_commands = 0, rebase_cousins = -1;\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\tADD_EXEC, APPEND_TODO_HELP\n+\t\tADD_EXEC, APPEND_TODO_HELP, EDIT_TODO\n \t} command = 0;\n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"ff\", &opts.allow_ff, N_(\"allow fast-forward\")),\n@@ -27,8 +27,6 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"rebase-merges\", &rebase_merges, N_(\"rebase merge commits\")),\n \t\tOPT_BOOL(0, \"rebase-cousins\", &rebase_cousins,\n \t\t\t N_(\"keep original branch points of cousins\")),\n-\t\tOPT_BOOL(0, \"write-edit-todo\", &write_edit_todo,\n-\t\t\t N_(\"append the edit-todo message to the todo-list\")),\n \t\tOPT_CMDMODE(0, \"continue\", &command, N_(\"continue rebase\"),\n \t\t\t\tCONTINUE),\n \t\tOPT_CMDMODE(0, \"abort\", &command, N_(\"abort rebase\"),\n@@ -49,6 +47,9 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\t\tN_(\"insert exec commands in todo list\"), ADD_EXEC),\n \t\tOPT_CMDMODE(0, \"append-todo-help\", &command,\n \t\t\t    N_(\"insert the help in the todo list\"), APPEND_TODO_HELP),\n+\t\tOPT_CMDMODE(0, \"edit-todo\", &command,\n+\t\t\t    N_(\"edit the todo list during an interactive rebase\"),\n+\t\t\t    EDIT_TODO),\n \t\tOPT_END()\n \t};\n \n@@ -89,6 +90,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \tif (command == ADD_EXEC && argc == 2)\n \t\treturn !!sequencer_add_exec_commands(argv[1]);\n \tif (command == APPEND_TODO_HELP && argc == 1)\n-\t\treturn !!append_todo_help(write_edit_todo, keep_empty);\n+\t\treturn !!append_todo_help(0, keep_empty);\n+\tif (command == EDIT_TODO && argc == 1)\n+\t\treturn !!edit_todo_list(flags);\n \tusage_with_options(builtin_rebase_helper_usage, options);\n }\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 94c23a7af..2defe607f 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -108,16 +108,7 @@ initiate_action () {\n \t\t     --continue\n \t\t;;\n \tedit-todo)\n-\t\tgit stripspace --strip-comments <\"$todo\" >\"$todo\".new\n-\t\tmv -f \"$todo\".new \"$todo\"\n-\t\tcollapse_todo_ids\n-\t\tgit rebase--helper --append-todo-help --write-edit-todo\n-\n-\t\tgit_sequence_editor \"$todo\" ||\n-\t\t\tdie \"$(gettext \"Could not execute editor\")\"\n-\t\texpand_todo_ids\n-\n-\t\texit\n+\t\texec git rebase--helper --edit-todo\n \t\t;;\n \tshow-current-patch)\n \t\texec git show REBASE_HEAD --\ndiff --git a/sequencer.c b/sequencer.c\nindex 1ffd990f7..7cc76332e 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -4386,6 +4386,37 @@ int append_todo_help(unsigned edit_todo, unsigned keep_empty)\n \treturn ret;\n }\n \n+int edit_todo_list(unsigned flags)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tconst char *todo_file = rebase_path_todo();\n+\tFILE *todo;\n+\n+\tif (strbuf_read_file(&buf, todo_file, 0) < 0)\n+\t\treturn error_errno(_(\"could not read '%s'.\"), todo_file);\n+\n+\tstrbuf_stripspace(&buf, 1);\n+\ttodo = fopen_or_warn(todo_file, \"w\");\n+\tif (!todo) {\n+\t\tstrbuf_release(&buf);\n+\t\treturn 1;\n+\t}\n+\n+\tstrbuf_write(&buf, todo);\n+\tfclose(todo);\n+\tstrbuf_release(&buf);\n+\n+\ttransform_todos(flags | TODO_LIST_SHORTEN_IDS);\n+\tappend_todo_help(1, 0);\n+\n+\tif (launch_sequence_editor(todo_file, NULL, NULL))\n+\t\treturn 1;\n+\n+\ttransform_todos(flags & ~(TODO_LIST_SHORTEN_IDS));\n+\n+\treturn 0;\n+}\n+\n static int rewrite_file(const char *path, const char *buf, size_t len)\n {\n \tint rc = 0;\ndiff --git a/sequencer.h b/sequencer.h\nindex e14f6590e..35730b13e 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -81,6 +81,7 @@ int sequencer_add_exec_commands(const char *command);\n int transform_todos(unsigned flags);\n int check_todo_list(void);\n int append_todo_help(unsigned edit_todo, unsigned keep_empty);\n+int edit_todo_list(unsigned flags);\n int skip_unnecessary_picks(void);\n int rearrange_squash(void);\n \n-- \n2.16.4\n\n"},{"id":"350144","messageId":"6db916e7-0c02-785f-cbf9-3f80b6452755@talktalk.net","threadId":"48688","inReplyTo":"20180613152211.12580-1-alban.gruin@gmail.com","subject":"Re: [GSoC][PATCH v2 0/2] rebase -i: rewrite the edit-todo functionality in C","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-06-14T09:53:09Z","receivedAt":"2018-06-14T09:53:17Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Alban\n\nOn 13/06/18 16:22, Alban Gruin wrote:\n> This patch rewrites the edit-todo functionality from shell to C. This is\n> part of the effort to rewrite interactive rebase in C.\n> \n> Changes since v1:\n> \n>  - Add a new function to launch the sequence editor, as advised by\n>    Phillip Wood[0]\n\nThat's great, I think these look fine now\n\nBest Wishes\n\nPhillip\n\n> [0] https://public-inbox.org/git/3bfd3470-4482-fe6a-2cd9-08311a0bbaac@talktalk.net/\n> \n> Alban Gruin (2):\n>   editor: add a function to launch the sequence editor\n>   rebase--interactive: rewrite the edit-todo functionality in C\n> \n>  builtin/rebase--helper.c   | 13 ++++++++-----\n>  cache.h                    |  1 +\n>  editor.c                   | 27 +++++++++++++++++++++++++--\n>  git-rebase--interactive.sh | 11 +----------\n>  sequencer.c                | 31 +++++++++++++++++++++++++++++++\n>  sequencer.h                |  1 +\n>  strbuf.h                   |  2 ++\n>  7 files changed, 69 insertions(+), 17 deletions(-)\n> \n\n"},{"id":"350197","messageId":"xmqqlgbh2khq.fsf@gitster-ct.c.googlers.com","threadId":"48688","inReplyTo":"20180613152211.12580-3-alban.gruin@gmail.com","subject":"Re: [GSoC][PATCH v2 2/2] rebase--interactive: rewrite the edit-todo functionality in C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-06-14T20:14:57Z","receivedAt":"2018-06-14T20:15:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alban Gruin <alban.gruin@gmail.com> writes:\n\n> diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\n> index ded5e291d..d2990b210 100644\n> --- a/builtin/rebase--helper.c\n> +++ b/builtin/rebase--helper.c\n> @@ -12,12 +12,12 @@ 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> -\tunsigned flags = 0, keep_empty = 0, rebase_merges = 0, write_edit_todo = 0;\n> +\tunsigned flags = 0, keep_empty = 0, rebase_merges = 0;\n\nSorry, but where does \"write_edit_todo = 0\" in the preimage come from?\n"},{"id":"350198","messageId":"e5e50510-8297-6e1b-d446-d717b021c6f0@gmail.com","threadId":"48688","inReplyTo":"xmqqlgbh2khq.fsf@gitster-ct.c.googlers.com","subject":"Re: [GSoC][PATCH v2 2/2] rebase--interactive: rewrite the edit-todo functionality in C","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2018-06-14T20:24:26Z","receivedAt":"2018-06-14T20:24:42Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"Hi Junio,\n\nLe 14/06/2018 à 22:14, Junio C Hamano a écrit :\n> Alban Gruin <alban.gruin@gmail.com> writes:\n> \n>> diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\n>> index ded5e291d..d2990b210 100644\n>> --- a/builtin/rebase--helper.c\n>> +++ b/builtin/rebase--helper.c\n>> @@ -12,12 +12,12 @@ 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>> -\tunsigned flags = 0, keep_empty = 0, rebase_merges = 0, write_edit_todo = 0;\n>> +\tunsigned flags = 0, keep_empty = 0, rebase_merges = 0;\n> \n> Sorry, but where does \"write_edit_todo = 0\" in the preimage come from?\n> \n\nIt comes from my conversion of append_todo_help()[0], on which this\nseries is based.\n\n[0]\nhttps://public-inbox.org/git/20180607103012.22981-1-alban.gruin@gmail.com/\n\nCheers,\nAlban\n\n"},{"id":"350201","messageId":"CABPp-BHRK5eNRY-WC9gbzPvw0tZcN2GYgSnLv2UJNUBqsRR6Gw@mail.gmail.com","threadId":"48688","inReplyTo":"e5e50510-8297-6e1b-d446-d717b021c6f0@gmail.com","subject":"Re: [GSoC][PATCH v2 2/2] rebase--interactive: rewrite the edit-todo functionality in C","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-06-14T20:42:48Z","receivedAt":"2018-06-14T20:42:54Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Alban,\n\nOn Thu, Jun 14, 2018 at 1:24 PM, Alban Gruin <alban.gruin@gmail.com> wrote:\n> Le 14/06/2018 à 22:14, Junio C Hamano a écrit :\n>> Alban Gruin <alban.gruin@gmail.com> writes:\n>>\n>>> diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\n>>> index ded5e291d..d2990b210 100644\n>>> --- a/builtin/rebase--helper.c\n>>> +++ b/builtin/rebase--helper.c\n>>> @@ -12,12 +12,12 @@ static const char * const builtin_rebase_helper_usage[] = {\n>>>  int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n>>>  {\n>>>      struct replay_opts opts = REPLAY_OPTS_INIT;\n>>> -    unsigned flags = 0, keep_empty = 0, rebase_merges = 0, write_edit_todo = 0;\n>>> +    unsigned flags = 0, keep_empty = 0, rebase_merges = 0;\n>>\n>> Sorry, but where does \"write_edit_todo = 0\" in the preimage come from?\n>>\n>\n> It comes from my conversion of append_todo_help()[0], on which this\n> series is based.\n>\n> [0]\n> https://public-inbox.org/git/20180607103012.22981-1-alban.gruin@gmail.com/\n\nGiven that both Junio and I had the same basic question, you'll\nprobably want to put dependency information into your cover letters.\nIf a series applies on master, then this isn't needed.  But if it\nrequires other changes that are in a topic in next or pu, you can name\nthe topic that it depends upon (e.g. ag/rebase-p, as from the \"What's\ncooking\" emails or you can grab them as branchnames from\ngit://github.com/gitster/git).  If the dependency is something that\nJunio hasn't picked up yet, provide a link to the other submission.\n\nOthers may have additional suggestions here...\n"},{"id":"350975","messageId":"20180626162143.31608-1-alban.gruin@gmail.com","threadId":"48688","inReplyTo":"20180613152211.12580-1-alban.gruin@gmail.com","subject":"[GSoC][PATCH v3 0/2] rebase -i: rewrite the edit-todo functionality in C","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2018-06-26T16:21:41Z","receivedAt":"2018-06-26T16:23:13Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"This patch rewrites the edit-todo functionality from shell to C. This is\npart of the effort to rewrite interactive rebase in C.\n\nThis patch is based on the fourth iteration of my series rewriting\nappend_todo_help() in C.\n\nChanges since v2:\n\n - Moving edit_todo() from sequencer.c to interactive-rebase.c.\n\nAlban Gruin (2):\n  editor: add a function to launch the sequence editor\n  rebase-interactive: rewrite the edit-todo functionality in C\n\n builtin/rebase--helper.c   | 13 ++++++++-----\n cache.h                    |  1 +\n editor.c                   | 27 +++++++++++++++++++++++++--\n git-rebase--interactive.sh | 11 +----------\n rebase-interactive.c       | 31 +++++++++++++++++++++++++++++++\n rebase-interactive.h       |  1 +\n strbuf.h                   |  2 ++\n 7 files changed, 69 insertions(+), 17 deletions(-)\n\n-- \n2.18.0\n\n"},{"id":"350976","messageId":"20180626162143.31608-2-alban.gruin@gmail.com","threadId":"48688","inReplyTo":"20180626162143.31608-1-alban.gruin@gmail.com","subject":"[GSoC][PATCH v3 1/2] editor: add a function to launch the sequence editor","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2018-06-26T16:21:42Z","receivedAt":"2018-06-26T16:23:16Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"As part of the rewrite of interactive rebase, the sequencer will need to\nopen the sequence editor to allow the user to edit the todo list.\nInstead of duplicating the existing launch_editor() function, this\nrefactors it to a new function, launch_specified_editor(), which takes\nthe editor as a parameter, in addition to the path, the buffer and the\nenvironment variables.  launch_sequence_editor() is then added to launch\nthe sequence editor.\n\nSigned-off-by: Alban Gruin <alban.gruin@gmail.com>\n---\n cache.h  |  1 +\n editor.c | 27 +++++++++++++++++++++++++--\n strbuf.h |  2 ++\n 3 files changed, 28 insertions(+), 2 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 8b447652a..d70ae49ca 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1409,6 +1409,7 @@ extern const char *fmt_name(const char *name, const char *email);\n extern const char *ident_default_name(void);\n extern const char *ident_default_email(void);\n extern const char *git_editor(void);\n+extern const char *git_sequence_editor(void);\n extern const char *git_pager(int stdout_is_tty);\n extern int is_terminal_dumb(void);\n extern int git_ident_config(const char *, const char *, void *);\ndiff --git a/editor.c b/editor.c\nindex 9a9b4e12d..c985eee1f 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -1,4 +1,5 @@\n #include \"cache.h\"\n+#include \"config.h\"\n #include \"strbuf.h\"\n #include \"run-command.h\"\n #include \"sigchain.h\"\n@@ -34,10 +35,21 @@ const char *git_editor(void)\n \treturn editor;\n }\n \n-int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)\n+const char *git_sequence_editor(void)\n {\n-\tconst char *editor = git_editor();\n+\tconst char *editor = getenv(\"GIT_SEQUENCE_EDITOR\");\n+\n+\tif (!editor)\n+\t\tgit_config_get_string_const(\"sequence.editor\", &editor);\n+\tif (!editor)\n+\t\teditor = git_editor();\n \n+\treturn editor;\n+}\n+\n+static int launch_specified_editor(const char *editor, const char *path,\n+\t\t\t\t   struct strbuf *buffer, const char *const *env)\n+{\n \tif (!editor)\n \t\treturn error(\"Terminal is dumb, but EDITOR unset\");\n \n@@ -95,3 +107,14 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en\n \t\treturn error_errno(\"could not read file '%s'\", path);\n \treturn 0;\n }\n+\n+int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)\n+{\n+\treturn launch_specified_editor(git_editor(), path, buffer, env);\n+}\n+\n+int launch_sequence_editor(const char *path, struct strbuf *buffer,\n+\t\t\t   const char *const *env)\n+{\n+\treturn launch_specified_editor(git_sequence_editor(), path, buffer, env);\n+}\ndiff --git a/strbuf.h b/strbuf.h\nindex 60a35aef1..66da9822f 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -575,6 +575,8 @@ extern void strbuf_add_unique_abbrev(struct strbuf *sb,\n  * file's contents are not read into the buffer upon completion.\n  */\n extern int launch_editor(const char *path, struct strbuf *buffer, const char *const *env);\n+extern int launch_sequence_editor(const char *path, struct strbuf *buffer,\n+\t\t\t\t  const char *const *env);\n \n extern void strbuf_add_lines(struct strbuf *sb, const char *prefix, const char *buf, size_t size);\n \n-- \n2.18.0\n\n"},{"id":"350977","messageId":"20180626162143.31608-3-alban.gruin@gmail.com","threadId":"48688","inReplyTo":"20180626162143.31608-1-alban.gruin@gmail.com","subject":"[GSoC][PATCH v3 2/2] rebase-interactive: rewrite the edit-todo functionality in C","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2018-06-26T16:21:43Z","receivedAt":"2018-06-26T16:23:19Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"This rewrites the edit-todo functionality from shell to C.\n\nTo achieve that, a new command mode, `edit-todo`, is added, and the\n`write-edit-todo` flag is removed, as the shell script does not need to\nwrite the edit todo help message to the todo list anymore.\n\nThe shell version is then stripped in favour of a call to the helper.\n\nSigned-off-by: Alban Gruin <alban.gruin@gmail.com>\n---\n builtin/rebase--helper.c   | 13 ++++++++-----\n git-rebase--interactive.sh | 11 +----------\n rebase-interactive.c       | 31 +++++++++++++++++++++++++++++++\n rebase-interactive.h       |  1 +\n 4 files changed, 41 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex 05e73e71d..731a64971 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -13,12 +13,12 @@ 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-\tunsigned flags = 0, keep_empty = 0, rebase_merges = 0, write_edit_todo = 0;\n+\tunsigned flags = 0, keep_empty = 0, rebase_merges = 0;\n \tint abbreviate_commands = 0, rebase_cousins = -1;\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\tADD_EXEC, APPEND_TODO_HELP\n+\t\tADD_EXEC, APPEND_TODO_HELP, EDIT_TODO\n \t} command = 0;\n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"ff\", &opts.allow_ff, N_(\"allow fast-forward\")),\n@@ -28,8 +28,6 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"rebase-merges\", &rebase_merges, N_(\"rebase merge commits\")),\n \t\tOPT_BOOL(0, \"rebase-cousins\", &rebase_cousins,\n \t\t\t N_(\"keep original branch points of cousins\")),\n-\t\tOPT_BOOL(0, \"write-edit-todo\", &write_edit_todo,\n-\t\t\t N_(\"append the edit-todo message to the todo-list\")),\n \t\tOPT_CMDMODE(0, \"continue\", &command, N_(\"continue rebase\"),\n \t\t\t\tCONTINUE),\n \t\tOPT_CMDMODE(0, \"abort\", &command, N_(\"abort rebase\"),\n@@ -50,6 +48,9 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\t\tN_(\"insert exec commands in todo list\"), ADD_EXEC),\n \t\tOPT_CMDMODE(0, \"append-todo-help\", &command,\n \t\t\t    N_(\"insert the help in the todo list\"), APPEND_TODO_HELP),\n+\t\tOPT_CMDMODE(0, \"edit-todo\", &command,\n+\t\t\t    N_(\"edit the todo list during an interactive rebase\"),\n+\t\t\t    EDIT_TODO),\n \t\tOPT_END()\n \t};\n \n@@ -90,6 +91,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \tif (command == ADD_EXEC && argc == 2)\n \t\treturn !!sequencer_add_exec_commands(argv[1]);\n \tif (command == APPEND_TODO_HELP && argc == 1)\n-\t\treturn !!append_todo_help(write_edit_todo, keep_empty);\n+\t\treturn !!append_todo_help(0, keep_empty);\n+\tif (command == EDIT_TODO && argc == 1)\n+\t\treturn !!edit_todo_list(flags);\n \tusage_with_options(builtin_rebase_helper_usage, options);\n }\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 94c23a7af..2defe607f 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -108,16 +108,7 @@ initiate_action () {\n \t\t     --continue\n \t\t;;\n \tedit-todo)\n-\t\tgit stripspace --strip-comments <\"$todo\" >\"$todo\".new\n-\t\tmv -f \"$todo\".new \"$todo\"\n-\t\tcollapse_todo_ids\n-\t\tgit rebase--helper --append-todo-help --write-edit-todo\n-\n-\t\tgit_sequence_editor \"$todo\" ||\n-\t\t\tdie \"$(gettext \"Could not execute editor\")\"\n-\t\texpand_todo_ids\n-\n-\t\texit\n+\t\texec git rebase--helper --edit-todo\n \t\t;;\n \tshow-current-patch)\n \t\texec git show REBASE_HEAD --\ndiff --git a/rebase-interactive.c b/rebase-interactive.c\nindex c79c819b9..ace8e095b 100644\n--- a/rebase-interactive.c\n+++ b/rebase-interactive.c\n@@ -66,3 +66,34 @@ int append_todo_help(unsigned edit_todo, unsigned keep_empty)\n \n \treturn ret;\n }\n+\n+int edit_todo_list(unsigned flags)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tconst char *todo_file = rebase_path_todo();\n+\tFILE *todo;\n+\n+\tif (strbuf_read_file(&buf, todo_file, 0) < 0)\n+\t\treturn error_errno(_(\"could not read '%s'.\"), todo_file);\n+\n+\tstrbuf_stripspace(&buf, 1);\n+\ttodo = fopen_or_warn(todo_file, \"w\");\n+\tif (!todo) {\n+\t\tstrbuf_release(&buf);\n+\t\treturn 1;\n+\t}\n+\n+\tstrbuf_write(&buf, todo);\n+\tfclose(todo);\n+\tstrbuf_release(&buf);\n+\n+\ttransform_todos(flags | TODO_LIST_SHORTEN_IDS);\n+\tappend_todo_help(1, 0);\n+\n+\tif (launch_sequence_editor(todo_file, NULL, NULL))\n+\t\treturn 1;\n+\n+\ttransform_todos(flags & ~(TODO_LIST_SHORTEN_IDS));\n+\n+\treturn 0;\n+}\ndiff --git a/rebase-interactive.h b/rebase-interactive.h\nindex 47372624e..155219e74 100644\n--- a/rebase-interactive.h\n+++ b/rebase-interactive.h\n@@ -2,5 +2,6 @@\n #define REBASE_INTERACTIVE_H\n \n int append_todo_help(unsigned edit_todo, unsigned keep_empty);\n+int edit_todo_list(unsigned flags);\n \n #endif\n-- \n2.18.0\n\n"},{"id":"351040","messageId":"nycvar.QRO.7.76.6.1806262344560.21419@tvgsbejvaqbjf.bet","threadId":"48688","inReplyTo":"20180626162143.31608-1-alban.gruin@gmail.com","subject":"Re: [GSoC][PATCH v3 0/2] rebase -i: rewrite the edit-todo functionality in C","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-06-26T21:45:17Z","receivedAt":"2018-06-26T21:45:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Alban,\n\nOn Tue, 26 Jun 2018, Alban Gruin wrote:\n\n> This patch rewrites the edit-todo functionality from shell to C. This is\n> part of the effort to rewrite interactive rebase in C.\n> \n> This patch is based on the fourth iteration of my series rewriting\n> append_todo_help() in C.\n> \n> Changes since v2:\n> \n>  - Moving edit_todo() from sequencer.c to interactive-rebase.c.\n\nExcellent.\n\nThank you,\nDscho\n"},{"id":"351041","messageId":"nycvar.QRO.7.76.6.1806262346540.21419@tvgsbejvaqbjf.bet","threadId":"48688","inReplyTo":"20180626162143.31608-3-alban.gruin@gmail.com","subject":"Re: [GSoC][PATCH v3 2/2] rebase-interactive: rewrite the edit-todo functionality in C","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-06-26T21:47:20Z","receivedAt":"2018-06-26T21:47:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Alban,\n\nOn Tue, 26 Jun 2018, Alban Gruin wrote:\n\n> This rewrites the edit-todo functionality from shell to C.\n> \n> To achieve that, a new command mode, `edit-todo`, is added, and the\n> `write-edit-todo` flag is removed, as the shell script does not need to\n> write the edit todo help message to the todo list anymore.\n> \n> The shell version is then stripped in favour of a call to the helper.\n> \n> Signed-off-by: Alban Gruin <alban.gruin@gmail.com>\n\nBoth patches are ACKed by me,\nDscho\n"}]}