rebase: cleanup "--exec" option handling
PATCH 1/1 rebase: cleanup "--exec" option handling
From: Phillip Wood <[email protected]>
When handling "--exec" rebase collects the commands into a struct string_list, then prepends "exec " to each command creating a multi line string and finally splits that string back into a list of commands. This is an artifact of the scripted rebase and the need to support "rebase --preserve-merges". Now that "--preserve-merges" no-longer exists we can cleanup the way the argument is handled. There is no need to add the "exec " prefix to the commands as that is added by todo_list_to_strbuf().
---
rebase: cleanup "--exec" option handling
A small cleanup following the removal of "--preserve-merges"
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1461%2Fphillipwood%2Frebase-cleanup-exec-handling-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1461/phillipwood/rebase-cleanup-exec-handling-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1461
builtin/rebase.c | 45 +++++++++++---------------------------------- sequencer.c | 4 ++-- 2 files changed, 13 insertions(+), 36 deletions(-)
builtin/rebase.c +11 -34
@@ -113,7 +113,7 @@ struct rebase_options {int autostash;int committer_date_is_author_date;int ignore_date; -char *cmd; +struct string_list exec;int allow_empty_message;int rebase_merges, rebase_cousins;char *strategy, *strategy_opts;
@@ -131,6 +131,7 @@ struct rebase_options { .default_backend = "merge", \ .flags = REBASE_NO_QUIET, \ .git_am_opts = STRVEC_INIT, \ + .exec = STRING_LIST_INIT_NODUP, \ .git_format_patch_opt = STRBUF_INIT, \ .fork_point = -1, \ }
@@ -243,17 +244,6 @@ static int init_basic_state(struct replay_opts *opts, const char *head_name,return write_basic_state (opts, head_name, onto, orig_head); } -static void split_exec_commands (const char *cmd,struct string_list *commands) -{ -if (cmd && *cmd) { -string_list_split (commands, cmd,'\n' ,-1 ); - -/* rebase.c adds a new line to cmd after every command, -* so here the last command is always empty */ -string_list_remove_empty_items (commands,0 ); - } -} -static int do_interactive_rebase (struct rebase_options *opts,unsigned flags) {int ret;
@@ -261,7 +251,6 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)struct strvec make_script_args =STRVEC_INIT ;struct todo_list todo_list =TODO_LIST_INIT ;struct replay_opts replay =get_replay_opts (opts); -struct string_list commands =STRING_LIST_INIT_DUP ;if (get_revision_ranges (opts->upstream, opts->onto, &opts->orig_head->object.oid, &revisions, &shortrevisions))
@@ -297,14 +286,12 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags) &todo_list))BUG ("unusable todo list" ); -split_exec_commands (opts->cmd, &commands); ret =complete_action (the_repository, &replay, flags, shortrevisions, opts->onto_name, opts->onto, - &opts->orig_head->object.oid, &commands, + &opts->orig_head->object.oid, &opts->exec, opts->autosquash, opts->update_refs, &todo_list); } -string_list_clear (&commands,0 );free (revisions);free (shortrevisions);todo_list_release (&todo_list);
@@ -1032,7 +1019,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)struct object_id branch_base;int ignore_whitespace =0 ;const char *gpg_sign =NULL ; -struct string_list exec =STRING_LIST_INIT_NODUP ;const char *rebase_merges =NULL ;struct string_list strategy_options =STRING_LIST_INIT_NODUP ;struct object_id squash_onto;
@@ -1127,7 +1113,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)N_ ("GPG-sign commits" ), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },OPT_AUTOSTASH (&options.autostash), -OPT_STRING_LIST ('x' ,"exec" , &exec,N_ ("exec" ), +OPT_STRING_LIST ('x' ,"exec" , &options.exec,N_ ("exec" ),N_ ("add exec lines after each commit of the " "editable list" )), OPT_BOOL_F(0, "allow-empty-message",
@@ -1250,7 +1236,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)if (trace2_is_enabled ()) {if (is_merge (&options))trace2_cmd_mode ("interactive" ); -else if (exec.nr) +else if (options.exec.nr)trace2_cmd_mode ("interactive-exec" );else trace2_cmd_mode (action_names[options.action]);
@@ -1378,7 +1364,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)if ((options.flags &REBASE_INTERACTIVE_EXPLICIT ) || (options.action !=ACTION_NONE ) || - (exec.nr >0 ) || + (options.exec.nr >0 ) || options.autosquash) { allow_preemptive_ff =0 ; }
@@ -1402,8 +1388,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) } } -for (i =0 ; i < exec.nr; i++) -if (check_exec_cmd (exec.items[i].string)) +for (i =0 ; i < options.exec.nr; i++) +if (check_exec_cmd (options.exec.items[i].string))exit (1 );if (!(options.flags &REBASE_NO_QUIET ))
@@ -1422,17 +1408,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)if (gpg_sign) options.gpg_sign_opt =xstrfmt ("-S%s" , gpg_sign); -if (exec.nr) { -int i; - +if (options.exec.nr)imply_merge (&options,"--exec" ); -strbuf_reset (&buf); -for (i =0 ; i < exec.nr; i++) -strbuf_addf (&buf,"exec %s\n" , exec.items[i].string); - options.cmd =xstrdup (buf.buf); - } -if (rebase_merges) {if (!*rebase_merges) ;/* default mode; do nothing */
@@ -1543,7 +1521,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)if (options.empty ==EMPTY_UNSPECIFIED ) {if (options.flags &REBASE_INTERACTIVE_EXPLICIT ) options.empty =EMPTY_ASK ; -else if (exec.nr >0 ) +else if (options.exec.nr >0 ) options.empty =EMPTY_KEEP ;else options.empty =EMPTY_DROP ;
@@ -1831,11 +1809,10 @@ cleanup:free (options.head_name);strvec_clear (&options.git_am_opts);free (options.gpg_sign_opt); -free (options.cmd); +string_list_clear (&options.exec,0 );free (options.strategy);strbuf_release (&options.git_format_patch_opt);free (squash_onto_name); -string_list_clear (&exec,0 );string_list_clear (&strategy_options,0 );return !!ret; }
sequencer.c +2 -2
@@ -5745,8 +5745,8 @@ static void todo_list_add_exec_commands(struct todo_list *todo_list, base_items[i].command =TODO_EXEC ; base_items[i].offset_in_buf = base_offset; - base_items[i].arg_offset = base_offset +strlen ("exec " ); - base_items[i].arg_len = command_len -strlen ("exec " ); + base_items[i].arg_offset = base_offset; + base_items[i].arg_len = command_len; base_offset += command_len +1 ; }
base-commit: a38d39a4c50d1275833aba54c4dbdfce9e2e9ca1
Signature · 1 line
gitgitgadget
Quoted 16 lines · Phillip Wood via GitGitGadget
From: Phillip Wood <[email protected]>
When handling "--exec" rebase collects the commands into a struct
string_list, then prepends "exec " to each command creating a multi line string and finally splits that string back into a list of commands. This is an artifact of the scripted rebase and the need to support "rebase --preserve-merges". Now that "--preserve-merges" no-longer exists we can cleanup the way the argument is handled. There is no need to add the "exec " prefix to the commands as that is added bytodo_list_to_strbuf().Signed-off-by Phillip Wood <[email protected]>---
rebase: cleanup "--exec" option handlingA small cleanup following the removal of "--preserve-merges"
Quoted 8 lines
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1461%2Fphillipwood%2Frebase-cleanup-exec-handling-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1461/phillipwood/rebase-cleanup-exec-handling-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1461builtin/rebase.c | 45 +++++++++++---------------------------------- sequencer.c | 4 ++-- 2 files changed, 13 insertions(+), 36 deletions(-)
I can't really comment on code, but I appreciate any simplifications around rebase --exec, because it will probably make it easier for me to revive the topic about rebase --edit-todo --exec from the end of 2019:
https://lore.kernel.org/git/[email protected]/T/#u
It's current state (experimented a bit a week ago) is on GitHub:
https://github.com/rybak/git/tree/edit-todo-exec
Thank you.