Alban Gruin <alban.gr...@gmail.com> writes:

> @@ -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.

> +             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?

> +             return ret;
> +     }
>       if (command == CHECK_TODO_LIST && argc == 1)
>               return !!check_todo_list();
>       if (command == REARRANGE_SQUASH && argc == 1)

Thanks.

> 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" \

Reply via email to