Skip to content

Commit 0e70bf8

Browse files
engalarengalar
committed
fix(tui): address PR #27 review — overlay switching, search, dead code cleanup
- Critical: fix overlay tab switching by passing OverlayViewOpts through OpenOverlayMsg - Critical: fix PlainDiff search by making buildMatchLines mode-aware - Critical: fix compare hint key d → D - Moderate: fix CJK wide-char handling in hslice/truncateToWidth with runewidth - Moderate: replace 10 nil-cmd goroutines with shared handledCmd variable - Moderate: remove redundant CompareView.visible field (ViewStack handles visibility) - Dead code: remove overlayQName/overlayNodeType/overlayIsNDSL fields - Dead code: remove unreachable "c"/"r" key cases in BrowserView.handleKey - Dead code: remove unused compareItems field and assignments - Minor: extract magic numbers to horizontalScrollStep / previewDebounceDelay constants - Minor: fix pluralization bug ("indexs" → "indexes") with irregularPlurals map - Minor: deduplicate loadBsonNDSL into shared package-level function
1 parent 6e0bdfd commit 0e70bf8

8 files changed

Lines changed: 100 additions & 101 deletions

File tree

‎cmd/mxcli/tui/app.go‎

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ import (
1414
// chromeHeight is the vertical space consumed by tab bar (1) + hint bar (1) + status bar (1).
1515
const chromeHeight = 3
1616

17+
// handledCmd is returned by handleBrowserAppKeys to signal that a key was
18+
// consumed without producing a follow-up message. Using a shared variable
19+
// avoids allocating a new closure on every handled keystroke.
20+
var handledCmd tea.Cmd = func() tea.Msg { return nil }
21+
1722
// compareFlashClearMsg is sent 1 s after a clipboard copy in compare view.
1823
type compareFlashClearMsg struct{}
1924

@@ -118,7 +123,6 @@ func (a *App) syncBrowserView() {
118123
}
119124
bv := NewBrowserView(tab, a.mxcliPath, a.previewEngine)
120125
bv.allNodes = tab.AllNodes
121-
bv.compareItems = flattenQualifiedNames(tab.AllNodes)
122126
// Ensure miller has current dimensions so scroll calculations in
123127
// Update() work correctly (Render operates on a value copy).
124128
if a.height > 0 {
@@ -162,7 +166,7 @@ func (a App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
162166

163167
// --- View creation messages ---
164168
case OpenOverlayMsg:
165-
ov := NewOverlayView(msg.Title, msg.Content, a.width, a.height, OverlayViewOpts{})
169+
ov := NewOverlayView(msg.Title, msg.Content, a.width, a.height, msg.Opts)
166170
a.views.Push(ov)
167171
return a, nil
168172

@@ -442,7 +446,6 @@ func (a App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
442446
// Update browser view if it's the base
443447
if bv, ok := a.views.Base().(BrowserView); ok {
444448
bv.allNodes = msg.Nodes
445-
bv.compareItems = flattenQualifiedNames(msg.Nodes)
446449
bv.miller = tab.Miller
447450
if a.height > 0 {
448451
contentH := max(5, a.height-chromeHeight)
@@ -524,14 +527,14 @@ func (a *App) handleBrowserAppKeys(msg tea.KeyMsg) tea.Cmd {
524527
a.syncBrowserView()
525528
a.syncTabBar()
526529
}
527-
return func() tea.Msg { return nil }
530+
return handledCmd
528531

529532
case "T":
530533
p := NewEmbeddedPicker()
531534
p.width = a.width
532535
p.height = a.height
533536
a.picker = &p
534-
return func() tea.Msg { return nil }
537+
return handledCmd
535538

536539
case "W":
537540
if len(a.tabs) > 1 {
@@ -543,7 +546,7 @@ func (a *App) handleBrowserAppKeys(msg tea.KeyMsg) tea.Cmd {
543546
a.syncBrowserView()
544547
a.syncTabBar()
545548
}
546-
return func() tea.Msg { return nil }
549+
return handledCmd
547550

548551
case "1", "2", "3", "4", "5", "6", "7", "8", "9":
549552
idx := int(msg.String()[0]-'0') - 1
@@ -552,23 +555,23 @@ func (a *App) handleBrowserAppKeys(msg tea.KeyMsg) tea.Cmd {
552555
a.syncBrowserView()
553556
a.syncTabBar()
554557
}
555-
return func() tea.Msg { return nil }
558+
return handledCmd
556559

557560
case "[":
558561
if a.activeTab > 0 {
559562
a.activeTab--
560563
a.syncBrowserView()
561564
a.syncTabBar()
562565
}
563-
return func() tea.Msg { return nil }
566+
return handledCmd
564567

565568
case "]":
566569
if a.activeTab < len(a.tabs)-1 {
567570
a.activeTab++
568571
a.syncBrowserView()
569572
a.syncTabBar()
570573
}
571-
return func() tea.Msg { return nil }
574+
return handledCmd
572575

573576
case "r":
574577
return a.Init()
@@ -579,18 +582,18 @@ func (a *App) handleBrowserAppKeys(msg tea.KeyMsg) tea.Cmd {
579582
jumper := NewJumperView(items, a.width, a.height)
580583
a.views.Push(jumper)
581584
}
582-
return func() tea.Msg { return nil }
585+
return handledCmd
583586

584587
case "x":
585588
ev := NewExecView(a.mxcliPath, a.activeTabProjectPath(), a.width, a.height)
586589
a.views.Push(ev)
587-
return func() tea.Msg { return nil }
590+
return handledCmd
588591

589592
case "!", "\\!":
590593
content := renderCheckResults(a.checkErrors)
591594
ov := NewOverlayView("mx check", content, a.width, a.height, OverlayViewOpts{HideLineNumbers: true})
592595
a.views.Push(ov)
593-
return func() tea.Msg { return nil }
596+
return handledCmd
594597

595598
case "c":
596599
cv := NewCompareView()
@@ -606,7 +609,7 @@ func (a *App) handleBrowserAppKeys(msg tea.KeyMsg) tea.Cmd {
606609
}
607610
}
608611
a.views.Push(cv)
609-
return func() tea.Msg { return nil }
612+
return handledCmd
610613
}
611614

612615
return nil
@@ -740,6 +743,12 @@ type CmdResultMsg struct {
740743
Err error
741744
}
742745

746+
// irregularPlurals maps singular type names to their correct plural forms
747+
// for types where simply appending "s" produces incorrect English.
748+
var irregularPlurals = map[string]string{
749+
"Index": "indexes",
750+
}
751+
743752
// renderContextSummary counts top-level node types and returns a compact summary.
744753
func renderContextSummary(nodes []*TreeNode) string {
745754
if len(nodes) == 0 {
@@ -772,7 +781,11 @@ func renderContextSummary(nodes []*TreeNode) string {
772781
// Add remaining types not in the predefined order
773782
for k, c := range counts {
774783
if !used[k] {
775-
parts = append(parts, fmt.Sprintf("%d %s", c, strings.ToLower(k)+"s"))
784+
plural, ok := irregularPlurals[k]
785+
if !ok {
786+
plural = strings.ToLower(k) + "s"
787+
}
788+
parts = append(parts, fmt.Sprintf("%d %s", c, plural))
776789
}
777790
}
778791
if len(parts) > 3 {
@@ -797,3 +810,24 @@ func inferBsonType(nodeType string) string {
797810
return ""
798811
}
799812
}
813+
814+
// loadBsonNDSL runs mxcli bson dump in NDSL format and returns a CompareLoadMsg.
815+
// Shared by BrowserView and CompareView to avoid duplicate implementations.
816+
func loadBsonNDSL(mxcliPath, projectPath, qname, nodeType string, side CompareFocus) tea.Cmd {
817+
return func() tea.Msg {
818+
bsonType := inferBsonType(nodeType)
819+
if bsonType == "" {
820+
return CompareLoadMsg{Side: side, Title: qname, NodeType: nodeType,
821+
Content: fmt.Sprintf("Error: type %q not supported for BSON dump", nodeType),
822+
Err: fmt.Errorf("unsupported type")}
823+
}
824+
args := []string{"bson", "dump", "-p", projectPath, "--format", "ndsl",
825+
"--type", bsonType, "--object", qname}
826+
out, err := runMxcli(mxcliPath, args...)
827+
out = StripBanner(out)
828+
if err != nil {
829+
return CompareLoadMsg{Side: side, Title: qname, NodeType: nodeType, Content: "Error: " + out, Err: err}
830+
}
831+
return CompareLoadMsg{Side: side, Title: qname, NodeType: nodeType, Content: HighlightNDSL(out)}
832+
}
833+
}

‎cmd/mxcli/tui/browserview.go‎

Lines changed: 20 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,10 @@ type BrowserView struct {
1616
miller MillerView
1717
tab *Tab
1818
allNodes []*TreeNode
19-
compareItems []PickerItem
2019
mxcliPath string
2120
projectPath string
2221
previewEngine *PreviewEngine
2322

24-
// Overlay state for NDSL/MDL tab switching context
25-
overlayQName string
26-
overlayNodeType string
27-
overlayIsNDSL bool
2823
}
2924

3025
// NewBrowserView creates a BrowserView wrapping the Miller view from the given tab.
@@ -125,32 +120,18 @@ func (bv BrowserView) handleKey(msg tea.KeyMsg) (View, tea.Cmd) {
125120
node := bv.miller.SelectedNode()
126121
if node != nil && node.QualifiedName != "" {
127122
if bsonType := inferBsonType(node.Type); bsonType != "" {
128-
bv.overlayQName = node.QualifiedName
129-
bv.overlayNodeType = node.Type
130-
bv.overlayIsNDSL = true
131-
return bv, bv.runBsonOverlay(bsonType, node.QualifiedName)
123+
return bv, bv.runBsonOverlay(bsonType, node.QualifiedName, node.Type)
132124
}
133125
}
134126
return bv, nil
135127

136128
case "m":
137129
node := bv.miller.SelectedNode()
138130
if node != nil && node.QualifiedName != "" {
139-
bv.overlayQName = node.QualifiedName
140-
bv.overlayNodeType = node.Type
141-
bv.overlayIsNDSL = false
142131
return bv, bv.runMDLOverlay(node.Type, node.QualifiedName)
143132
}
144133
return bv, nil
145134

146-
case "c":
147-
node := bv.miller.SelectedNode()
148-
var loadCmd tea.Cmd
149-
if node != nil && node.QualifiedName != "" {
150-
loadCmd = bv.loadBsonNDSL(node.QualifiedName, node.Type, CompareFocusLeft)
151-
}
152-
return bv, loadCmd
153-
154135
case "d":
155136
node := bv.miller.SelectedNode()
156137
if node != nil && node.QualifiedName != "" {
@@ -165,10 +146,6 @@ func (bv BrowserView) handleKey(msg tea.KeyMsg) (View, tea.Cmd) {
165146
}
166147
return bv, nil
167148

168-
case "r":
169-
// Return nil — App handles refresh via Init()
170-
return bv, nil
171-
172149
case "z":
173150
bv.miller.zenMode = !bv.miller.zenMode
174151
bv.miller.relayout()
@@ -190,55 +167,51 @@ func (bv BrowserView) handleKey(msg tea.KeyMsg) (View, tea.Cmd) {
190167

191168
// --- Load helpers (moved from app.go) ---
192169

193-
func (bv BrowserView) runBsonOverlay(bsonType, qname string) tea.Cmd {
170+
func (bv BrowserView) overlayOpts(qname, nodeType string, isNDSL bool) OverlayViewOpts {
171+
return OverlayViewOpts{
172+
QName: qname,
173+
NodeType: nodeType,
174+
IsNDSL: isNDSL,
175+
Switchable: true,
176+
MxcliPath: bv.mxcliPath,
177+
ProjectPath: bv.projectPath,
178+
}
179+
}
180+
181+
func (bv BrowserView) runBsonOverlay(bsonType, qname, nodeType string) tea.Cmd {
194182
mxcliPath := bv.mxcliPath
195183
projectPath := bv.projectPath
184+
opts := bv.overlayOpts(qname, nodeType, true)
196185
return func() tea.Msg {
197186
args := []string{"bson", "dump", "-p", projectPath, "--format", "ndsl",
198187
"--type", bsonType, "--object", qname}
199188
out, err := runMxcli(mxcliPath, args...)
200189
out = StripBanner(out)
201190
title := fmt.Sprintf("BSON: %s", qname)
202191
if err != nil {
203-
return OpenOverlayMsg{Title: title, Content: "Error: " + out}
192+
return OpenOverlayMsg{Title: title, Content: "Error: " + out, Opts: opts}
204193
}
205-
return OpenOverlayMsg{Title: title, Content: HighlightNDSL(out)}
194+
return OpenOverlayMsg{Title: title, Content: HighlightNDSL(out), Opts: opts}
206195
}
207196
}
208197

209198
func (bv BrowserView) runMDLOverlay(nodeType, qname string) tea.Cmd {
210199
mxcliPath := bv.mxcliPath
211200
projectPath := bv.projectPath
201+
opts := bv.overlayOpts(qname, nodeType, false)
212202
return func() tea.Msg {
213203
out, err := runMxcli(mxcliPath, "-p", projectPath, "-c", buildDescribeCmd(nodeType, qname))
214204
out = StripBanner(out)
215205
title := fmt.Sprintf("MDL: %s", qname)
216206
if err != nil {
217-
return OpenOverlayMsg{Title: title, Content: "Error: " + out}
207+
return OpenOverlayMsg{Title: title, Content: "Error: " + out, Opts: opts}
218208
}
219-
return OpenOverlayMsg{Title: title, Content: DetectAndHighlight(out)}
209+
return OpenOverlayMsg{Title: title, Content: DetectAndHighlight(out), Opts: opts}
220210
}
221211
}
222212

223213
func (bv BrowserView) loadBsonNDSL(qname, nodeType string, side CompareFocus) tea.Cmd {
224-
mxcliPath := bv.mxcliPath
225-
projectPath := bv.projectPath
226-
return func() tea.Msg {
227-
bsonType := inferBsonType(nodeType)
228-
if bsonType == "" {
229-
return CompareLoadMsg{Side: side, Title: qname, NodeType: nodeType,
230-
Content: fmt.Sprintf("Error: type %q not supported for BSON dump", nodeType),
231-
Err: fmt.Errorf("unsupported type")}
232-
}
233-
args := []string{"bson", "dump", "-p", projectPath, "--format", "ndsl",
234-
"--type", bsonType, "--object", qname}
235-
out, err := runMxcli(mxcliPath, args...)
236-
out = StripBanner(out)
237-
if err != nil {
238-
return CompareLoadMsg{Side: side, Title: qname, NodeType: nodeType, Content: "Error: " + out, Err: err}
239-
}
240-
return CompareLoadMsg{Side: side, Title: qname, NodeType: nodeType, Content: HighlightNDSL(out)}
241-
}
214+
return loadBsonNDSL(bv.mxcliPath, bv.projectPath, qname, nodeType, side)
242215
}
243216

244217
// navigateToNode resets the miller view to root and drills down to the node

‎cmd/mxcli/tui/compare.go‎

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ func (p comparePane) lineInfo() string {
7474

7575
// CompareView is a side-by-side comparison overlay (lazygit-style).
7676
type CompareView struct {
77-
visible bool
7877
kind CompareKind
7978
focus CompareFocus
8079
left comparePane
@@ -107,7 +106,6 @@ func NewCompareView() CompareView {
107106
}
108107

109108
func (c *CompareView) Show(kind CompareKind, w, h int) {
110-
c.visible = true
111109
c.kind = kind
112110
c.focus = CompareFocusLeft
113111
c.width = w
@@ -182,10 +180,6 @@ func (c *CompareView) closePicker() { c.picker = false; c.pickerInput.Blur() }
182180
// --- Update ---
183181

184182
func (c CompareView) updateInternal(msg tea.Msg) (CompareView, tea.Cmd) {
185-
if !c.visible {
186-
return c, nil
187-
}
188-
189183
switch msg := msg.(type) {
190184
case tea.KeyMsg:
191185
if c.picker {
@@ -247,7 +241,6 @@ func (c CompareView) updateNormal(msg tea.KeyMsg) (CompareView, tea.Cmd) {
247241

248242
switch msg.String() {
249243
case "esc", "q":
250-
c.visible = false
251244
return c, func() tea.Msg { return PopViewMsg{} }
252245

253246
// Focus switching — lazygit style: Tab only
@@ -357,10 +350,6 @@ func (c *CompareView) syncOtherPane() {
357350
// --- View ---
358351

359352
func (c CompareView) View() string {
360-
if !c.visible {
361-
return ""
362-
}
363-
364353
pw, _ := c.paneDimensions()
365354

366355
// Pane rendering
@@ -598,24 +587,7 @@ func (c CompareView) Mode() ViewMode {
598587

599588
// loadBsonNDSL loads BSON NDSL content for a compare pane.
600589
func (c CompareView) loadBsonNDSL(qname, nodeType string, side CompareFocus) tea.Cmd {
601-
mxcliPath := c.mxcliPath
602-
projectPath := c.projectPath
603-
return func() tea.Msg {
604-
bsonType := inferBsonType(nodeType)
605-
if bsonType == "" {
606-
return CompareLoadMsg{Side: side, Title: qname, NodeType: nodeType,
607-
Content: fmt.Sprintf("Error: type %q not supported for BSON dump", nodeType),
608-
Err: fmt.Errorf("unsupported type")}
609-
}
610-
args := []string{"bson", "dump", "-p", projectPath, "--format", "ndsl",
611-
"--type", bsonType, "--object", qname}
612-
out, err := runMxcli(mxcliPath, args...)
613-
out = StripBanner(out)
614-
if err != nil {
615-
return CompareLoadMsg{Side: side, Title: qname, NodeType: nodeType, Content: "Error: " + out, Err: err}
616-
}
617-
return CompareLoadMsg{Side: side, Title: qname, NodeType: nodeType, Content: HighlightNDSL(out)}
618-
}
590+
return loadBsonNDSL(c.mxcliPath, c.projectPath, qname, nodeType, side)
619591
}
620592

621593
// loadMDL loads MDL content for a compare pane.

0 commit comments

Comments
 (0)