Skip to content

Commit 482b01d

Browse files
akoako
authored
Merge pull request #184 from engalar/fix/pr168-review-feedback
fix: improve data container context hints and LSP completion
2 parents 9e83ea7 + 76ba6fb commit 482b01d

5 files changed

Lines changed: 371 additions & 42 deletions

File tree

‎cmd/mxcli/lsp_completion.go‎

Lines changed: 158 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func (s *mdlServer) Completion(ctx context.Context, params *protocol.CompletionP
3333

3434
// Check if typing a $ variable reference inside page/snippet context
3535
if strings.Contains(linePrefix, "$") {
36-
varItems := s.variableCompletionItems(text, linePrefix)
36+
varItems := s.variableCompletionItems(text, linePrefix, int(params.Position.Line))
3737
if len(varItems) > 0 {
3838
return &protocol.CompletionList{
3939
IsIncomplete: false,
@@ -373,9 +373,10 @@ func objectTypeToCompletionKind(objectType string) (protocol.CompletionItemKind,
373373
}
374374

375375
// variableCompletionItems returns completion items for $ variable references.
376-
// It suggests $currentObject (common in data containers) and any page parameters
377-
// found in the document's CREATE PAGE Params declaration.
378-
func (s *mdlServer) variableCompletionItems(docText string, linePrefix string) []protocol.CompletionItem {
376+
// It suggests $currentObject (with entity type from enclosing data container) and
377+
// any page parameters found in the document's CREATE PAGE Params declaration.
378+
// cursorLine is the 0-based line number of the cursor position.
379+
func (s *mdlServer) variableCompletionItems(docText string, linePrefix string, cursorLine int) []protocol.CompletionItem {
379380
// Extract the partial after the last $ to filter suggestions
380381
lastDollar := strings.LastIndex(linePrefix, "$")
381382
partial := ""
@@ -385,15 +386,38 @@ func (s *mdlServer) variableCompletionItems(docText string, linePrefix string) [
385386

386387
var items []protocol.CompletionItem
387388

388-
// Always suggest $currentObject — it's the most common data container variable
389+
// Scan upward from cursor to find enclosing data container context
390+
entityType, widgetName := scanEnclosingDataContainer(docText, cursorLine)
391+
392+
// Suggest $currentObject with entity type if found
389393
if partial == "" || strings.HasPrefix("CURRENTOBJECT", partial) {
394+
detail := "Current object from enclosing data container"
395+
if entityType != "" {
396+
detail = entityType
397+
}
390398
items = append(items, protocol.CompletionItem{
391399
Label: "$currentObject",
392400
Kind: protocol.CompletionItemKindVariable,
393-
Detail: "Current object from enclosing data container",
401+
Detail: detail,
394402
})
395403
}
396404

405+
// Suggest $widgetName (selection) for list containers
406+
if widgetName != "" {
407+
wNameUpper := strings.ToUpper(widgetName)
408+
if partial == "" || strings.HasPrefix(wNameUpper, partial) {
409+
detail := "Selection from list container"
410+
if entityType != "" {
411+
detail = entityType + " (selection)"
412+
}
413+
items = append(items, protocol.CompletionItem{
414+
Label: "$" + widgetName,
415+
Kind: protocol.CompletionItemKindVariable,
416+
Detail: detail,
417+
})
418+
}
419+
}
420+
397421
// Extract page parameter names from CREATE PAGE ... Params: { $Name: Type, ... }
398422
paramNames := extractPageParamNames(docText)
399423
for _, name := range paramNames {
@@ -409,21 +433,141 @@ func (s *mdlServer) variableCompletionItems(docText string, linePrefix string) [
409433
return items
410434
}
411435

412-
// extractPageParamNames extracts parameter names from CREATE PAGE ... Params: { $Name: Type } declarations.
436+
// scanEnclosingDataContainer scans upward from cursorLine to find the nearest
437+
// enclosing data container widget and its entity type.
438+
// Returns (entityType, widgetName) where widgetName is set for list containers.
439+
// Best-effort: uses brace matching and keyword scanning (Enclosing Scope Scanning pattern).
440+
func scanEnclosingDataContainer(text string, cursorLine int) (string, string) {
441+
lines := strings.Split(text, "\n")
442+
if cursorLine >= len(lines) {
443+
return "", ""
444+
}
445+
446+
// Track brace nesting depth as we scan upward
447+
depth := 0
448+
for i := cursorLine; i >= 0; i-- {
449+
line := lines[i]
450+
// Count braces on this line (right to left for correct nesting)
451+
for j := len(line) - 1; j >= 0; j-- {
452+
switch line[j] {
453+
case '}':
454+
depth++
455+
case '{':
456+
depth--
457+
}
458+
}
459+
460+
// At depth < 0, we've found an opening brace that encloses the cursor
461+
if depth < 0 {
462+
trimmed := strings.TrimSpace(line)
463+
upper := strings.ToUpper(trimmed)
464+
465+
// Check for data container keywords
466+
entityType, widgetName, isList := extractContainerInfo(upper, trimmed)
467+
if entityType != "" {
468+
if isList {
469+
return entityType, widgetName
470+
}
471+
return entityType, ""
472+
}
473+
// Reset depth — we passed through this opening brace but it wasn't a data container
474+
depth = 0
475+
}
476+
}
477+
return "", ""
478+
}
479+
480+
// extractContainerInfo extracts entity type and widget name from a data container line.
481+
// upperLine is the uppercase version, originalLine preserves case for widget name extraction.
482+
// Returns (entityType, widgetName, isList).
483+
func extractContainerInfo(upperLine string, originalLine string) (string, string, bool) {
484+
// Patterns: DATAVIEW name (DataSource: ...) {
485+
// LISTVIEW name (DataSource: DATABASE FROM Module.Entity ...) {
486+
// DATAGRID name (DataSource: DATABASE FROM Module.Entity ...) {
487+
// GALLERY name (DataSource: DATABASE FROM Module.Entity ...) {
488+
type containerPattern struct {
489+
keyword string
490+
isList bool
491+
}
492+
patterns := []containerPattern{
493+
{"DATAVIEW ", false},
494+
{"LISTVIEW ", true},
495+
{"DATAGRID ", true},
496+
{"GALLERY ", true},
497+
}
498+
499+
for _, p := range patterns {
500+
if !strings.HasPrefix(upperLine, p.keyword) {
501+
continue
502+
}
503+
504+
// Extract widget name (first token after keyword)
505+
rest := strings.TrimSpace(originalLine[len(p.keyword):])
506+
widgetName := ""
507+
for j, c := range rest {
508+
if c == ' ' || c == '(' || c == '{' {
509+
widgetName = rest[:j]
510+
break
511+
}
512+
}
513+
if widgetName == "" {
514+
widgetName = rest
515+
}
516+
517+
// Extract entity from DataSource (use original case for entity name)
518+
entityType := extractEntityFromLine(originalLine)
519+
if entityType != "" {
520+
return entityType, widgetName, p.isList
521+
}
522+
}
523+
return "", "", false
524+
}
525+
526+
// extractEntityFromLine extracts the entity type from a DataSource declaration in a line.
527+
// Preserves original casing of the entity name (e.g., "Sales.Order" not "SALES.ORDER").
528+
func extractEntityFromLine(line string) string {
529+
// Case-insensitive search for "DATABASE FROM" pattern
530+
upperLine := strings.ToUpper(line)
531+
if idx := strings.Index(upperLine, "DATABASE FROM "); idx >= 0 {
532+
rest := strings.TrimSpace(line[idx+len("DATABASE FROM "):])
533+
// Entity is the next qualified name (Module.Entity)
534+
end := strings.IndexAny(rest, " \t,)}")
535+
if end < 0 {
536+
end = len(rest)
537+
}
538+
entity := rest[:end]
539+
if strings.Contains(entity, ".") {
540+
return entity
541+
}
542+
}
543+
// MICROFLOW/NANOFLOW datasource — entity type not directly available
544+
// $ParamName datasource — would need to resolve param type
545+
return ""
546+
}
547+
548+
// extractPageParamNames extracts parameter names from CREATE PAGE/SNIPPET parameter declarations.
549+
// Best-effort: scans for "$Name: Type" patterns (colon after identifier distinguishes
550+
// declarations from usage like "DataSource: $Param" or context comments like "$currentObject").
413551
func extractPageParamNames(text string) []string {
414552
var names []string
415553
for _, line := range strings.Split(text, "\n") {
416554
trimmed := strings.TrimSpace(line)
417-
// Look for $ParamName patterns in Params declarations
418-
// Format: Params: { $Name: Type } or $Name: Type on separate lines
555+
// Skip DECLARE lines (variable declarations, not page params)
556+
if strings.HasPrefix(strings.ToUpper(trimmed), "DECLARE") {
557+
continue
558+
}
559+
// Skip comment lines
560+
if strings.HasPrefix(trimmed, "--") {
561+
continue
562+
}
419563
idx := 0
420564
for idx < len(trimmed) {
421565
dollar := strings.Index(trimmed[idx:], "$")
422566
if dollar < 0 {
423567
break
424568
}
425569
dollar += idx
426-
// Extract the name after $
570+
// Extract the identifier after $
427571
end := dollar + 1
428572
for end < len(trimmed) {
429573
c := trimmed[end]
@@ -435,8 +579,10 @@ func extractPageParamNames(text string) []string {
435579
}
436580
if end > dollar+1 {
437581
name := trimmed[dollar+1 : end]
438-
// Skip if this looks like a variable declaration (DECLARE) rather than a param
439-
if !strings.HasPrefix(strings.ToUpper(trimmed), "DECLARE") {
582+
// Only match parameter declarations: "$Name:" followed by a type.
583+
// Skip references like "DataSource: $Param" where $ is a value, not a declaration.
584+
rest := strings.TrimSpace(trimmed[end:])
585+
if strings.HasPrefix(rest, ":") {
440586
names = append(names, name)
441587
}
442588
}

‎cmd/mxcli/lsp_completion_test.go‎

Lines changed: 94 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,21 @@ func TestExtractPageParamNames(t *testing.T) {
3232
text: "DECLARE $Temp String = '';\n$Order: Mod.Order",
3333
expected: []string{"Order"},
3434
},
35+
{
36+
name: "reject body $currentObject reference",
37+
text: "CREATE PAGE Mod.Page ($Order: Mod.Order) {\n -- Context: $currentObject (Mod.Order)\n DATAVIEW dv1 (DataSource: $Order)\n}",
38+
expected: []string{"Order"},
39+
},
40+
{
41+
name: "reject body $var usage without colon",
42+
text: "CREATE PAGE Mod.Page ($Item: Mod.Item) {\n TEXTBOX t1 (Attribute: $Item/Name)\n}",
43+
expected: []string{"Item"},
44+
},
45+
{
46+
name: "reject comment lines",
47+
text: "-- $FakeParam: NotReal\n$RealParam: Mod.Entity",
48+
expected: []string{"RealParam"},
49+
},
3550
}
3651

3752
for _, tt := range tests {
@@ -52,9 +67,10 @@ func TestExtractPageParamNames(t *testing.T) {
5267

5368
func TestVariableCompletionItems(t *testing.T) {
5469
s := &mdlServer{}
55-
docText := "CREATE PAGE Mod.Page (\n Params: { $Customer: Mod.Customer }\n) {\n DATAVIEW dv1 (DataSource: $Customer) {\n"
70+
docText := "CREATE PAGE Mod.Page (\n Params: { $Customer: Mod.Customer }\n) {\n DATAVIEW dv1 (DataSource: $Customer) {\n TEXTBOX t1 (Attribute: $\n"
5671

57-
items := s.variableCompletionItems(docText, "$")
72+
// Cursor at last line (line 4, 0-based)
73+
items := s.variableCompletionItems(docText, "$", 4)
5874
if len(items) == 0 {
5975
t.Fatal("expected completion items for $ prefix")
6076
}
@@ -77,3 +93,79 @@ func TestVariableCompletionItems(t *testing.T) {
7793
t.Error("expected $Customer in completion items")
7894
}
7995
}
96+
97+
func TestVariableCompletionItems_DataGridContext(t *testing.T) {
98+
s := &mdlServer{}
99+
docText := "CREATE PAGE Mod.Page ($Order: Sales.Order) {\n DATAGRID dgOrders (DataSource: DATABASE FROM Sales.Order) {\n COLUMN Name {\n TEXTBOX t1 (Attribute: $\n"
100+
101+
// Cursor inside DATAGRID column (line 3)
102+
items := s.variableCompletionItems(docText, "$", 3)
103+
104+
var currentObjDetail string
105+
foundSelection := false
106+
for _, item := range items {
107+
if item.Label == "$currentObject" {
108+
currentObjDetail = item.Detail
109+
}
110+
if item.Label == "$dgOrders" {
111+
foundSelection = true
112+
}
113+
}
114+
if currentObjDetail != "Sales.Order" {
115+
t.Errorf("expected $currentObject detail = %q, got %q", "Sales.Order", currentObjDetail)
116+
}
117+
if !foundSelection {
118+
t.Error("expected $dgOrders selection variable in completion items")
119+
}
120+
}
121+
122+
func TestScanEnclosingDataContainer(t *testing.T) {
123+
tests := []struct {
124+
name string
125+
text string
126+
cursorLine int
127+
wantEntity string
128+
wantWidgetName string
129+
}{
130+
{
131+
name: "inside DATAVIEW",
132+
text: "DATAVIEW dv1 (DataSource: DATABASE FROM Mod.Order) {\n TEXTBOX t1\n}",
133+
cursorLine: 1,
134+
wantEntity: "Mod.Order",
135+
wantWidgetName: "",
136+
},
137+
{
138+
name: "inside DATAGRID",
139+
text: "DATAGRID dg1 (DataSource: DATABASE FROM Shop.Product) {\n COLUMN Name {\n TEXTBOX t1\n }\n}",
140+
cursorLine: 2,
141+
wantEntity: "Shop.Product",
142+
wantWidgetName: "dg1",
143+
},
144+
{
145+
name: "no container",
146+
text: "CREATE PAGE Mod.Page ($P: Mod.E) {\n TEXTBOX t1\n}",
147+
cursorLine: 1,
148+
wantEntity: "",
149+
wantWidgetName: "",
150+
},
151+
{
152+
name: "nested DataView inside DataGrid",
153+
text: "DATAGRID dg1 (DataSource: DATABASE FROM Sales.Order) {\n COLUMN col1 {\n DATAVIEW dv1 (DataSource: DATABASE FROM Sales.Line) {\n TEXTBOX t1\n }\n }\n}",
154+
cursorLine: 3,
155+
wantEntity: "Sales.Line",
156+
wantWidgetName: "",
157+
},
158+
}
159+
160+
for _, tt := range tests {
161+
t.Run(tt.name, func(t *testing.T) {
162+
entity, widgetName := scanEnclosingDataContainer(tt.text, tt.cursorLine)
163+
if entity != tt.wantEntity {
164+
t.Errorf("scanEnclosingDataContainer() entity = %q, want %q", entity, tt.wantEntity)
165+
}
166+
if widgetName != tt.wantWidgetName {
167+
t.Errorf("scanEnclosingDataContainer() widgetName = %q, want %q", widgetName, tt.wantWidgetName)
168+
}
169+
})
170+
}
171+
}

0 commit comments

Comments
 (0)