Hi Elijah
On 28/03/2019 17:39, Elijah Newren wrote:
Hi Phillip,
On Thu, Mar 28, 2019 at 9:23 AM Phillip Wood <phillip.wood123@xxxxxxxxx> wrote:
On 28/03/2019 11:04, Duy Nguyen wrote:
On Wed, Mar 27, 2019 at 5:24 PM Phillip Wood <phillip.wood123@xxxxxxxxx> wrote:
On 26/03/2019 15:48, Elijah Newren wrote:
On Tue, Mar 26, 2019 at 8:24 AM Duy Nguyen <pclouds@xxxxxxxxx> wrote:
On Tue, Mar 26, 2019 at 10:01 PM Elijah Newren <newren@xxxxxxxxx> wrote:
Yes, and in the middle of a cherry-pick with a range you've added some
commits to one branch and some to another. In the middle of a revert
you're doing similar. It sounds like crazytown to me (and maybe we
shouldn't provide the --ignore-in-process flag unless users clamor for
it
I missed this part in my last reading. I think if we could safely
switch away and get back to resume, then --ignore-in-process could
still be useful.
If we can get back safely then that makes sense, I'm not sure about
switching while there are conflicts or staged changes though, it feels
like there's more potential for things to go wrong there.
I really like that way of putting it; I think that makes it much
clearer. Note, though that staged changes and conflicts could happen
with any of rebase, merge, cherry-pick, or revert, so this problem is
not limited to a subset of those operations.
Indeed
I sometimes switch to another commit to check out
stuff then back. For interactive rebase with "edit" command for
example, it's quite safe to do so. (yes the other option is "git
worktree add", but that could be a heavy hammer sometimes)
I think that could be the way to go for merges and cherry-picks, or
Just so we're clear, what is your "the way" to go? to remove
CHERRY_HEAD_PICK and MERGE_HEAD (and other MERGE_* as well) if
--ignore-in-process is specified? Or to leave MERGE_* and
CHERRY_PICK_HEAD alone and delete other stuff?
I was agreeing with Elijah about dropping --ignore-in-progress unless
there's a demand for it or at least restricting it so that it requires
--discard-changes and aborts in-progress merges and single in-progress
cherry-picks/reverts. (I'm worried about people switching branches when
cherry-picking more than one commit, though as you say it can make sense
during a rebase.)
I understand the desire to prevent mis-uses, and I agree that if there
are staged changes or conflicts it's really likely things will go
sideways. But I think we should instead check for those situations
rather than use e.g. rebase vs. merge as a proxy for whether those
problems could be present.
When cherry-picking multiple commits if the user commits the conflict
resolution with 'git commit' then the presence of .git/sequencer is the
only sign that a cherry-pick is in progress (wt-status.c fails to detect
this, I've got a fix but no tests yet). rebase can also stop without
having conflicts or staged changes so I think we need to check for in
progress commands as well as conflicts (what do we want to do if someone
tries to switch in the middle of a bisect? - I don't have a strong
opinion). I agree switch should fail if there are conflicts, but I think
it is fine to switch with staged or unstaged changes if there isn't a
merge etc in progress (I quite often start working on something and
then realize I haven't started a new branch just before I commit). I
could possibly be convinced that silently switching with staged changes
is always a bad idea though.
I am especially concerned with the idea of
having something like "git switch --ignore-in-progress
--discard-changes" being used to quit merges or cherry-picks or
reverts or even rebases. In my opinion, doing so is creating flags to > combine uncommon pairs of git commands (git <operation> --quit + git
switch) in a way that is far less clear. I think that's a bad route
to go down, and we should keep the commands orthogonal
keeping commands orthogonal is certainly clearer, if less convenient -
lets do it (assuming Duy agrees).
(if I could
start all over, I'd also make reset and checkout and everything else
stop modifying any in-progress state).
Instead, I would either:
* Drop `--ignore-in-progress` for now. (Although Duy had a
meaningful usecase)
I think it could be useful during a rebase, I'm not sure about any of
the other operations though.
OR
* Make `git switch --ignore-in-progress <branch>` leave all process
state in place and switch branches, if we would otherwise be able to
switch branches (i.e. there isn't dirty or conflicted changes in the
way).
I thought we allowed branch switches when there are staged or unstaged
changes, I don't think that is a problem unless we're in the middle of a
merge etc. I'm still not sure it's a good idea to switch branches in the
middle of a multiple cherry-pick, maybe we should print a warning.
BUT, make sure to also:
* Make '--ignore-in-progress' incompatible with both '-m' and
'--discard-changes'; if folks try to use either of those additional
options with --ignore-in-progress, tell people to use `<operation>
--quit` first.
I think of --discard-changes like --abort. --quit only removes the state
dir so would pair with -m, as it does not reset the index or working tree.
Overall I think we're more or less in agreement modulo the treatment of
staged changes when there is no merge etc in progress.
Best Wishes
Phillip
Do NOT provide an override. (Alternatively, refer to
`<operation> --quit` as the override, since it is).
Elijah
