From 7e9a4359cd42d98c8f0472846e7b9eae4c3389a5 Mon Sep 17 00:00:00 2001 From: Christoph Brandau Date: Thu, 10 Sep 2026 08:45:25 +0200 Subject: [PATCH 1/2] feat(cdl): include args in generated event handler snippet The generated event handler snippet now declares the proc with an args block. This enables parameters to be referenced within the handler body. The change makes the snippet compatible with events that pass arguments. - Include an args block in the proc declaration for event handlers. --- client/src/common/cdlEventHandler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/common/cdlEventHandler.ts b/client/src/common/cdlEventHandler.ts index cdb7742..b6282c9 100644 --- a/client/src/common/cdlEventHandler.ts +++ b/client/src/common/cdlEventHandler.ts @@ -105,7 +105,7 @@ export function createCdlEventHandlerSnippet(handler: CdlEventHandler): string { const globals = [ ...new Set(handler.parameterNames.map((parameter) => momVariableName(parameter))) ] - const lines = [`proc ${momEventName(handler.eventName)} { } {`] + const lines = [`proc ${momEventName(handler.eventName)} {args} {`] if (globals.length > 0) { lines.push(...globals.map((variable) => ` global ${variable}`), "") -- 2.54.0 From d8611d7aea18ffadbac9bae350808624381a3a09 Mon Sep 17 00:00:00 2001 From: Christoph Brandau Date: Thu, 10 Sep 2026 09:07:18 +0200 Subject: [PATCH 2/2] feat(tcl): add static variable name extraction and array key completion This adds tooling to statically extract Tcl variable names from syntax trees without evaluating substitutions, enabling better completions for array elements and plain variables. The new helpers are wired into the completion and navigation flows and are supported by tests covering array keys and substitutions. - Introduce variable_names.py with variable_name() and array_key_parts() - Wire static name extraction into completion and symbol indexing - Add tests for array key completion with substitutions --- server/src/lsp_server.py | 11 +- server/src/tools/completion_items.py | 100 +++++++++++++- server/src/tools/navigation.py | 17 ++- server/src/tools/semantic_tokens.py | 6 + server/src/tools/variable_index.py | 3 +- server/src/tools/variable_names.py | 44 ++++++ .../python_tests/test_completion_context.py | 127 ++++++++++++++++++ 7 files changed, 300 insertions(+), 8 deletions(-) create mode 100644 server/src/tools/variable_names.py diff --git a/server/src/lsp_server.py b/server/src/lsp_server.py index e9b1c32..1cb4554 100644 --- a/server/src/lsp_server.py +++ b/server/src/lsp_server.py @@ -47,6 +47,7 @@ from common.load_data import standard_items from lsp_tclserver import TclLanguageServer from tools.completion_items import ( CompletionContext, + array_element_completions, completion_context, ranked_completion_items, ) @@ -271,12 +272,18 @@ def document_diagnostic(params: lsp.DocumentDiagnosticParams): @LSP_SERVER.feature( lsp.TEXT_DOCUMENT_COMPLETION, - lsp.CompletionOptions(trigger_characters=["$", " ", "-"]), + lsp.CompletionOptions(trigger_characters=["$", " ", "-", "(", ","]), ) def on_completion(params: lsp.CompletionParams) -> lsp.CompletionList: doc = LSP_SERVER.workspace.get_text_document(params.text_document.uri) position = params.position source_lines = LSP_SERVER.get_lines(doc) + array_items = array_element_completions( + source_lines, position, LSP_SERVER.navigation_snapshot().values(), + str(pathlib.Path(uris.to_fs_path(doc.uri))), + ) + if array_items is not None: + return lsp.CompletionList(is_incomplete=False, items=array_items) context = completion_context(source_lines, position) # Variable completion wins inside command arguments. Otherwise prefer the @@ -323,7 +330,7 @@ def on_completion(params: lsp.CompletionParams) -> lsp.CompletionList: and params.context is not None and params.context.trigger_kind == lsp.CompletionTriggerKind.TriggerCharacter - and params.context.trigger_character in {" ", "-"} + and params.context.trigger_character in {" ", "-", "(", ","} ): return lsp.CompletionList(is_incomplete=False, items=[]) diff --git a/server/src/tools/completion_items.py b/server/src/tools/completion_items.py index 27586a6..86042b3 100644 --- a/server/src/tools/completion_items.py +++ b/server/src/tools/completion_items.py @@ -9,7 +9,13 @@ import lsprotocol.types as lsp from tclint.syntax_tree import BareWord, Command, List, Visitor from common.load_data import standard_items -from tools.tcl_command_completion import line_prefix_at_position +from tools.navigation import FileSymbolIndex +from tools.variable_names import variable_name +from tools.tcl_command_completion import ( + DynamicCompletionKind, + line_prefix_at_position, + tcl_argument_completion, +) BUILTIN_VAR_LABELS = {ci.label for ci in standard_items.nx_variables} BUILTIN_PROC_LABELS = {ci.label for ci in standard_items.nx_procs} @@ -36,6 +42,94 @@ COMMAND_KINDS = { _VARIABLE_PREFIX_RE = re.compile(r"(?(?:::)?[A-Za-z_][A-Za-z0-9_:]*)\((?P[^()\n]*)$" +) + + +def array_element_completions( + source_lines: Sequence[str], + position: lsp.Position, + indexes: Iterable[FileSymbolIndex], + current_path: str, +) -> list[lsp.CompletionItem] | None: + """Complete literal array keys, replacing only the text inside parentheses.""" + prefix = line_prefix_at_position(source_lines, position) + if prefix is None: + return None + match = _ARRAY_PREFIX_RE.search(prefix) + if match is None: + return None + before = prefix[:match.start()] + if not before.endswith("$"): + argument = tcl_argument_completion(source_lines, position) + if argument is None or argument.dynamic_kind != DynamicCompletionKind.VARIABLE: + return None + name, key_prefix = match.group("name", "key") + part_index = key_prefix.count(",") + key_prefix = key_prefix.rsplit(",", 1)[-1] + if any(char in key_prefix for char in "$[]{}\\"): + return None + indexes = list(indexes) + local_scope = None + for index in indexes: + if index.path != current_path: + continue + for occurrence in index.occurrences: + span = occurrence.declaration_range + if ( + occurrence.identity.kind == "proc" + and occurrence.is_definition + and span is not None + and span.start.line <= position.line <= span.end.line + ): + local_scope = f"{index.path}::proc::{occurrence.identity.name}" + keys: dict[str, set[tuple[str | None, ...]]] = {} + for index in indexes: + for occurrence in index.occurrences: + parts = occurrence.array_parts + key = parts[part_index] if part_index < len(parts) else None + if ( + key + and ( + occurrence.identity.scope is None + or occurrence.identity.scope == local_scope + ) + and key.startswith(key_prefix) + and not any(char in key for char in "$[]\\") + and occurrence.identity.name.removeprefix("::") == name.removeprefix("::") + ): + keys.setdefault(key, set()).add(occurrence.array_template_parts[part_index + 1:]) + line = source_lines[position.line] + suffix = line[len(prefix):] + remaining = re.match(r"[^(),\s$\[\]{}]*", suffix).group() + has_close = suffix[len(remaining):].startswith((")", ",")) + start = position.character - len(key_prefix.encode("utf-16-le")) // 2 + end = position.character + len(remaining.encode("utf-16-le")) // 2 + items = [] + for key in sorted(keys): + new_text = key + # Fill missing index components only; keep an existing comma and suffix. + if not suffix[len(remaining):].startswith(","): + tails = sorted(keys[key], key=lambda tail: (len(tail), repr(tail))) + tail = tails[0] + if all(part is not None for part in tail) and any("$" in part for part in tail): + new_text = ",".join([key, *tail]) + new_text += "" if has_close else ")" + items.append(lsp.CompletionItem( + label=key, + kind=lsp.CompletionItemKind.Field, + detail=f"{name}({key})", + insert_text_format=lsp.InsertTextFormat.PlainText, + text_edit=lsp.TextEdit( + range=lsp.Range( + start=lsp.Position(line=position.line, character=start), + end=lsp.Position(line=position.line, character=end), + ), + new_text=new_text, + ), + )) + return items def completion_context( @@ -167,8 +261,8 @@ class CompletionCollector(Visitor): # Collect variables set with explicit global namespace: set ::var_name ... elif routine.contents == "set" and command.args: first = command.args[0] - if isinstance(first, BareWord) and getattr(first, "value", None): - var_name = first.value + var_name = variable_name(first) + if var_name: if var_name.startswith("::"): base_name = var_name.split("(", 1)[0] clean_name = base_name[2:] # remove leading '::' for completion display diff --git a/server/src/tools/navigation.py b/server/src/tools/navigation.py index 462e8ac..4410743 100644 --- a/server/src/tools/navigation.py +++ b/server/src/tools/navigation.py @@ -4,7 +4,8 @@ from dataclasses import dataclass from pathlib import Path import lsprotocol.types as lsp -from tclint.syntax_tree import Command, List, Node, Script, VarSub +from tclint.syntax_tree import Command, List, Node, QuotedWord, Script, VarSub +from tools.variable_names import array_key_parts, variable_name ROOT_NAMESPACE = "::" @@ -27,6 +28,9 @@ class SymbolOccurrence: fallback_identity: SymbolIdentity | None = None caller: SymbolIdentity | None = None declaration_range: lsp.Range | None = None + array_element: str | None = None + array_parts: tuple[str | None, ...] = () + array_template_parts: tuple[str | None, ...] = () @dataclass(frozen=True) @@ -96,6 +100,8 @@ def _name_range(node: Node, raw_name: str, *, variable_sub: bool = False) -> lsp column += 2 if getattr(node, "braced", False) else 1 else: position = getattr(node, "contents_pos", None) or node.pos + if isinstance(node, QuotedWord) and node.contents is None and node.children: + position = node.children[0].pos line, column = position normalized = _without_array_index(raw_name) @@ -312,6 +318,13 @@ def build_file_symbol_index( is_definition=is_definition, symbol_kind=lsp.SymbolKind.Variable, container_name=_container_name(symbol_identity), + array_element=( + raw_name.split("(", 1)[1][:-1] + if "(" in raw_name and raw_name.endswith(")") + else None + ), + array_parts=array_key_parts(node), + array_template_parts=array_key_parts(node, preserve_variables=True), ) ) @@ -444,7 +457,7 @@ def build_file_symbol_index( for node, is_definition in _variable_command_nodes(command): if id(node) in declaration_ids: continue - raw_name = _static_contents(node) + raw_name = variable_name(node) if raw_name: add_variable( node, diff --git a/server/src/tools/semantic_tokens.py b/server/src/tools/semantic_tokens.py index be8fb5e..ff58796 100644 --- a/server/src/tools/semantic_tokens.py +++ b/server/src/tools/semantic_tokens.py @@ -5,6 +5,7 @@ import attrs from common.load_data import standard_items from tclint.commands.plugins import PluginManager from tclint.syntax_tree import BareWord, Command, QuotedWord, Visitor +from tools.variable_names import variable_name # Constructing a PluginManager scans entry points, and get_commands() rebuilds # the builtin command set on every call. Semantic tokens are requested often, so @@ -159,6 +160,11 @@ class _Highlighter(Visitor): if routine.contents == "set" and command.args: first_arg = command.args[0] token_info = self._get_token_info(first_arg) + if first_arg.contents is None: + name = variable_name(first_arg) + if name: + line, col = first_arg.children[0].pos + token_info = ((line - 1, col - 1), len(name)) if token_info: (line, col), length = token_info self._append_token((line, col), length, "variable", [TokenModifier.declaration]) diff --git a/server/src/tools/variable_index.py b/server/src/tools/variable_index.py index 564ef22..6592149 100644 --- a/server/src/tools/variable_index.py +++ b/server/src/tools/variable_index.py @@ -4,6 +4,7 @@ from dataclasses import dataclass from tclint.syntax_tree import Command, Node, Script from tclint.syntax_tree import List as TclList +from tools.variable_names import variable_name @dataclass @@ -81,7 +82,7 @@ def build_variable_index( return if routine == "set" and node.args: - raw_name = getattr(node.args[0], "contents", None) + raw_name = variable_name(node.args[0]) base = _normalize_var_name(raw_name) if base is not None: if raw_name and raw_name.startswith("::"): diff --git a/server/src/tools/variable_names.py b/server/src/tools/variable_names.py new file mode 100644 index 0000000..fc92e58 --- /dev/null +++ b/server/src/tools/variable_names.py @@ -0,0 +1,44 @@ +"""Extract statically known variable names without evaluating Tcl substitutions.""" + +from tclint.syntax_tree import BareWord, CompoundBareWord, Node, QuotedWord, VarSub + + +def variable_name(node: Node) -> str | None: + contents = node.contents + if isinstance(contents, str): + return contents + if isinstance(node, (CompoundBareWord, QuotedWord)) and node.children: + first = node.children[0] + last = node.children[-1] + if ( + isinstance(first, BareWord) + and isinstance(last, BareWord) + and "(" in first.value + and last.value.endswith(")") + ): + # Substitutions in an array index do not change the array's name. + return first.value.split("(", 1)[0] or None + return None + + +def array_key_parts(node: Node, *, preserve_variables: bool = False) -> tuple[str | None, ...]: + """Keep comma-separated literal index components; substitutions are unknown.""" + contents = node.contents + if not isinstance(contents, str): + if not isinstance(node, (CompoundBareWord, QuotedWord)) or not variable_name(node): + return () + chunks = [] + for child in node.children: + if isinstance(child, BareWord): + chunks.append(child.value) + elif preserve_variables and isinstance(child, VarSub) and not child.children: + chunks.append("${" + child.value + "}" if child.braced else "$" + child.value) + else: + chunks.append("\0") + contents = "".join(chunks) + if "(" not in contents or not contents.endswith(")"): + return () + return tuple( + part if part and not any(char in part for char in ("\0[]\\" if preserve_variables else "\0$[]\\")) else None + for part in contents.split("(", 1)[1][:-1].split(",") + ) diff --git a/server/tests/python_tests/test_completion_context.py b/server/tests/python_tests/test_completion_context.py index b6a8d30..0e2accf 100644 --- a/server/tests/python_tests/test_completion_context.py +++ b/server/tests/python_tests/test_completion_context.py @@ -103,6 +103,133 @@ def _argument_completion_labels(source: str) -> set[str] | None: return {item.label for item in completion.items} +def test_array_keys_complete_in_set_and_substitution(tmp_path: Path, monkeypatch): + server, _, _ = _completion_server(tmp_path, monkeypatch) + workspace = _document( + tmp_path / "arrays.tcl", + "set ::lib_flag(enabled) 1\nset ::lib_flag(external) 1\n", + ) + assert server.update_poco_completion_for_file(workspace) + source = ( + "set lib_flag(enabled) 0\n" + "set lib_flag(empty) 1\n" + "set other(wrong) 1\n" + "proc hidden {} { set lib_flag(private) 1 }\n" + "set lib_flag()\n" + "puts $lib_flag(en)\n" + "puts 😀; set lib_flag(em\n" + ) + current = _document(tmp_path / "arrays-current.tcl", source) + server.workspace.put_text_document(lsp.TextDocumentItem( + uri=current.uri, language_id="tcl", version=1, text=source, + )) + assert server.update_poco_completion_for_file(current) + items = _complete(current, _position_after(source, "set lib_flag(", 3)) + assert [item.label for item in items] == ["empty", "enabled", "external"] + assert all(item.text_edit.new_text == item.label for item in items) + items = _complete(current, _position_after(source, "puts $lib_flag(en")) + assert [item.label for item in items] == ["enabled"] + assert items[0].text_edit.new_text == "enabled" + position = lsp.Position(line=6, character=len(source.splitlines()[6].encode("utf-16-le")) // 2) + items = _complete(current, position) + assert [item.label for item in items] == ["empty"] + assert items[0].text_edit.new_text == "empty)" + assert items[0].text_edit.range.start.character == position.character - 2 + + +def test_dynamic_array_index_keeps_variable_identity(tmp_path: Path, monkeypatch): + from tools.navigation import build_file_symbol_index + from tools.parser import CustomParser + from tools.semantic_tokens import _Highlighter + from tools.variable_index import build_variable_index + from tools.variable_names import variable_name + + source = ( + "set custom_flag(from_move,$::mom_path_name) 1\n" + 'set "::quoted_flag(from_move,$::mom_path_name)" 1\n' + "set ::command_flag([info hostname]) 1\n" + "set ${dynamic_name}(entry) 1\n" + "proc example {} { set local_flag($::mom_path_name) 1 }\n" + "puts $custom_flag\n" + ) + tree = CustomParser().parse(source) + globals_, locals_, _ = build_variable_index(source, tree) + assert {"custom_flag", "quoted_flag", "command_flag"} <= globals_ + assert locals_["example"] == {"local_flag"} + assert variable_name(tree.children[3].args[0]) is None + path = tmp_path / "dynamic.tcl" + index = build_file_symbol_index(str(path), path.as_uri(), tree) + definition = next( + item for item in index.occurrences + if item.identity.name == "::custom_flag" and item.is_definition + ) + assert definition.range.start.character == 4 + assert definition.range.end.character == 15 + assert definition.array_element is None + assert any(item.identity.name == "::mom_path_name" for item in index.occurrences) + highlighter = _Highlighter([], {}) + tree.accept(highlighter, recurse=True) + assert any( + position == (0, 4) and length == 11 and kind == "variable" + for position, length, kind, _ in highlighter._tokens + ) + server, _, _ = _completion_server(tmp_path, monkeypatch) + current = _document(path, source) + server.workspace.put_text_document(lsp.TextDocumentItem( + uri=current.uri, language_id="tcl", version=1, text=source, + )) + assert server.update_poco_completion_for_file(current) + items = _complete(current, _position_after(source, "puts $custom")) + assert "custom_flag" in {item.label for item in items} + workspace_items = next( + items for item_path, items in server.completion_items_by_file_snapshot().items() + if server.paths_equal(item_path, str(path)) + ) + assert {"quoted_flag", "command_flag"} <= {item.label for item in workspace_items} + + +def test_literal_array_components_complete_around_substitutions(tmp_path: Path, monkeypatch): + from tools.completion_items import array_element_completions + + server, _, _ = _completion_server(tmp_path, monkeypatch) + source = ( + "set custom_flag(from_move,$::mom_path_name) 1\n" + "set custom_flag(to_move,$::mom_path_name) 1\n" + "set custom_flag($::mom_path_name,finished) 1\n" + "set custom_flag(prefix_$::mom_path_name,other) 1\n" + "set custom_flag([info hostname],command_tail) 1\n" + "set other_flag(wrong,$::mom_path_name) 1\n" + "set multi_flag(move,$first,axis,$second) 1\n" + ) + document = _document(tmp_path / "components.tcl", source) + assert server.update_poco_completion_for_file(document) + cases = [ + ("set custom_flag(|,$::mom_path_name)", {"from_move", "to_move"}, "from_move", "set custom_flag(from_move,$::mom_path_name)"), + ("set custom_flag(fr|om_old,$::mom_path_name)", {"from_move"}, "from_move", "set custom_flag(from_move,$::mom_path_name)"), + ("puts $custom_flag($::mom_path_name,fi|)", {"finished"}, "finished", "puts $custom_flag($::mom_path_name,finished)"), + ("set custom_flag($::mom_path_name,|)", {"finished", "other", "command_tail"}, "other", "set custom_flag($::mom_path_name,other)"), + ("set custom_flag(fr|", {"from_move"}, "from_move", "set custom_flag(from_move,$::mom_path_name)"), + ("set custom_flag(fr|)", {"from_move"}, "from_move", "set custom_flag(from_move,$::mom_path_name)"), + ("set multi_flag(m|)", {"move"}, "move", "set multi_flag(move,$first,axis,$second)"), + ] + for marked, labels, selected, expected in cases: + offset = marked.index("|") + line = marked.replace("|", "") + items = array_element_completions( + [line], lsp.Position(line=0, character=offset), + server.navigation_snapshot().values(), str(tmp_path / "caller.tcl"), + ) + assert {item.label for item in items} == labels + edit = next(item.text_edit for item in items if item.label == selected) + item = next(item for item in items if item.label == selected) + assert item.insert_text_format == lsp.InsertTextFormat.PlainText + assert line[:edit.range.start.character] + edit.new_text + line[edit.range.end.character:] == expected + assert array_element_completions( + ["set custom_flag(from_move,$::mom"], lsp.Position(line=0, character=31), + server.navigation_snapshot().values(), str(tmp_path / "caller.tcl"), + ) is None + + def _argument_completion_request(source: str): lines = source.split("\n") character = len(lines[-1].encode("utf-16-le")) // 2 -- 2.54.0