* [PATCH] rebase: cleanup "--exec" option handling
@ 2023-01-12 16:50 Phillip Wood via GitGitGadget
2023-01-13 20:06 ` Andrei Rybak
0 siblings, 1 reply; 2+ messages in thread
From: Phillip Wood via GitGitGadget @ 2023-01-12 16:50 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
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().
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
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(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 1481c5b6a5b..a26cc0cfdb5 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -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;
}
diff --git a/sequencer.c b/sequencer.c
index bcb662e23be..3e4a1972897 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -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
--
gitgitgadget
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] rebase: cleanup "--exec" option handling
2023-01-12 16:50 [PATCH] rebase: cleanup "--exec" option handling Phillip Wood via GitGitGadget
@ 2023-01-13 20:06 ` Andrei Rybak
0 siblings, 0 replies; 2+ messages in thread
From: Andrei Rybak @ 2023-01-13 20:06 UTC (permalink / raw)
To: Phillip Wood via GitGitGadget, git; +Cc: Johannes Schindelin, Phillip Wood
On 12/01/2023 17:50, Phillip Wood via GitGitGadget wrote:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> 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().
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> 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(-)
>
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/20191114163549.7648-1-rybak.a.v@gmail.com/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.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-01-13 20:07 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-12 16:50 [PATCH] rebase: cleanup "--exec" option handling Phillip Wood via GitGitGadget
2023-01-13 20:06 ` Andrei Rybak
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).