Fix issues #1-#4: navigation, chain completion, GUI tree — plus default hotkeys and a quieter footprint - #6
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves navigation and UX across the CK3 Modding Toolkit by expanding what the indexer and language features recognize, and by adding targeted on-demand scanning where full indexing would be too costly. While the title focuses on inline scripted_trigger/scripted_effect declarations in event files, the change set also includes fixes for issues #1–#4 as reflected in the changelog.
Changes:
- Index inline
scripted_trigger/scripted_effectdeclarations in event files (including$PARAM$harvesting + doc-comment capture) so definition/hover/completion can see them. - Merge on-demand vanilla/parent reference hits into “Find References” results (without fully reference-indexing those roots) and adjust definition ordering to show all sources (mod → parent → vanilla).
- Improve datafunction completion replacement ranges in
.gui/.ymlcontexts and update the GUI tree filter to optionally hide ancestor rows.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/lazyRefs.test.ts | Adds unit coverage for textual reference scanning behavior (token boundaries, comment skipping, memoization). |
| test/indexer.test.ts | Adds regression test ensuring inline scripted triggers/effects in event files are indexed with correct kind/line/params. |
| test/definitionProvider.test.ts | Adds coverage for definition ordering across sources and merged reference results. |
| test/dataTypes.test.ts | Adds coverage for cursor-anchored datafunction completion replace ranges. |
| server/src/server.ts | Wires lazy reference scanning into the LSP references handler; passes cursor position into datafunction completion. |
| server/src/index/lazyRefs.ts | Introduces on-demand, memoized textual scanning for references in vanilla/parent roots. |
| server/src/index/extract.ts | Extends event-id extraction to also capture inline scripted_trigger/scripted_effect declarations. |
| server/src/features/references.ts | Makes references provider async and merges indexed refs with lazy-scanned refs while avoiding def-site duplication. |
| server/src/features/guiLanguage.ts | Passes cursor position into datafunction completion for GUI contexts. |
| server/src/features/definition.ts | Ensures go-to-definition lists all sources with consistent ordering (mod → parent → vanilla). |
| server/src/features/datafunction.ts | Adds optional cursor-aware textEdit ranges so completion replaces only the typed tail segment after dots. |
| client/src/webviews/guiTree/panel.ts | Updates GUI tree filtering UI/logic to support “matches only” by default with an optional “parents” toggle. |
| CHANGELOG.md | Documents fixes for #1–#5 under Unreleased. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+26
to
+34
| const refs: Reference[] = data.refIndex.lookup(name).slice(); | ||
| if (lazyRefs) { | ||
| // The textual scan cannot tell a non-top-level definition site (inline | ||
| // scripted_trigger, nested title) from a use: drop hits the definition | ||
| // index already knows, they are appended under includeDeclaration below. | ||
| const isDefSite = (r: Reference) => | ||
| data.index.inFile(r.file).some((d) => d.name === name && d.line === r.line); | ||
| refs.push(...(await lazyRefs(name)).filter((r) => !isDefSite(r))); | ||
| } |
Owner
Author
There was a problem hiding this comment.
Good catch — implemented in 53c931b. The sites now come from lookupAll(name) once (a handful of entries) into a Set, keyed with the same path normalization inFile applied internally so the comparison stays case-insensitive on Windows.
Generated by Claude Code
JDeffner
changed the title
JDeffner
pushed a commit
that referenced
this pull request
Jul 19, 2026
Review feedback (Copilot on #6): filtering lazy-scan hits with index.inFile(file).some(...) per reference is O(refs x defs-in-file), and big vanilla files carry 1000+ definitions. The sites of the looked-up name come from lookupAll(name) once (a handful), keyed with the same path normalization inFile used, so the filter is O(refs). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bev9y4WSYSVyDr2SDrB98u
Go to References listed only definition sites for names whose usage lives in vanilla or read-only parent mods (#3): those roots are deliberately not reference-indexed up front (memory guard, AD-4), and nothing filled the gap on demand. References now merge an on-demand textual scan over the un-indexed roots (LazyReferenceScanner): exact-token matches per line, comments stripped, column-0 definition keys skipped, engine tokens and grammar keywords excluded, memoized per name until the roots change. The scan is textual rather than schema-driven on purpose - script-value arguments like `add_dynasty_prestige = miniscule_dynasty_prestige_loss` are real usage sites with no schema ref-field for the enclosing key. Go to Definition hid the game-folder and parent originals whenever a mod override existed (#4). All sources are now listed, mod first - an unintended override of a vanilla or parent name is exactly what a modder wants to notice (as requested in the #5 discussion). Datatype chain completion after a dot in .gui and .yml files matched the suggest widget against the whole dotted word (#2): the gui/loc word pattern includes ".", so after `[GetPlayer.` the client filtered member labels against "GetPlayer." (and would have replaced the whole chain on accept). Datafn completion items now carry an explicit textEdit range covering exactly the typed tail segment. Closes #2. Closes #3. Closes #4. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bev9y4WSYSVyDr2SDrB98u
Filtering the widget tree interleaved every ancestor row with the matches, which buries the results in deep vanilla windows (hud.gui). Matches now render as a flat list (ancestor rows hidden, indentation collapsed); a "parents" checkbox next to the filter box restores the ancestor context. Also records the batch in the changelog. Closes #1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bev9y4WSYSVyDr2SDrB98u
Review feedback (Copilot on #6): filtering lazy-scan hits with index.inFile(file).some(...) per reference is O(refs x defs-in-file), and big vanilla files carry 1000+ definitions. The sites of the looked-up name come from lookupAll(name) once (a handful), keyed with the same path normalization inFile used, so the filter is O(refs). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bev9y4WSYSVyDr2SDrB98u
…mer banner - pnpm testp packages a <version>-preview VSIX (manifest-only version, package.json untouched) and installs it via the code CLI, so trying the next iteration is one command. - Bug report form asks which editor (VS Code/Cursor/VSCodium/Windsurf) instead of the exact version, and the Data sources checkboxes now include the DumpDataTypes dump, which was always a loaded source but never listed. Feature request folds workarounds into the problem and proposal fields instead of a separate field. - Banner recompressed (630 KB -> 557 KB) and excluded from the VSIX. - Version bumped to 0.1.2 for the upcoming release. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
URI.fsPath uses backslashes on Windows, so comparing it against forward-slash literals only ever passed on the Linux CI runners. Normalize the separators like the #4 test above already does. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
JDeffner
force-pushed
the
claude/open-issues-d8p51u
branch
from
July 22, 2026 00:03
43ac656 to
936ce05
Compare
…focus The #1 follow-up shipped a "parents" checkbox that was only consulted while filter text existed — applyFilter()'s empty-query branch returned before reading it, so clicking it in the idle tree did nothing, and its polarity was inverted from the user's mental model (unchecked already meant "ancestors hidden"). Replaced with a proper toggle button: - While filtering, "Hide ancestors" (pressed by default) keeps the flat matches-only list; unpressing restores ancestor context. Same semantics, now visible as a pressed state instead of tooltip lore. - In the idle tree, clicking a row now selects it (single click previews the source line without stealing focus; double click jumps into the editor). With a selection, the toggle focuses the view on that node's subtree — the "hide parent nodes of the selected node" gesture. Esc clears the selection, and the button disables itself when it could have no effect, instead of silently ignoring clicks. - Hotkeys: `h` and `/` inside the panel (toggle / focus the filter), and a new ck3.guiTreeToggleParents command bound to ctrl+alt+h while the panel is active, so the gesture is discoverable and rebindable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ctrl+alt+t (edit localization) was the only binding; the rest of the daily drivers now ship defaults too: preview P, widget tree W, event graph G, dependencies D, tiger validate V, loc side-by-side L, jump to script reference J, .info docs O. All chords are scoped by editorLangId / context-key when-clauses so they are inert outside CK3 files, and the letters avoid AltGr-producing combinations on German-family layouts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The status bar item, the CK3 activity-bar container with its six views, the setup nudge and the CK3: palette commands appeared in every window, CK3-related or not. They are now gated on a real workspace predicate, cfg.isCk3Workspace: the workspace contains a mod (descriptor.mod or the usual content dirs), a game install, or ck3.modPath points at a mod — and ck3.enableForWorkspace is on. The machine-scope ck3.gamePath setting deliberately does not count, or one setup run would light the extension up in every window forever. The predicate re-evaluates on folder and setting changes, so adding a mod folder brings the UI up live. Bootstrap commands stay reachable everywhere (setup, tiger download, tutorial, image guidelines, DDS conversion, descriptor creation). ensureFileAssociations now trusts the same predicate, which also grants explorer icons to game-install workspaces. And bare .info files outside the game's _*.info naming are no longer claimed globally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
JDeffner
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes four of the five open issues. The issue screenshots were used to diagnose each report; two of the bugs were not where the titles pointed. The #5 fix (inline
scripted_trigger/scripted_effectdeclarations) has moved to theissue-5-inline-declarationsbranch for further iteration and is no longer part of this PR.Closes #1. Closes #2. Closes #3. Closes #4.
What changed
Go to definitions does not include all top level directories in the workspace #4 — Go to Definition does not include all top-level workspace directories (the
gamedir). Definition lookup shadow-resolved to the highest-priority source, hiding the vanilla/parent originals whenever a mod override existed. All sources are now listed, mod first — spotting an unintended override is exactly the point (as also requested in the Go to definition/reference navigation cannot find scripted_triggers/scripted_effects #5 discussion).Go to references navigation shows definitions #3 — Go to References shows definitions instead of references. Root cause: vanilla and read-only parent roots are never reference-indexed up front (memory guard, AD-4), so a name used only by vanilla files had zero usage sites and the peek showed only the definition entries. References now merge an on-demand scan over the un-indexed roots (
LazyReferenceScanner): exact-token matches per line, comments stripped, column-0 definition keys skipped, engine tokens and grammar keywords excluded, memoized per name until the roots change. The scan is textual on purpose — usage likeadd_dynasty_prestige = miniscule_dynasty_prestige_loss(the screenshot's own example) has no schema ref-field for the enclosing key, so a parse-based scan would miss it.Autocomplete for chaining datatypes in .gui and .yml files #2 — Autocomplete for chaining datatypes in
.guiand.ymlfiles. Server-side chain resolution was already correct; the gui/loc word pattern includes., so after[GetPlayer.the suggest widget filtered member labels against the whole dotted word (and accepting one would have replaced the entire chain). Datafunction completion items now carry an explicittextEditrange covering exactly the typed tail segment.Hide Parents in GUI Tree and add toggle to show them #1 — Hide parents in GUI tree, with a toggle to show them. Filtering the widget tree now renders a flat list of matches only (ancestor rows hidden, indentation collapsed). The first cut shipped this as a "parents" checkbox that silently did nothing without filter text; it is now a real "Hide ancestors" toggle button that also works in the idle tree: click a row to select it (single click previews the source line, double click jumps into the editor) and toggle (
hin the panel,Ctrl+Alt+Hanywhere) to focus on that node's subtree, Esc to clear. The button disables itself when it could have no effect.New
Ctrl+Alt+P, widget treeCtrl+Alt+W, event graphCtrl+Alt+G, dependenciesCtrl+Alt+D, run tigerCtrl+Alt+V, localization side-by-sideCtrl+Alt+L, jump to script referenceCtrl+Alt+J,.infodocsCtrl+Alt+O— all scoped to CK3 editors via when-clauses, AltGr-safe on German-family layouts, and freely remappable.CK3:palette commands only appear when the workspace holds a mod or a game install (newck3.isCk3Workspacecontext key, re-evaluated live on folder/setting changes). The machine-scopeck3.gamePathdeliberately does not count. Bootstrap commands (setup, tiger download, tutorial, DDS conversion, descriptor creation) stay reachable everywhere. Bare.infofiles outside the game's_*.infonaming are no longer claimed.Release housekeeping
pnpm testppackages a<version>-previewVSIX (manifest-only version, package.json untouched) and installs it via thecodeCLI.DumpDataTypesdump as a data source; the feature request form folds workarounds into the problem/proposal fields.URI.fsPathseparators were compared against forward-slash literals, so it only ever passed on the Linux CI runners).Note on the transparency remark in #3/#4: that is VS Code's native peek widget (its preview pane renders left of the list) and cannot be changed from an extension — it's a VSCodium/theme concern.
Testing
tsc --noEmitclean, esbuild bundle builds.🤖 Generated with Claude Code
https://claude.ai/code/session_01Bev9y4WSYSVyDr2SDrB98u