On 03/09/2023 21:18, Oswald Buddenhagen wrote:
On Sun, Sep 03, 2023 at 08:48:14PM +0100, Phillip Wood wrote:
On 03/09/2023 20:25, Oswald Buddenhagen wrote:
On Sun, Sep 03, 2023 at 07:40:00PM +0100, Phillip Wood wrote:
it only matters for "cherry-pick --skip"
that doesn't seem right. a --skip is just a --continue with a prior
reset, more or less.
sequencer_skip() calls rollback_is_safe() which checks the abort
safety file.
that's weird. can you think of a good reason for doing that?
I think it is clear from the code - so it does not reset changes after
the user has committed the conflict resolution.
i'll try to find a better "choke point".
I think that is probably tricky,
yeah
I'm not really clear what the aim/purpose of this refactoring is.
to make my head not explode.
more specifically, to get it out of the way of the rebase path, which is
what i'm actually concerned with.
rebase and cherry-pick share the same code path most of the time. In
particular "cherry-pick --continue" and "rebase --continue" both use
sequencer_continue() as their entry point so I think the best you can do
is guard the calls to update_abort_safety_file() with "if
(!is_rebase_i(opts))" or add "if (is_rebase_i(opts)) return" to the
start of update_abort_safety_file().
generally, i think this whole ad-hoc state management is a nightmare,
and i'd be surprised if there weren't some more loose ends.
i think i'd aim for an object-oriented-ish design with an encapsulated
state, lazy loading getters, lazy setters, and a commit entry point (or
maybe several partial ones). no idea how that would play out.
I've been working on something similar to only write the state to disc
when the sequencer stops for user interaction. I'm hoping to have the
first set of patches ready to submit in the next development cycle. You
can see the branch at [1]. It is very much a work in progress at the
moment, the code is mostly OK (I'm running it in my git build) but some
commits are empty, others need splitting and the commit messages need a
lot of work. The basic idea is to add a private struct that holds the
state and write that to disc when pick_commits() returns.
Best Wishes
Phillip
[1] https://github.com/phillipwood/git/commits/wip/sequencer-context
if you did a fresh commit before or after the single pick, you'd lose
it.
Oh, I can see that you'd lose a commit made before a single pick but I
don't see how you'd lose a commit made after it.
right. thinko. it's a bit late here. ^^
regards
