Re: [GSoC][PATCH v3 12/13] rebase -i: implement the logic to initialize the variable $revision in C
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 12, 2018, 18:15 UTC
- Message-ID
- <xmqqo9fcxquk.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20180710121557.6698-13-alban.gruin@gmail.com>
Alban Gruin <alban.gruin@gmail.com> writes:
Show 21 quoted lines
> @@ -50,6 +71,13 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
> N_("prepare the branch to be rebased"), PREPARE_BRANCH),
> OPT_CMDMODE(0, "complete-action", &command,
> N_("complete the action"), COMPLETE_ACTION),
> + OPT_STRING(0, "onto", &onto, N_("onto"), N_("onto")),
> + OPT_STRING(0, "restrict-revision", &restrict_revisions,
> + N_("restrict-revision"), N_("restrict revision")),
> + OPT_STRING(0, "squash-onto", &squash_onto, N_("squash-onto"),
> + N_("squash onto")),
> + OPT_STRING(0, "upstream", &upstream, N_("upstream"),
> + N_("the upstream commit")),
> OPT_END()
> };
>
> @@ -77,8 +105,30 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
> return !!sequencer_continue(&opts);
> if (command == ABORT && argc == 1)
> return !!sequencer_remove_state(&opts);
> - if (command == MAKE_SCRIPT && argc > 1)
> - return !!sequencer_make_script(stdout, argc, argv, flags);
> + if (command == MAKE_SCRIPT && (argc == 1 || argc == 2)) {Sorry but I am confused. The call to rebase--helper --make-script in git-rebase--interactive.sh we see below has only dashed options (like --upstream, --onto, --squash-onto, --restrict-revision) and the option parameters given to these options.
What are the remaining 1 or 2 arguments we are processing here?
Well, actually argc==1 means there is nothing else, so I am asking about the case where argc==2.
Show 17 quoted lines
> + char *revisions = NULL; > + struct argv_array make_script_args = ARGV_ARRAY_INIT; > + > + if (!upstream && squash_onto) > + write_file(path_squash_onto(), "%s\n", squash_onto); > + > + ret = get_revision_ranges(upstream, onto, &head_hash, &revisions); > + if (ret) > + return ret; > + > + argv_array_pushl(&make_script_args, "", revisions, NULL); > + if (argc == 2) > + argv_array_push(&make_script_args, restrict_revisions); > + > + ret = !!sequencer_make_script(stdout, > + make_script_args.argc, make_script_args.argv, > + flags);
Exactly the same comment on !! as an earlier step applies here.
> + free(revisions); > + argv_array_clear(&make_script_args);
If I am reading the body of this if() block correctly, I think it does everything init_revisions_and_shortrevisions shell function does, i.e. compute $revisions for both cases with or without upstream and write squash-onto state if needed, so that we can call the sequencer_make_script() helper with necessary $revisions arg without being passed from the command line of --make-script helper.
But the hunk below tells me that you are still calling init_revisions_and_shortrevisions shell function before we are called. Why? IOW, why isn't this step removing that shell function *and* the call to it, now its logic is fully implemented in the body of this if() block?
Show 5 quoted lines
> + return ret; > + } > if (command == CHECK_TODO_LIST && argc == 1) > return !!check_todo_list(); > if (command == REARRANGE_SQUASH && argc == 1)
Thanks.
Show 15 quoted lines
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 0d66c0f8b..4ca47aed1 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -92,7 +92,9 @@ git_rebase__interactive () {
> git rebase--helper --make-script ${keep_empty:+--keep-empty} \
> ${rebase_merges:+--rebase-merges} \
> ${rebase_cousins:+--rebase-cousins} \
> - $revisions ${restrict_revision+^$restrict_revision} >"$todo" ||
> + ${upstream:+--upstream "$upstream"} ${onto:+--onto "$onto"} \
> + ${squash_onto:+--squash-onto "$squash_onto"} \
> + ${restrict_revision:+--restrict-revision ^"$restrict_revision"} >"$todo" ||
> die "$(gettext "Could not generate todo list")"
>
> exec git rebase--helper --complete-action "$shortrevisions" "$onto_name" \