| Message ID | 20190620095523.10003-3-pclouds@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Some more on top of nd/switch-and-restore | expand |
On 6/20/2019 5:55 AM, Nguyễn Thái Ngọc Duy wrote: > In c45f0f525d (switch: reject if some operation is in progress, > 2019-03-29), a check is added to prevent switching when some operation > is in progress. The reason is it's often not safe to do so. > > This is true for merge, am, rebase, cherry-pick and revert, but not so > much for bisect because bisecting is basically jumping/switching between > a bunch of commits to pin point the first bad one. git-bisect suggests > the next commit to test, but it's not wrong for the user to test a > different commit because git-bisect cannot have the knowledge to know > better. When a user switches commits during a bisect, it can just create new known-good or known-bad commits, right? It won't mess up the next selection of a test commit? I'm imagining someone jumping to a commit between two known-bad commits or something, when it is more likely that they are jumping to a parallel history. > For this reason, allow to switch when bisecting (*). I considered if we > should still prevent switching by default and allow it with > --ignore-in-progress. But I don't think the prevention really adds > anything much. > > If the user switches away by mistake, since we print the previous HEAD > value, even if they don't know about the "-" shortcut, switching back is > still possible. I tell everyone I know about the "-" shortcut, and I'm always surprised they didn't already know about "cd -". > The warning will be printed on every switch while bisect is still > ongoing, not the first time you switch away from bisect's suggested > commit, so it could become a bit annoying. I think a one-line warning is fine, as a power user doing this a lot will develop blinders that ignore the message as they switch during a bisect. -Stolee
On Thu, Jun 20, 2019 at 9:02 PM Derrick Stolee <stolee@gmail.com> wrote: > > On 6/20/2019 5:55 AM, Nguyễn Thái Ngọc Duy wrote: > > In c45f0f525d (switch: reject if some operation is in progress, > > 2019-03-29), a check is added to prevent switching when some operation > > is in progress. The reason is it's often not safe to do so. > > > > This is true for merge, am, rebase, cherry-pick and revert, but not so > > much for bisect because bisecting is basically jumping/switching between > > a bunch of commits to pin point the first bad one. git-bisect suggests > > the next commit to test, but it's not wrong for the user to test a > > different commit because git-bisect cannot have the knowledge to know > > better. > > When a user switches commits during a bisect, it can just create new > known-good or known-bad commits, right? It won't mess up the next > selection of a test commit? I'm imagining someone jumping to a commit > between two known-bad commits or something, when it is more likely that > they are jumping to a parallel history. Until you run "git bisect bad" or "git bisect good" I don't think switching could mess up bisect. And I'm pretty sure I marked wrong ones once or twice and git-bisect did complain. It would be a good idea to warn about jumping in known-good-or-bad commit ranges. I'll keep this as an improvement point (there's still another one I haven't done, also about warning improvement, sigh). > > For this reason, allow to switch when bisecting (*). I considered if we > > should still prevent switching by default and allow it with > > --ignore-in-progress. But I don't think the prevention really adds > > anything much. > > > > If the user switches away by mistake, since we print the previous HEAD > > value, even if they don't know about the "-" shortcut, switching back is > > still possible. > > I tell everyone I know about the "-" shortcut, and I'm always surprised > they didn't already know about "cd -". I actually wanted to go a bit more aggressive/verbose about "teaching" users via the advice framework, but allows the user to turn off the "lessons" they already learned. Something like gcc adding [-Wno-foobar] in a warning to hint how to turn it off. I'll come back to this at some point, hopefully.
diff --git a/builtin/checkout.c b/builtin/checkout.c index bed79ae595..f884d27f1f 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1326,9 +1326,7 @@ static void die_if_some_operation_in_progress(void) "Consider \"git revert --quit\" " "or \"git worktree add\".")); if (state.bisect_in_progress) - die(_("cannot switch branch while bisecting\n" - "Consider \"git bisect reset HEAD\" " - "or \"git worktree add\".")); + warning(_("you are switching branch while bisecting")); } static int checkout_branch(struct checkout_opts *opts,
In c45f0f525d (switch: reject if some operation is in progress, 2019-03-29), a check is added to prevent switching when some operation is in progress. The reason is it's often not safe to do so. This is true for merge, am, rebase, cherry-pick and revert, but not so much for bisect because bisecting is basically jumping/switching between a bunch of commits to pin point the first bad one. git-bisect suggests the next commit to test, but it's not wrong for the user to test a different commit because git-bisect cannot have the knowledge to know better. For this reason, allow to switch when bisecting (*). I considered if we should still prevent switching by default and allow it with --ignore-in-progress. But I don't think the prevention really adds anything much. If the user switches away by mistake, since we print the previous HEAD value, even if they don't know about the "-" shortcut, switching back is still possible. The warning will be printed on every switch while bisect is still ongoing, not the first time you switch away from bisect's suggested commit, so it could become a bit annoying. (*) of course when it's safe to do so, i.e. no loss of local changes and stuff. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- builtin/checkout.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)