Skip to content

Commit 8d8bdb9

Browse files
jesseduffieldjesseduffield
committed
avoid deadlock in merge panel
1 parent cdcfeb3 commit 8d8bdb9

File tree

9 files changed

+79
-59
lines changed

9 files changed

+79
-59
lines changed

‎pkg/gui/diff_context_size.go‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,25 @@ import (
44
"errors"
55
)
66

7+
var CONTEXT_KEYS_SHOWING_DIFFS = []ContextKey{
8+
FILES_CONTEXT_KEY,
9+
COMMIT_FILES_CONTEXT_KEY,
10+
STASH_CONTEXT_KEY,
11+
BRANCH_COMMITS_CONTEXT_KEY,
12+
SUB_COMMITS_CONTEXT_KEY,
13+
MAIN_STAGING_CONTEXT_KEY,
14+
MAIN_PATCH_BUILDING_CONTEXT_KEY,
15+
}
16+
717
func isShowingDiff(gui *Gui) bool {
818
key := gui.currentStaticContext().GetKey()
919

10-
return key == FILES_CONTEXT_KEY || key == COMMIT_FILES_CONTEXT_KEY || key == STASH_CONTEXT_KEY || key == BRANCH_COMMITS_CONTEXT_KEY || key == SUB_COMMITS_CONTEXT_KEY || key == MAIN_STAGING_CONTEXT_KEY || key == MAIN_PATCH_BUILDING_CONTEXT_KEY
20+
for _, contextKey := range CONTEXT_KEYS_SHOWING_DIFFS {
21+
if key == contextKey {
22+
return true
23+
}
24+
}
25+
return false
1126
}
1227

1328
func (gui *Gui) IncreaseContextInDiffView() error {

‎pkg/gui/diff_context_size_test.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ func TestDoesntIncreaseContextInDiffViewInContextWithoutDiff(t *testing.T) {
6565
func(gui *Gui) Context { return gui.State.Contexts.ReflogCommits },
6666
func(gui *Gui) Context { return gui.State.Contexts.RemoteBranches },
6767
func(gui *Gui) Context { return gui.State.Contexts.Tags },
68-
func(gui *Gui) Context { return gui.State.Contexts.Merging },
68+
// not testing this because it will kick straight back to the files context
69+
// upon pushing the context
70+
// func(gui *Gui) Context { return gui.State.Contexts.Merging },
6971
func(gui *Gui) Context { return gui.State.Contexts.CommandLog },
7072
}
7173

@@ -115,7 +117,9 @@ func TestDoesntDecreaseContextInDiffViewInContextWithoutDiff(t *testing.T) {
115117
func(gui *Gui) Context { return gui.State.Contexts.ReflogCommits },
116118
func(gui *Gui) Context { return gui.State.Contexts.RemoteBranches },
117119
func(gui *Gui) Context { return gui.State.Contexts.Tags },
118-
func(gui *Gui) Context { return gui.State.Contexts.Merging },
120+
// not testing this because it will kick straight back to the files context
121+
// upon pushing the context
122+
// func(gui *Gui) Context { return gui.State.Contexts.Merging },
119123
func(gui *Gui) Context { return gui.State.Contexts.CommandLog },
120124
}
121125

‎pkg/gui/files_panel.go‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func (gui *Gui) filesRenderToMain() error {
5454
}
5555

5656
if node.File != nil && node.File.HasInlineMergeConflicts {
57-
return gui.refreshMergePanelWithLock()
57+
return gui.renderConflictsFromFilesPanel()
5858
}
5959

6060
cmdObj := gui.Git.WorkingTree.WorktreeFileDiffCmdObj(node, false, !node.GetHasUnstagedChanges() && node.GetHasStagedChanges(), gui.State.IgnoreWhitespaceInDiffView)
@@ -182,7 +182,7 @@ func (gui *Gui) enterFile(opts OnFocusOpts) error {
182182
}
183183

184184
if file.HasInlineMergeConflicts {
185-
return gui.handleSwitchToMerge()
185+
return gui.switchToMerge()
186186
}
187187
if file.HasMergeConflicts {
188188
return gui.createErrorPanel(gui.Tr.FileStagingRequirements)
@@ -201,7 +201,7 @@ func (gui *Gui) handleFilePress() error {
201201
file := node.File
202202

203203
if file.HasInlineMergeConflicts {
204-
return gui.handleSwitchToMerge()
204+
return gui.switchToMerge()
205205
}
206206

207207
if file.HasUnstagedChanges {
@@ -880,16 +880,12 @@ func (gui *Gui) upstreamForBranchInConfig(branchName string) (string, string, er
880880
return "", "", nil
881881
}
882882

883-
func (gui *Gui) handleSwitchToMerge() error {
883+
func (gui *Gui) switchToMerge() error {
884884
file := gui.getSelectedFile()
885885
if file == nil {
886886
return nil
887887
}
888888

889-
if !file.HasInlineMergeConflicts {
890-
return gui.createErrorPanel(gui.Tr.FileNoMergeCons)
891-
}
892-
893889
return gui.pushContext(gui.State.Contexts.Merging)
894890
}
895891

‎pkg/gui/menu_panel.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func (gui *Gui) getMenuOptions() map[string]string {
3939
}
4040

4141
func (gui *Gui) handleMenuClose() error {
42-
return gui.returnFromContextSync()
42+
return gui.returnFromContext()
4343
}
4444

4545
type createMenuOptions struct {

‎pkg/gui/merge_panel.go‎

Lines changed: 52 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ func (gui *Gui) handlePickHunk() error {
9090
if err := gui.handleCompleteMerge(); err != nil {
9191
return err
9292
}
93+
return nil
9394
}
9495
return gui.refreshMergePanel()
9596
})
@@ -151,32 +152,40 @@ func (gui *Gui) refreshMergePanelWithLock() error {
151152
return gui.withMergeConflictLock(gui.refreshMergePanel)
152153
}
153154

154-
func (gui *Gui) refreshMergePanel() error {
155-
panelState := gui.State.Panels.Merging
155+
// not re-using state here because we can run into issues with mutexes when
156+
// doing that.
157+
func (gui *Gui) renderConflictsFromFilesPanel() error {
158+
state := mergeconflicts.NewState()
159+
_, err := gui.renderConflicts(state, false)
160+
161+
return err
162+
}
163+
164+
func (gui *Gui) renderConflicts(state *mergeconflicts.State, hasFocus bool) (bool, error) {
156165
cat, err := gui.catSelectedFile()
157166
if err != nil {
158-
return gui.refreshMainViews(refreshMainOpts{
167+
return false, gui.refreshMainViews(refreshMainOpts{
159168
main: &viewUpdateOpts{
160169
title: "",
161170
task: NewRenderStringTask(err.Error()),
162171
},
163172
})
164173
}
165174

166-
panelState.SetConflictsFromCat(cat)
175+
state.SetConflictsFromCat(cat)
167176

168-
if panelState.NoConflicts() {
169-
return gui.handleCompleteMerge()
177+
if state.NoConflicts() {
178+
// we shouldn't end up here
179+
return false, nil
170180
}
171181

172-
hasFocus := gui.currentViewName() == "main"
173-
content := mergeconflicts.ColoredConflictFile(cat, panelState.State, hasFocus)
182+
content := mergeconflicts.ColoredConflictFile(cat, state, hasFocus)
174183

175-
if err := gui.scrollToConflict(); err != nil {
176-
return err
184+
if !gui.State.Panels.Merging.UserVerticalScrolling {
185+
gui.centerYPos(gui.Views.Main, state.GetConflictMiddle())
177186
}
178187

179-
return gui.refreshMainViews(refreshMainOpts{
188+
return true, gui.refreshMainViews(refreshMainOpts{
180189
main: &viewUpdateOpts{
181190
title: gui.Tr.MergeConflictsTitle,
182191
task: NewRenderStringWithoutScrollTask(content),
@@ -185,6 +194,19 @@ func (gui *Gui) refreshMergePanel() error {
185194
})
186195
}
187196

197+
func (gui *Gui) refreshMergePanel() error {
198+
conflictsFound, err := gui.renderConflicts(gui.State.Panels.Merging.State, true)
199+
if err != nil {
200+
return err
201+
}
202+
203+
if !conflictsFound {
204+
return gui.handleCompleteMerge()
205+
}
206+
207+
return nil
208+
}
209+
188210
func (gui *Gui) catSelectedFile() (string, error) {
189211
item := gui.getSelectedFile()
190212
if item == nil {
@@ -203,21 +225,6 @@ func (gui *Gui) catSelectedFile() (string, error) {
203225
return cat, nil
204226
}
205227

206-
func (gui *Gui) scrollToConflict() error {
207-
if gui.State.Panels.Merging.UserVerticalScrolling {
208-
return nil
209-
}
210-
211-
panelState := gui.State.Panels.Merging
212-
if panelState.NoConflicts() {
213-
return nil
214-
}
215-
216-
gui.centerYPos(gui.Views.Main, panelState.GetConflictMiddle())
217-
218-
return nil
219-
}
220-
221228
func (gui *Gui) centerYPos(view *gocui.View, y int) {
222229
ox, _ := view.Origin()
223230
_, height := view.Size()
@@ -238,18 +245,11 @@ func (gui *Gui) getMergingOptions() map[string]string {
238245
}
239246

240247
func (gui *Gui) handleEscapeMerge() error {
241-
gui.takeOverMergeConflictScrolling()
242-
243-
gui.State.Panels.Merging.Reset()
244248
if err := gui.refreshSidePanels(refreshOptions{scope: []RefreshableView{FILES}}); err != nil {
245249
return err
246250
}
247-
// it's possible this method won't be called from the merging view so we need to
248-
// ensure we only 'return' focus if we already have it
249-
if gui.g.CurrentView() == gui.Views.Main {
250-
return gui.pushContext(gui.State.Contexts.Files)
251-
}
252-
return nil
251+
252+
return gui.escapeMerge()
253253
}
254254

255255
func (gui *Gui) handleCompleteMerge() error {
@@ -259,16 +259,26 @@ func (gui *Gui) handleCompleteMerge() error {
259259
if err := gui.refreshSidePanels(refreshOptions{scope: []RefreshableView{FILES}}); err != nil {
260260
return err
261261
}
262-
// if we got conflicts after unstashing, we don't want to call any git
263-
// commands to continue rebasing/merging here
264-
if gui.Git.Status.WorkingTreeState() == enums.REBASE_MODE_NONE {
265-
return gui.handleEscapeMerge()
266-
}
262+
267263
// if there are no more files with merge conflicts, we should ask whether the user wants to continue
268-
if !gui.anyFilesWithMergeConflicts() {
264+
if gui.Git.Status.WorkingTreeState() != enums.REBASE_MODE_NONE && !gui.anyFilesWithMergeConflicts() {
269265
return gui.promptToContinueRebase()
270266
}
271-
return gui.handleEscapeMerge()
267+
268+
return gui.escapeMerge()
269+
}
270+
271+
func (gui *Gui) escapeMerge() error {
272+
gui.takeOverMergeConflictScrolling()
273+
274+
gui.State.Panels.Merging.Reset()
275+
276+
// it's possible this method won't be called from the merging view so we need to
277+
// ensure we only 'return' focus if we already have it
278+
if gui.g.CurrentView() == gui.Views.Main {
279+
return gui.pushContext(gui.State.Contexts.Files)
280+
}
281+
return nil
272282
}
273283

274284
// promptToContinueRebase asks the user if they want to continue the rebase/merge that's in progress

‎pkg/i18n/chinese.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ func chineseTranslationSet() TranslationSet {
7373
PullWait: "拉取中……",
7474
PushWait: "推送中……",
7575
FetchWait: "正在抓取……",
76-
FileNoMergeCons: "该文件没有合并冲突",
7776
LcSoftReset: "软重置",
7877
AlreadyCheckedOutBranch: "您已经检出了这个分支",
7978
SureForceCheckout: "您确定要强制检出吗？您将丢失所有本地更改",

‎pkg/i18n/dutch.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ func dutchTranslationSet() TranslationSet {
4444
PullWait: "Pullen...",
4545
PushWait: "Pushen...",
4646
FetchWait: "Fetchen...",
47-
FileNoMergeCons: "Dit bestand heeft geen merge conflicten",
4847
LcSoftReset: "zacht reset",
4948
AlreadyCheckedOutBranch: "Je hebt deze branch al uitgecheckt",
5049
SureForceCheckout: "Weet je zeker dat je het uitchecken wil forceren? Al je lokale verandering zullen worden verwijdert",

‎pkg/i18n/english.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ type TranslationSet struct {
5858
PullWait string
5959
PushWait string
6060
FetchWait string
61-
FileNoMergeCons string
6261
LcSoftReset string
6362
AlreadyCheckedOutBranch string
6463
SureForceCheckout string
@@ -608,7 +607,6 @@ func EnglishTranslationSet() TranslationSet {
608607
PullWait: "Pulling...",
609608
PushWait: "Pushing...",
610609
FetchWait: "Fetching...",
611-
FileNoMergeCons: "This file has no inline merge conflicts",
612610
LcSoftReset: "soft reset",
613611
AlreadyCheckedOutBranch: "You have already checked out this branch",
614612
SureForceCheckout: "Are you sure you want force checkout? You will lose all local changes",

‎pkg/i18n/polish.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ func polishTranslationSet() TranslationSet {
3939
PullWait: "Pobieranie zmian...",
4040
PushWait: "Wysyłanie zmian...",
4141
FetchWait: "Pobieram...",
42-
FileNoMergeCons: "Brak konfliktów scalania w pliku",
4342
AlreadyCheckedOutBranch: "Już przęłączono na tą gałąź",
4443
SureForceCheckout: "Jesteś pewny, że chcesz wymusić przełączenie? Stracisz wszystkie lokalne zmiany",
4544
ForceCheckoutBranch: "Wymuś przełączenie gałęzi",

0 commit comments

Comments
 (0)