Keep selected commits stable across refreshes#5717
Merged
Conversation
This guards against regressions from the changes that follow. We're about to add a mechanism that keeps the selection anchored by commit hash, but we need to make sure that it doesn't take effect here; after a merge we want to select the newly added merge commit. In the current state of the code this happens to work because we keep the selection index the same, which happened to be 0 here; later we will change this to explicitly select the head commit after the merge.
Not used yet, we'll need it in the next commit.
This makes the following diff a little easier to read.
Preparation for the next commit, which selects the newly created commit after a commit succeeds, while leaving the selection alone on failure. For now success and failure use the same refresh options, so behavior is unchanged.
87da619 to
2c2a424
Compare
With the recently added external change detection, it happens more often now that we refresh the commits list because an agent made a commit in the background. In this case, if we keep the selection index the same, it now points at a different commit, making the main view show a different commit too, which is confusing and annoying. To fix this, track the selected commit and range anchor by hash before reloading, then restore those rows if both hashes still exist. This also allows us to get rid of some bespoke code that did this for the specific cases of reverting a commit or cherry-picking commits, because those are now handled by the generic mechanism.
When restoring the commit selection after a refresh we match by hash and TODO status. The TODO status is part of the match so that a commit being reverted or cherry-picked is matched to the real commit rather than to the rebase TODO entry that shares its hash. But a selected commit can also change its TODO status across a refresh: when starting an interactive rebase that stops to edit it, the real commit becomes a TODO entry. Fall back to matching by hash alone when there is no exact match, so the selection is still restored in that case. The next commit relies on this to remove bespoke selection-restoration code in the local commits controller that matched by hash alone, which the generic mechanism otherwise wouldn't fully replace.
Starting an interactive rebase (the `edit` command and quick-start) used to capture the selected commit range by hash before starting the rebase and restore it afterwards, because new update-ref lines for stacked branches can shift the commits' positions in the list. The generic keep-selection-by-hash mechanism now does exactly this for every refresh, including these, so the bespoke code is redundant. This relies on the previous commit, which taught the generic matcher to handle the case where the selected commit turns into a rebase TODO entry while it's being edited - something the bespoke code handled implicitly by matching on hash alone.
2c2a424 to
7f96c8f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With the recently added external change detection, it happens more often now that we refresh the commits list because an agent made a commit in the background. In this case, if we keep the selection index the same, it now points at a different commit, making the main view show a different commit too, which is confusing and annoying. To fix this, track the selected commit and range anchor by hash before reloading, then restore those rows if both hashes still exist. This also allows us to get rid of some bespoke code that did this for the specific cases of reverting a commit or cherry-picking commits, because those are now handled by the generic mechanism.