Re: [GSoC][PATCH v6 11/20] rebase -i: rewrite complete_action() in C

2 messages, 2 authors, 2018-08-10 · open the first message on its own page

Re: [GSoC][PATCH v6 11/20] rebase -i: rewrite complete_action() in C

From: Junio C Hamano <hidden>
Date: 2018-08-10 19:25:53

Alban Gruin [off-list ref] writes:
This rewrites complete_action() from shell to C.

A new mode is added to rebase--helper (`--complete-action`), as well as
a new flag (`--autosquash`).

Finally, complete_action() is stripped from git-rebase--interactive.sh.

The original complete_action() would return the code 2 when the todo
list contained no actions.  This was a special case for rebase -i and
-p; git-rebase.sh would then apply the autostash, delete the state
directory, and die with the message "Nothing to do".  This cleanup is
rewritten in C instead of returning 2.  As rebase -i no longer returns
2, the comment describing this behaviour in git-rebase.sh is updated to
reflect this change.
Hmph.  It is easy enough to do the clean-up ourselves in this code,
instead of asking the caller to do so.  On the other hand, stashing
of local changes is done by the caller, so it feels a bit strange
way to divide the labor between the two parts.

Other than that design choice, the patch itself looks reasonable and
a fairly faithful reimplementation of what the scripted one did.

Re: [GSoC][PATCH v6 11/20] rebase -i: rewrite complete_action() in C

From: Alban Gruin <hidden>
Date: 2018-08-10 19:36:53

Hi Junio,

Le 10/08/2018 à 21:25, Junio C Hamano a écrit :
Alban Gruin [off-list ref] writes:
quoted
This rewrites complete_action() from shell to C.

A new mode is added to rebase--helper (`--complete-action`), as well as
a new flag (`--autosquash`).

Finally, complete_action() is stripped from git-rebase--interactive.sh.

The original complete_action() would return the code 2 when the todo
list contained no actions.  This was a special case for rebase -i and
-p; git-rebase.sh would then apply the autostash, delete the state
directory, and die with the message "Nothing to do".  This cleanup is
rewritten in C instead of returning 2.  As rebase -i no longer returns
2, the comment describing this behaviour in git-rebase.sh is updated to
reflect this change.
Hmph.  It is easy enough to do the clean-up ourselves in this code,
instead of asking the caller to do so.  On the other hand, stashing
of local changes is done by the caller, so it feels a bit strange
way to divide the labor between the two parts.

Other than that design choice, the patch itself looks reasonable and
a fairly faithful reimplementation of what the scripted one did.
This was the behaviour of the old complete_action().  The new one cleans
up by itself; this is the
apply_autostash()/sequencer_remove_state()/todo_list_release() dance
done at three places inside complete_action(), and has not changed since
v3[0].

Maybe I misunderstood what you said?

[0]
https://public-inbox.org/git/20180710121557.6698-11-alban.gruin@gmail.com/

Cheers,
Alban
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help