Skip to content

Commit 2085e9d

Browse files
committed
fix(tui): code review fixes + check overlay rerun with JSON output
- Critical: fix Watcher.Close() race condition with sync.Once - Fix renderCheckResults nil/empty ambiguity (nil = no check run yet) - Fix fsnotify error value being discarded in watcher - Add value-receiver intent comments on ExecView.Render - Add terminal compatibility comment for "\\!" key binding - Document ResolveMx lexicographic version sort limitation Check overlay improvements: - Add "r" key to rerun mx check from within the overlay - Switch from text parsing to mx check -j JSON output for richer location info (document-name, element-name, module-name) - Format locations as qualified names: MyModule.PageName (Page) - Live-update overlay content during check run (spinner → results) - Add refreshable/RefreshMsg plumbing to OverlayView and Overlay
1 parent 0e70bf8 commit 2085e9d

9 files changed

Lines changed: 332 additions & 93 deletions

File tree

cmd/mxcli/docker/check.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,11 +102,13 @@ func ResolveMx(mxbuildPath string) (string, error) {
102102
return p, nil
103103
}
104104

105-
// Try cached mxbuild installations (~/.mxcli/mxbuild/*/modeler/mx)
105+
// Try cached mxbuild installations (~/.mxcli/mxbuild/*/modeler/mx).
106+
// NOTE: lexicographic sort is imperfect for versions (e.g. "9.x" > "10.x"),
107+
// but this is a fallback-of-last-resort — in practice users typically have
108+
// only one mxbuild version installed.
106109
if home, err := os.UserHomeDir(); err == nil {
107110
matches, _ := filepath.Glob(filepath.Join(home, ".mxcli", "mxbuild", "*", "modeler", mxBinaryName()))
108111
if len(matches) > 0 {
109-
// Use the last match (highest version when sorted lexicographically)
110112
return matches[len(matches)-1], nil
111113
}
112114
}

cmd/mxcli/tui/app.go

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -463,8 +463,18 @@ func (a App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
463463
projectPath := a.activeTabProjectPath()
464464
return a, tea.Batch(a.Init(), runMxCheck(projectPath))
465465

466+
case MxCheckRerunMsg:
467+
Trace("app: manual mx check rerun requested")
468+
projectPath := a.activeTabProjectPath()
469+
return a, runMxCheck(projectPath)
470+
466471
case MxCheckStartMsg:
467472
a.checkRunning = true
473+
// Update check overlay content if it's currently visible
474+
if ov, ok := a.views.Active().(OverlayView); ok && ov.refreshable {
475+
ov.overlay.Show("mx check", CheckRunningStyle.Render("⟳ Running mx check..."), ov.overlay.width, ov.overlay.height)
476+
a.views.SetActive(ov)
477+
}
468478
return a, nil
469479

470480
case MxCheckResultMsg:
@@ -476,6 +486,12 @@ func (a App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
476486
a.checkErrors = msg.Errors
477487
Trace("app: mx check done: %d diagnostics", len(msg.Errors))
478488
}
489+
// Update check overlay content if it's currently visible
490+
if ov, ok := a.views.Active().(OverlayView); ok && ov.refreshable {
491+
content := renderCheckResults(a.checkErrors)
492+
ov.overlay.Show("mx check", content, ov.overlay.width, ov.overlay.height)
493+
a.views.SetActive(ov)
494+
}
479495
return a, nil
480496

481497
case PreviewReadyMsg, PreviewLoadingMsg, CursorChangedMsg, animTickMsg, previewDebounceMsg:
@@ -589,9 +605,13 @@ func (a *App) handleBrowserAppKeys(msg tea.KeyMsg) tea.Cmd {
589605
a.views.Push(ev)
590606
return handledCmd
591607

592-
case "!", "\\!":
608+
case "!", "\\!": // some terminals send "\\!" for shifted-1; accept both forms
593609
content := renderCheckResults(a.checkErrors)
594-
ov := NewOverlayView("mx check", content, a.width, a.height, OverlayViewOpts{HideLineNumbers: true})
610+
ov := NewOverlayView("mx check", content, a.width, a.height, OverlayViewOpts{
611+
HideLineNumbers: true,
612+
Refreshable: true,
613+
RefreshMsg: MxCheckRerunMsg{},
614+
})
595615
a.views.Push(ov)
596616
return handledCmd
597617

cmd/mxcli/tui/checker.go

Lines changed: 120 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
package tui
22

33
import (
4-
"bufio"
4+
"encoding/json"
5+
"os"
56
"os/exec"
6-
"regexp"
77
"strings"
88

99
tea "github.com/charmbracelet/bubbletea"
@@ -13,10 +13,12 @@ import (
1313

1414
// CheckError represents a single mx check diagnostic.
1515
type CheckError struct {
16-
Severity string // "ERROR" or "WARNING"
17-
Code string // e.g. "CE0001"
18-
Message string
19-
Location string // e.g. "Module.Microflow" (may be empty)
16+
Severity string // "ERROR" or "WARNING"
17+
Code string // e.g. "CE0001"
18+
Message string
19+
DocumentName string // e.g. "Page 'P_ComboBox_Enum'" (from JSON output)
20+
ElementName string // e.g. "Property 'Association' of combo box 'cmbPriority'"
21+
ModuleName string // e.g. "MyFirstModule"
2022
}
2123

2224
// MxCheckResultMsg carries the result of an async mx check run.
@@ -28,12 +30,29 @@ type MxCheckResultMsg struct {
2830
// MxCheckStartMsg signals that a check run has started.
2931
type MxCheckStartMsg struct{}
3032

31-
// checkOutputPattern matches mx check output lines like:
32-
// [error] [CE1613] "The selected association no longer exists." at Combo box 'cmbPriority'
33-
// [warning] [CW0001] "Some warning" at Page 'MyPage'
34-
var checkOutputPattern = regexp.MustCompile(`^\[(error|warning)\]\s+\[(\w+)\]\s+"(.+?)"\s+at\s+(.+?)\s*$`)
33+
// MxCheckRerunMsg requests a manual re-run of mx check (e.g. from overlay "r" key).
34+
type MxCheckRerunMsg struct{}
35+
36+
// mxCheckJSON mirrors the JSON structure produced by `mx check -j`.
37+
type mxCheckJSON struct {
38+
Errors []mxCheckEntry `json:"errors"`
39+
Warnings []mxCheckEntry `json:"warnings"`
40+
}
41+
42+
type mxCheckEntry struct {
43+
Code string `json:"code"`
44+
Message string `json:"message"`
45+
Locations []mxCheckLocation `json:"locations"`
46+
}
47+
48+
type mxCheckLocation struct {
49+
ModuleName string `json:"module-name"`
50+
DocumentName string `json:"document-name"`
51+
ElementName string `json:"element-name"`
52+
}
3553

3654
// runMxCheck returns a tea.Cmd that runs mx check asynchronously.
55+
// Uses `-j` for JSON output to get document-level location information.
3756
func runMxCheck(projectPath string) tea.Cmd {
3857
return tea.Batch(
3958
func() tea.Msg { return MxCheckStartMsg{} },
@@ -44,47 +63,82 @@ func runMxCheck(projectPath string) tea.Cmd {
4463
return MxCheckResultMsg{Err: err}
4564
}
4665

47-
Trace("checker: running %s check %s", mxPath, projectPath)
48-
cmd := exec.Command(mxPath, "check", projectPath)
49-
out, err := cmd.CombinedOutput()
50-
output := string(out)
66+
jsonFile, err := os.CreateTemp("", "mx-check-*.json")
67+
if err != nil {
68+
return MxCheckResultMsg{Err: err}
69+
}
70+
jsonPath := jsonFile.Name()
71+
jsonFile.Close()
72+
defer os.Remove(jsonPath)
73+
74+
Trace("checker: running %s check %s -j %s", mxPath, projectPath, jsonPath)
75+
cmd := exec.Command(mxPath, "check", projectPath, "-j", jsonPath)
76+
_, runErr := cmd.CombinedOutput()
77+
78+
checkErrors, parseErr := parseCheckJSON(jsonPath)
79+
if parseErr != nil {
80+
Trace("checker: JSON parse error: %v", parseErr)
81+
// If JSON parsing fails, return the run error
82+
if runErr != nil {
83+
return MxCheckResultMsg{Err: runErr}
84+
}
85+
return MxCheckResultMsg{Err: parseErr}
86+
}
5187

52-
errors := parseCheckOutput(output)
53-
Trace("checker: done, %d diagnostics, err=%v", len(errors), err)
88+
Trace("checker: done, %d diagnostics", len(checkErrors))
5489

5590
// mx check returns non-zero exit code when there are errors,
56-
// but we still want to show the parsed errors — only propagate
57-
// err if we got no parseable output at all.
58-
if err != nil && len(errors) == 0 {
59-
return MxCheckResultMsg{Err: err}
91+
// but we still want to show the parsed errors.
92+
if runErr != nil && len(checkErrors) == 0 {
93+
return MxCheckResultMsg{Err: runErr}
6094
}
61-
return MxCheckResultMsg{Errors: errors}
95+
return MxCheckResultMsg{Errors: checkErrors}
6296
},
6397
)
6498
}
6599

66-
// parseCheckOutput extracts CheckError entries from mx check stdout.
67-
func parseCheckOutput(output string) []CheckError {
68-
var errors []CheckError
69-
scanner := bufio.NewScanner(strings.NewReader(output))
70-
for scanner.Scan() {
71-
line := scanner.Text()
72-
matches := checkOutputPattern.FindStringSubmatch(line)
73-
if matches == nil {
74-
continue
75-
}
76-
errors = append(errors, CheckError{
77-
Severity: strings.ToUpper(matches[1]),
78-
Code: matches[2],
79-
Message: matches[3],
80-
Location: matches[4],
81-
})
82-
}
83-
return errors
100+
// parseCheckJSON reads the JSON file produced by `mx check -j` and converts to CheckError slice.
101+
func parseCheckJSON(jsonPath string) ([]CheckError, error) {
102+
data, err := os.ReadFile(jsonPath)
103+
if err != nil {
104+
return nil, err
105+
}
106+
107+
var result mxCheckJSON
108+
if err := json.Unmarshal(data, &result); err != nil {
109+
return nil, err
110+
}
111+
112+
var checkErrors []CheckError
113+
for _, entry := range result.Errors {
114+
checkErrors = append(checkErrors, entryToCheckError(entry, "ERROR"))
115+
}
116+
for _, entry := range result.Warnings {
117+
checkErrors = append(checkErrors, entryToCheckError(entry, "WARNING"))
118+
}
119+
return checkErrors, nil
120+
}
121+
122+
func entryToCheckError(entry mxCheckEntry, severity string) CheckError {
123+
ce := CheckError{
124+
Severity: severity,
125+
Code: entry.Code,
126+
Message: entry.Message,
127+
}
128+
if len(entry.Locations) > 0 {
129+
loc := entry.Locations[0]
130+
ce.ModuleName = loc.ModuleName
131+
ce.DocumentName = loc.DocumentName
132+
ce.ElementName = loc.ElementName
133+
}
134+
return ce
84135
}
85136

86137
// renderCheckResults formats check errors for display in an overlay.
87138
func renderCheckResults(errors []CheckError) string {
139+
if errors == nil {
140+
return "No check has been run yet. Changes to the project will trigger an automatic check."
141+
}
88142
if len(errors) == 0 {
89143
return CheckPassStyle.Render("✓ Project check passed — no errors or warnings")
90144
}
@@ -112,7 +166,7 @@ func renderCheckResults(errors []CheckError) string {
112166
sb.WriteString(strings.Join(summaryParts, " "))
113167
sb.WriteString("\n\n")
114168

115-
// Detail lines — severity+code on first line, message and location on next
169+
// Detail lines
116170
for _, e := range errors {
117171
var label string
118172
if e.Severity == "ERROR" {
@@ -122,14 +176,38 @@ func renderCheckResults(errors []CheckError) string {
122176
}
123177
sb.WriteString(label + "\n")
124178
sb.WriteString(" " + e.Message + "\n")
125-
if e.Location != "" {
126-
sb.WriteString(" " + CheckLocStyle.Render("at "+e.Location) + "\n")
179+
if e.DocumentName != "" {
180+
loc := formatDocLocation(e.ModuleName, e.DocumentName)
181+
if e.ElementName != "" {
182+
loc += " > " + e.ElementName
183+
}
184+
sb.WriteString(" " + CheckLocStyle.Render(loc) + "\n")
127185
}
128186
sb.WriteString("\n")
129187
}
130188
return sb.String()
131189
}
132190

191+
// formatDocLocation converts mx JSON document-name (e.g. "Page 'P_ComboBox'")
192+
// into a qualified name like "MyModule.P_ComboBox (Page)".
193+
func formatDocLocation(moduleName, documentName string) string {
194+
// documentName format: "Type 'Name'" — extract type and name
195+
if idx := strings.Index(documentName, " '"); idx > 0 {
196+
docType := documentName[:idx]
197+
docName := strings.TrimSuffix(documentName[idx+2:], "'")
198+
qname := docName
199+
if moduleName != "" {
200+
qname = moduleName + "." + docName
201+
}
202+
return qname + " (" + docType + ")"
203+
}
204+
// Fallback: just prefix with module
205+
if moduleName != "" {
206+
return moduleName + "." + documentName
207+
}
208+
return documentName
209+
}
210+
133211
// formatCheckBadge returns a compact badge string for the status bar.
134212
func formatCheckBadge(errors []CheckError, running bool) string {
135213
if running {
@@ -158,4 +236,3 @@ func formatCheckBadge(errors []CheckError, running bool) string {
158236
}
159237
return strings.Join(parts, " ")
160238
}
161-

0 commit comments

Comments
 (0)