Fix panic in custom patch actions when the patch's commit is no longer in the view - #5865
Open
faizanmohiuddin482 wants to merge 1 commit into
Conversation
When a custom patch is built from a commit and you then check out a branch that doesn't contain that commit, getPatchCommitIndex() returns -1, which was passed straight into commits[commitIndex], panicking with 'index out of range [-1]' (jesseduffield#5802). This affected all five custom-patch menu actions. Make getPatchCommitIndex() return an error when the patch's commit is no longer present, and have each action surface it as a friendly message instead of crashing. Add an integration test reproducing the scenario.
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
1 participant
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.
Fixes #5802.
When you build a custom patch from a commit and then check out a branch that doesn't contain that commit,
getPatchCommitIndex()returns-1, which was passed straight intocommits[commitIndex], causingpanic: runtime error: index out of range [-1].The reported repro is "Move patch out into index", but the same
-1reaches all five custom-patch menu actions (delete from commit, move to selected commit, move into index, and both pull-into-new-commit actions).Fix
getPatchCommitIndex()now returns an error when the patch's commit can't be found in the current commits list, and each action surfaces it as a friendly message instead of crashing:Test
Added an integration test (
MoveToIndexWhenCommitNotInCurrentBranch) that builds a patch, checks out a branch without the source commit, and moves the patch into the index. It panics withindex out of range [-1]onmasterand passes with this change.Note
PatchBuildingController.getPatchCommitIndex()has the samereturn -1pattern and could be guarded similarly; I kept this PR scoped to the reported crash, but happy to cover it too if you'd like.