When bisecting, always mark the current commit as good/bad, not the selected

[stefanhaller]

For marking as good or bad, the current commit is pretty much always the one you want to mark, not the selected. It's different for skipping; sometimes you know already that a certain commit doesn't compile, for example, so you might navigate there and mark it as skipped. So in the case that the current commit is not the selected one, we now offer two separate menu entries for skipping, one for the current commit and one for the selected.

Fixes #2836.

• Please check if the PR fulfills these requirements

• Cheatsheets are up-to-date (run go run scripts/cheatsheet/main.go generate)
• Code has been formatted (see here)
• Tests have been added/updated (see here for the integration test guide)
• Text is internationalised (see here)
• Docs (specifically docs/Config.md) have been updated if necessary
• You've read through your own file changes for silly mistakes etc

[jesseduffield]

When we're bisecting (meaning there is a 'current' commit), what are your thoughts on showing both skip options unconditionally, for the sake of muscle memory?

[stefanhaller]

I'd vote against it. In the vast majority of cases, the selected commit and the current commit will be the same, so it would be confusing to always see two menu entries for the same sha that only differ in their keyboard shortcut. Also (in my personal experience, at least) it's much more common to skip the current commit rather than some arbitrary different one, so most people will have s in their muscle memory and not S.

[jesseduffield]

Fair point: you've convinced me!

[jesseduffield]

LGTM

[Dagur]

I don't understand how to use this fix. If I press b it asks me if I want to mark the current commit as good or bad but it's always the selected commit that gets marked, not the current one.

[Dagur]

I think I found a bug because it only works after I've found both a good commit and a bad one.
If I've only found one of the two, then it suggests the selected item as the "current" commit instead of the checked out commit.

[stefanhaller]

That's not a bug, that's how bisect works. You tell it a good and a bad commit explicitly (by selecting them), and then it checks out commits for you that you test. It's only from that point on that the "current, not selected" logic makes sense.

[Dagur]

I see your point but I still think it makes sense to say that the checked out commit is the "current" one.

Here's why that would be useful to me. I check out tags to find the first good and first bad commits. When I've found a version that fits, I want to be able to mark it good or bad right away. Currently I have to either:

1. Go to the list of commits and scroll down (often for a very long time) to find the tagged commit
2. Use the command line and type "git bisect good|bad"

Another solution would be to add a bisect option to tags, I wouldn't mind that at all.

[stefanhaller]

OK, I do see two bugs here:

• if you check out the good commit and mark it "good", and then go back to the branches panel to check out a newer commit (often master), then this is not shown in the local commits panel. It still only shows the commits from the good one downwards. As a workaround, always mark the bad commit first.
• if you have only marked one commit as good or bad, the bisect dialog says "current commit", which is wrong, it should say "selected commit". (The sha in parens is the right one though.)

I agree that it might be useful to show bisect options in the tags panel (only as long as we don't have a good and a bad commit, that is). Feel free to file an issue for that.

Apart from that, I can only recommend to use the / key to find tags in the local commits panel. I use that all the time, works great.

[Dagur]

Thanks. I've created an issue here #2941

The / key doesn't search the whole history. I'm not sure how far back it goes, but often it's not enough.

[stefanhaller]

That no longer seems to be true; typing / triggers loading all commits. I'm not sure when this has changed, I remember that previously you had to type > manually to trigger loading all commits.

(Actually, I don't like this behavior; our monorepo at work is so big that loading all commits takes a really long time, and consumes so much memory that everything is super slow afterwards. I want to be able to search in the loaded commits only, and then only trigger loading more when needed.)