Repository navigation
feat: III Language Server (LSP) with VS Code extension - #8
Merged
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds a new Rust LSP server (iii-lsp) and a VS Code extension (iii-lsp-vscode), implements analyzer/completions/diagnostics/hover modules and an EngineClient cache, updates CI to run conditional jobs by path, and includes minor test and .gitignore adjustments. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant VSCode as VS Code
participant Ext as Extension\n(extension.js)
participant LSPClient as LanguageClient
participant Server as LSP Server\n(main.rs)
participant EngineClient as Engine Client
participant III as III Engine
User->>VSCode: Open file / edit
VSCode->>Ext: Activate extension
Ext->>LSPClient: Create LanguageClient
LSPClient->>Server: Connect (stdio)
Server->>EngineClient: new(url) / start()
EngineClient->>III: Register worker / List functions
III-->>EngineClient: Functions & trigger types
Server-->>LSPClient: Initialized
User->>VSCode: Request completion / hover
VSCode->>LSPClient: textDocument/completion or hover
LSPClient->>Server: completion/hover request
Server->>Server: analyze(source, position)
Server->>EngineClient: get_function / get_trigger_type / known values
EngineClient-->>Server: Metadata & schemas
Server->>Server: get_completions / get_hover
Server-->>LSPClient: CompletionItem[] / Hover
LSPClient-->>VSCode: Show suggestions / hover
sequenceDiagram
actor User
participant VSCode as VS Code
participant Server as LSP Server
participant Analyzer as Analyzer\n(tree-sitter)
participant Diagnostics as Diagnostics
participant EngineClient as Engine Client
User->>VSCode: Edit document
VSCode->>Server: textDocument/didChange
Server->>Analyzer: analyze(source)
Analyzer-->>Server: CompletionContext
Server->>Diagnostics: diagnose(source)
Diagnostics->>EngineClient: get_function/get_trigger_type
EngineClient-->>Diagnostics: Schema data
Diagnostics-->>Server: Diagnostic[]
Server->>VSCode: publishDiagnostics
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
guibeira
changed the title
guibeira
marked this pull request as ready for review
April 6, 2026 21:18
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (10)
iii-lsp-vscode/package.json (2)
40-42: Consider addingdevDependenciesfor development.Adding
@types/vscodeand@types/nodewould enable TypeScript type checking in editors, improving the development experience."dependencies": { "vscode-languageclient": "^9.0.1" + }, + "devDependencies": { + "@types/vscode": "^1.75.0", + "@types/node": "^20.0.0" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iii-lsp-vscode/package.json` around lines 40 - 42, Add a devDependencies section to package.json and include TypeScript type packages to improve editor/type-checking: add "@types/vscode" and "@types/node" under "devDependencies" alongside the existing "dependencies" (which currently contains "vscode-languageclient") so editors and build tools can resolve VS Code and Node types during development.
33-35: Placeholder test script.The test script currently just exits with an error. Consider either removing it or adding actual extension tests using
@vscode/test-electron.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iii-lsp-vscode/package.json` around lines 33 - 35, The package.json currently has a placeholder "test" script that always fails; replace it with a real VS Code extension test script or remove it. Install the `@vscode/test-electron` dev dependency, add a proper test entry under "scripts" (replace the "test" key) that invokes your test runner (e.g., the test runner bootstrap like runTest.js or the `@vscode/test-electron` runCLI/ run function used by your test harness), and ensure your extension's test bootstrap (e.g., test/runTest or test/index) is wired up to launch the VS Code test runner; alternatively simply remove the "test" script if you don't want tests. Keep the change focused on the "test" script value in package.json so CI and local npm test behave correctly.iii-lsp-vscode/README.md (1)
67-70: ClarifyserverPathdefault behavior.The table states default is
"", butextension.jsfalls back to"iii-lsp"when the setting is empty. The description is accurate ("usesiii-lspfrom PATH"), but consider updating the default column for clarity:-| `iii-lsp.serverPath` | `""` (uses `iii-lsp` from PATH) | Path to the `iii-lsp` binary | +| `iii-lsp.serverPath` | `"iii-lsp"` | Path to the `iii-lsp` binary (defaults to PATH lookup) |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iii-lsp-vscode/README.md` around lines 67 - 70, Update the README table to accurately reflect the runtime default for the server path: change the Default cell for the `iii-lsp.serverPath` setting from `""` to `"iii-lsp"` (or `"" (defaults to "iii-lsp" from PATH)`) and clarify the description to note that when the setting is empty the extension.js fallback uses the literal `"iii-lsp"` from PATH; reference the `iii-lsp.serverPath` setting and the `extension.js` fallback value `"iii-lsp"` to locate the relevant documentation text.iii-lsp/Cargo.toml (1)
12-12: Consider adding a rev/tag for the git dependency.Pinning to a specific commit or tag prevents unexpected breakage from upstream changes and improves reproducible builds.
-iii-sdk = { git = "ssh://git@github.com/iii-hq/iii.git" } +iii-sdk = { git = "ssh://git@github.com/iii-hq/iii.git", rev = "<commit-sha>" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iii-lsp/Cargo.toml` at line 12, The git dependency iii-sdk in Cargo.toml is unpinned; update the dependency entry for iii-sdk to include a specific tag or commit (e.g., add tag = "vX.Y.Z" or rev = "abcdef123456...") so the crate is pinned for reproducible builds and to avoid upstream breakage; change the iii-sdk line to include the chosen rev or tag value and commit the updated Cargo.toml.iii-lsp-vscode/extension.js (1)
40-40: Handleclient.start()promise and potential errors.
client.start()returns aPromise. If it fails (e.g., binary not found), the error is silently swallowed. Consider returning it or adding error handling.Proposed improvement
- client.start(); + return client.start().catch((err) => { + console.error("Failed to start III LSP server:", err); + throw err; + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iii-lsp-vscode/extension.js` at line 40, client.start() returns a Promise and currently any startup failure is swallowed; update the activation flow in extension.js (where client.start() is called) to handle the promise by either returning/awaiting it from the activate function or attaching a .catch handler that logs the error and surfaces it to the user (e.g., via console.error/processLogger and vscode.window.showErrorMessage), and ensure any resources are cleaned up on failure (stop client via client.stop() if started). Locate the client.start() call and replace the fire-and-forget call with an awaited/returned promise or a .catch that logs and handles the error.iii-lsp/src/hover.rs (1)
70-114: Consider adding integration tests forget_hover.Current tests validate string formatting but don't exercise
get_hoverwith a mockEngineClient. This would provide better coverage of the lookup logic and edge cases (e.g., function without description, missing request/response formats).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iii-lsp/src/hover.rs` around lines 70 - 114, Add integration tests that call the get_hover function using a mocked EngineClient instead of only testing string formatting; implement a test harness that constructs a fake/mocked EngineClient (or trait impl) returning controlled data for functions/triggers (including cases: function with description, function without description, missing request/response schemas, and trigger entries) and assert the returned Hover/TextDocumentHover contents contain the expected markdown snippets and edge-case fallbacks. Locate get_hover and the EngineClient trait/struct in hover.rs (and related engine client module) and create tests that exercise get_hover end-to-end by injecting the mock client and verifying lookup logic, formatting of description, worker name, request/response JSON blocks, and behavior when fields are absent..github/workflows/ci.yml (1)
26-30: Consider workflow behavior for shared files.Changes to files outside
image-resize/**andiii-lsp/**(e.g., root-level configs, shared libraries, or the CI workflow itself) won't trigger any test jobs. If the repository grows with shared dependencies, you may want to add a fallback or include common paths.Example: trigger on workflow changes
iii-lsp: - 'iii-lsp/**' + ci: + - '.github/workflows/ci.yml'Then add a job that runs on
cichanges to validate the workflow syntax or re-run all tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 26 - 30, The current CI path filters under the "filters" block only trigger on changes within 'image-resize/**' and 'iii-lsp/**', so edits to shared files or workflow config won't run tests; update the workflow to include common/shared paths or a fallback (e.g., add 'ci/**', '.github/**', 'package.json', 'shared/**', or a catch-all) in the filters and/or add a dedicated job that triggers on changes to the CI workflow itself (referencing the "filters" block and the 'image-resize' and 'iii-lsp' patterns) so modifying root-level configs or shared libraries will run the appropriate test jobs.iii-lsp/src/main.rs (1)
114-117: Engine shutdown is called twice.
engine.shutdown().awaitis called inLanguageServer::shutdown()(Line 115) and again at the end ofmain()(Line 215). Whileshutdown_async()may be idempotent, this is redundant and could mask issues if the shutdown behavior changes.♻️ Consider removing the redundant shutdown
Either remove the shutdown in
main()(relying on the LSP lifecycle) or remove it fromLanguageServer::shutdown()and keep only the final one inmain(). The latter is safer as it ensures cleanup even if the LSP shutdown handler isn't called.async fn shutdown(&self) -> Result<()> { - self.engine.shutdown().await; + // Shutdown handled in main() after server exits Ok(()) }Also applies to: 215-215
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iii-lsp/src/main.rs` around lines 114 - 117, LanguageServer::shutdown() currently calls self.engine.shutdown().await while main() also calls engine.shutdown() at exit, causing a duplicate shutdown; decide which place should own engine shutdown and remove the redundant call accordingly — either delete the engine.shutdown().await from LanguageServer::shutdown() so main()'s final cleanup (engine.shutdown()) remains the single shutdown point, or remove the call in main() and keep LanguageServer::shutdown() as the LSP lifecycle-managed shutdown; update only the duplicate call (references: LanguageServer::shutdown(), engine.shutdown(), main()) and ensure any tests or comments reflect the chosen single shutdown location.iii-lsp/src/diagnostics.rs (1)
552-552: Consider making HTTP methods case-insensitive.
VALID_HTTP_METHODScontains uppercase values, but user input might be lowercase (e.g.,"get"instead of"GET"). The comparison on Line 622 is case-sensitive.♻️ Proposed fix for case-insensitive comparison
- if key == "http_method" && !VALID_HTTP_METHODS.contains(&value.as_str()) { + if key == "http_method" && !VALID_HTTP_METHODS.contains(&value.to_uppercase().as_str()) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iii-lsp/src/diagnostics.rs` at line 552, VALID_HTTP_METHODS currently holds uppercase strings and the runtime check compares user input case-sensitively; update the comparison to be case-insensitive by normalizing the input or the constants. For example, either store VALID_HTTP_METHODS as lowercase and compare using method.to_ascii_lowercase(), or keep the uppercase constants and compare using method.eq_ignore_ascii_case(constant) at the site where the input method is checked against VALID_HTTP_METHODS (the comparison around the current check of the HTTP method). Ensure all places that validate the method use the same case-insensitive approach and update any tests accordingly.iii-lsp/src/engine_client.rs (1)
139-143: Idiomatic improvement: usecloned()instead ofmap(|v| v.clone()).♻️ Proposed fix
pub fn get_known_values(&self, field_name: &str) -> Vec<String> { match field_name { - "stream_name" => self.known_stream_names.iter().map(|v| v.clone()).collect(), - "topic" => self.known_topics.iter().map(|v| v.clone()).collect(), - "api_path" => self.known_api_paths.iter().map(|v| v.clone()).collect(), - "scope" => self.known_scopes.iter().map(|v| v.clone()).collect(), - "queue" => self.known_topics.iter().map(|v| v.clone()).collect(), + "stream_name" => self.known_stream_names.iter().map(|r| r.key().clone()).collect(), + "topic" => self.known_topics.iter().map(|r| r.key().clone()).collect(), + "api_path" => self.known_api_paths.iter().map(|r| r.key().clone()).collect(), + "scope" => self.known_scopes.iter().map(|r| r.key().clone()).collect(), + "queue" => self.known_topics.iter().map(|r| r.key().clone()).collect(), _ => Vec::new(), } }Note: DashSet iterator returns
RefMultiwhich derefs to the value, so the original code works butr.key().clone()is more explicit about what's being cloned.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iii-lsp/src/engine_client.rs` around lines 139 - 143, Replace the verbose .iter().map(|v| v.clone()).collect() calls for known_stream_names, known_topics, known_api_paths, and known_scopes with the idiomatic .iter().cloned().collect() (or, if iterating a DashSet RefMulti, use .iter().map(|r| r.key().clone()).collect()) so the "stream_name", "topic", "api_path", and "scope" (and the "queue" line that reuses known_topics) entries use .cloned() instead of map(|v| v.clone()) for clearer, more concise cloning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@iii-lsp-vscode/package.json`:
- Around line 1-5: The package.json is missing the required "publisher" field
needed by vsce packaging; add a "publisher" property to package.json (alongside
existing keys like "name", "displayName", "description", and "version") with
your VS Code Marketplace publisher identifier (e.g., "publisher":
"your-publisher-name") so vsce package can produce a VSIX.
In `@iii-lsp/src/analyzer.rs`:
- Around line 221-239: position_to_byte_offset currently treats
Position.character as a byte index which breaks on non-ASCII/UTF-8 characters;
update the function to interpret position.character as UTF-16 code units (LSP
semantics) and convert that to a byte offset by iterating the target line with
line.char_indices(), accumulating utf16 unit counts using ch.len_utf16() until
reaching or exceeding position.character, then return byte_offset + the
corresponding byte index (or end of line if the UTF-16 column is past the line);
keep the existing loop over lines and the final-case when position.line ==
source.lines().count() returning source.len().
- Around line 127-134: The branch that handles odd double_quotes constructs a
patched string but never returns it; update the branch so that after building
and inserting the suffix you return the patched result (e.g., return
Some(patched)) instead of falling through to Option::None; ensure the return
type matches the function's Option<T> (using Some(patched)) and keep the call to
build_closing_suffix and insert_str unchanged.
In `@iii-lsp/src/completions.rs`:
- Around line 204-208: The test none_context_returns_empty currently only
constructs an empty Vec; replace it so it calls get_completions with
CompletionContext::None (and any required dummy Params/Context) and then assert
the returned Vec<CompletionItem> is empty; update the test function name or body
to invoke get_completions(...) and assert!(result.is_empty()) referencing the
get_completions function and CompletionContext::None to ensure the actual
behavior is verified.
- Around line 134-168: The test filters_engine_functions is invalid because it
hand-iterates a DashMap instead of exercising the real function get_completions;
update the test to call get_completions (or the public wrapper that uses it)
with a prepared functions map and a realistic current_text value so the
namespace-based filtering is exercised, include both a "todos::create" and an
"engine::functions::list" FunctionInfo in the dataset and assert that
get_completions returns only the expected suggestions for the namespace implied
by current_text; if get_completions depends on EngineClient or other external
state, replace that dependency with a lightweight mock or construct the required
input structures directly so the unit test targets get_completions behavior
rather than manual DashMap iteration.
In `@iii-lsp/src/diagnostics.rs`:
- Around line 127-134: The function patch_unclosed_string currently builds a
corrected string when it detects an odd number of double quotes but fails to
return it, falling through to the final Option::None; update
patch_unclosed_string to return Some(patched_string) immediately after
constructing the patched result for the double_quotes % 2 == 1 branch (mirroring
the existing single-quote branch behavior), ensuring the function returns the
patched string instead of None.
- Around line 638-656: The cron validation enforces a 6-field (sec min hour day
month weekday) Quartz-style expression which can confuse users of standard
5-field POSIX cron; update diagnostics.rs by adding a brief code comment above
the check for call.trigger_type == "cron" stating that this is Quartz-style
cron, and enhance the Diagnostic message built in the loop (the one using
call.config_range and config_values) to explicitly mention "Quartz-style 6-field
cron (sec min hour day month weekday)" so users know which format is expected.
In `@iii-lsp/src/engine_client.rs`:
- Around line 51-70: The callback currently captures Arc::clone(self) creating a
reference cycle between EngineClient and the guard returned by
iii.on_functions_available; change the closure to capture a Weak<EngineClient>
(use Arc::downgrade(self)) and inside the closure call weak.upgrade() to get an
Arc and return early if upgrade() yields None, then operate on the upgraded Arc
to clear/insert into client.functions and spawn the task that calls
reseed_secondary_caches(); update any variable names accordingly (start method,
the on_functions_available closure, reseed_secondary_caches, and the self.guard
storage) so the guard no longer holds a strong Arc to EngineClient.
In `@iii-lsp/src/main.rs`:
- Around line 119-124: The did_open handler runs run_diagnostics before storing
the document which can cause a race if another request queries documents; modify
async fn did_open to insert the document into self.documents first (using the
same uri/text values) and then await self.run_diagnostics(uri.clone(),
&text).await so diagnostics run after the document is available; update
references to uri/text as needed to preserve ownership (e.g., clone uri where
required).
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 26-30: The current CI path filters under the "filters" block only
trigger on changes within 'image-resize/**' and 'iii-lsp/**', so edits to shared
files or workflow config won't run tests; update the workflow to include
common/shared paths or a fallback (e.g., add 'ci/**', '.github/**',
'package.json', 'shared/**', or a catch-all) in the filters and/or add a
dedicated job that triggers on changes to the CI workflow itself (referencing
the "filters" block and the 'image-resize' and 'iii-lsp' patterns) so modifying
root-level configs or shared libraries will run the appropriate test jobs.
In `@iii-lsp-vscode/extension.js`:
- Line 40: client.start() returns a Promise and currently any startup failure is
swallowed; update the activation flow in extension.js (where client.start() is
called) to handle the promise by either returning/awaiting it from the activate
function or attaching a .catch handler that logs the error and surfaces it to
the user (e.g., via console.error/processLogger and
vscode.window.showErrorMessage), and ensure any resources are cleaned up on
failure (stop client via client.stop() if started). Locate the client.start()
call and replace the fire-and-forget call with an awaited/returned promise or a
.catch that logs and handles the error.
In `@iii-lsp-vscode/package.json`:
- Around line 40-42: Add a devDependencies section to package.json and include
TypeScript type packages to improve editor/type-checking: add "@types/vscode"
and "@types/node" under "devDependencies" alongside the existing "dependencies"
(which currently contains "vscode-languageclient") so editors and build tools
can resolve VS Code and Node types during development.
- Around line 33-35: The package.json currently has a placeholder "test" script
that always fails; replace it with a real VS Code extension test script or
remove it. Install the `@vscode/test-electron` dev dependency, add a proper test
entry under "scripts" (replace the "test" key) that invokes your test runner
(e.g., the test runner bootstrap like runTest.js or the `@vscode/test-electron`
runCLI/ run function used by your test harness), and ensure your extension's
test bootstrap (e.g., test/runTest or test/index) is wired up to launch the VS
Code test runner; alternatively simply remove the "test" script if you don't
want tests. Keep the change focused on the "test" script value in package.json
so CI and local npm test behave correctly.
In `@iii-lsp-vscode/README.md`:
- Around line 67-70: Update the README table to accurately reflect the runtime
default for the server path: change the Default cell for the
`iii-lsp.serverPath` setting from `""` to `"iii-lsp"` (or `"" (defaults to
"iii-lsp" from PATH)`) and clarify the description to note that when the setting
is empty the extension.js fallback uses the literal `"iii-lsp"` from PATH;
reference the `iii-lsp.serverPath` setting and the `extension.js` fallback value
`"iii-lsp"` to locate the relevant documentation text.
In `@iii-lsp/Cargo.toml`:
- Line 12: The git dependency iii-sdk in Cargo.toml is unpinned; update the
dependency entry for iii-sdk to include a specific tag or commit (e.g., add tag
= "vX.Y.Z" or rev = "abcdef123456...") so the crate is pinned for reproducible
builds and to avoid upstream breakage; change the iii-sdk line to include the
chosen rev or tag value and commit the updated Cargo.toml.
In `@iii-lsp/src/diagnostics.rs`:
- Line 552: VALID_HTTP_METHODS currently holds uppercase strings and the runtime
check compares user input case-sensitively; update the comparison to be
case-insensitive by normalizing the input or the constants. For example, either
store VALID_HTTP_METHODS as lowercase and compare using
method.to_ascii_lowercase(), or keep the uppercase constants and compare using
method.eq_ignore_ascii_case(constant) at the site where the input method is
checked against VALID_HTTP_METHODS (the comparison around the current check of
the HTTP method). Ensure all places that validate the method use the same
case-insensitive approach and update any tests accordingly.
In `@iii-lsp/src/engine_client.rs`:
- Around line 139-143: Replace the verbose .iter().map(|v| v.clone()).collect()
calls for known_stream_names, known_topics, known_api_paths, and known_scopes
with the idiomatic .iter().cloned().collect() (or, if iterating a DashSet
RefMulti, use .iter().map(|r| r.key().clone()).collect()) so the "stream_name",
"topic", "api_path", and "scope" (and the "queue" line that reuses known_topics)
entries use .cloned() instead of map(|v| v.clone()) for clearer, more concise
cloning.
In `@iii-lsp/src/hover.rs`:
- Around line 70-114: Add integration tests that call the get_hover function
using a mocked EngineClient instead of only testing string formatting; implement
a test harness that constructs a fake/mocked EngineClient (or trait impl)
returning controlled data for functions/triggers (including cases: function with
description, function without description, missing request/response schemas, and
trigger entries) and assert the returned Hover/TextDocumentHover contents
contain the expected markdown snippets and edge-case fallbacks. Locate get_hover
and the EngineClient trait/struct in hover.rs (and related engine client module)
and create tests that exercise get_hover end-to-end by injecting the mock client
and verifying lookup logic, formatting of description, worker name,
request/response JSON blocks, and behavior when fields are absent.
In `@iii-lsp/src/main.rs`:
- Around line 114-117: LanguageServer::shutdown() currently calls
self.engine.shutdown().await while main() also calls engine.shutdown() at exit,
causing a duplicate shutdown; decide which place should own engine shutdown and
remove the redundant call accordingly — either delete the
engine.shutdown().await from LanguageServer::shutdown() so main()'s final
cleanup (engine.shutdown()) remains the single shutdown point, or remove the
call in main() and keep LanguageServer::shutdown() as the LSP lifecycle-managed
shutdown; update only the duplicate call (references:
LanguageServer::shutdown(), engine.shutdown(), main()) and ensure any tests or
comments reflect the chosen single shutdown location.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0dab1dc1-b298-4f14-93ca-1a57d9cdb935
⛔ Files ignored due to path filters (3)
iii-lsp-vscode/package-lock.jsonis excluded by!**/package-lock.jsoniii-lsp/Cargo.lockis excluded by!**/*.lockimage-resize/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
.github/workflows/ci.ymliii-lsp-vscode/.gitignoreiii-lsp-vscode/README.mdiii-lsp-vscode/extension.jsiii-lsp-vscode/package.jsoniii-lsp/Cargo.tomliii-lsp/src/analyzer.rsiii-lsp/src/completions.rsiii-lsp/src/diagnostics.rsiii-lsp/src/engine_client.rsiii-lsp/src/hover.rsiii-lsp/src/main.rsimage-resize/src/manifest.rs
Comment on lines
+134
to
+168
| #[test] | ||
| fn filters_engine_functions() { | ||
| let functions: DashMap<String, FunctionInfo> = DashMap::new(); | ||
| functions.insert( | ||
| "todos::create".to_string(), | ||
| FunctionInfo { | ||
| function_id: "todos::create".to_string(), | ||
| description: Some("Create a todo".to_string()), | ||
| request_format: None, | ||
| response_format: None, | ||
| metadata: None, | ||
| }, | ||
| ); | ||
| functions.insert( | ||
| "engine::functions::list".to_string(), | ||
| FunctionInfo { | ||
| function_id: "engine::functions::list".to_string(), | ||
| description: Some("Internal".to_string()), | ||
| request_format: None, | ||
| response_format: None, | ||
| metadata: None, | ||
| }, | ||
| ); | ||
|
|
||
| let mut items = Vec::new(); | ||
| for entry in functions.iter() { | ||
| let func: &FunctionInfo = entry.value(); | ||
| if !func.function_id.starts_with("engine::") { | ||
| items.push(func.function_id.clone()); | ||
| } | ||
| } | ||
|
|
||
| assert_eq!(items.len(), 1); | ||
| assert_eq!(items[0], "todos::create"); | ||
| } |
There was a problem hiding this comment.
Test doesn't exercise get_completions function.
The test filters_engine_functions manually iterates over a DashMap and checks for engine:: prefix filtering, but this logic doesn't exist in get_completions. The actual get_completions function at Lines 22-37 filters by namespace prefix from current_text, not by excluding engine:: prefixed functions.
This test verifies behavior that isn't implemented in the production code.
💡 Suggested test that exercises actual function
Consider testing the actual get_completions function with a mock EngineClient or testing that namespace filtering via current_text works correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@iii-lsp/src/completions.rs` around lines 134 - 168, The test
filters_engine_functions is invalid because it hand-iterates a DashMap instead
of exercising the real function get_completions; update the test to call
get_completions (or the public wrapper that uses it) with a prepared functions
map and a realistic current_text value so the namespace-based filtering is
exercised, include both a "todos::create" and an "engine::functions::list"
FunctionInfo in the dataset and assert that get_completions returns only the
expected suggestions for the namespace implied by current_text; if
get_completions depends on EngineClient or other external state, replace that
dependency with a lightweight mock or construct the required input structures
directly so the unit test targets get_completions behavior rather than manual
DashMap iteration.
Comment on lines
+204
to
+208
| #[test] | ||
| fn none_context_returns_empty() { | ||
| let items: Vec<CompletionItem> = Vec::new(); | ||
| assert!(items.is_empty()); | ||
| } |
There was a problem hiding this comment.
Test none_context_returns_empty is trivially true.
This test creates an empty Vec and asserts it's empty. It doesn't test get_completions with CompletionContext::None.
💡 Suggested fix
The test should actually call get_completions with CompletionContext::None and verify it returns an empty vector.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@iii-lsp/src/completions.rs` around lines 204 - 208, The test
none_context_returns_empty currently only constructs an empty Vec; replace it so
it calls get_completions with CompletionContext::None (and any required dummy
Params/Context) and then assert the returned Vec<CompletionItem> is empty;
update the test function name or body to invoke get_completions(...) and
assert!(result.is_empty()) referencing the get_completions function and
CompletionContext::None to ensure the actual behavior is verified.
Comment on lines
+127
to
+134
| } | ||
|
|
||
| fn is_trigger_method(name: &str) -> bool { | ||
| name == "trigger" | ||
| } | ||
|
|
||
| fn is_register_trigger_method(name: &str) -> bool { | ||
| name == "registerTrigger" || name == "register_trigger" |
There was a problem hiding this comment.
Missing return statement causes silent failure for double-quoted unclosed strings.
In patch_unclosed_string, when double_quotes % 2 == 1 (Line 127-131), the function builds a patched string but never returns it. The function falls through to Option::None on Line 133.
🐛 Proposed fix
This is in analyzer.rs, but the same pattern applies - noting for cross-reference. The actual fix should be in analyzer.rs lines 127-134.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@iii-lsp/src/diagnostics.rs` around lines 127 - 134, The function
patch_unclosed_string currently builds a corrected string when it detects an odd
number of double quotes but fails to return it, falling through to the final
Option::None; update patch_unclosed_string to return Some(patched_string)
immediately after constructing the patched result for the double_quotes % 2 == 1
branch (mirroring the existing single-quote branch behavior), ensuring the
function returns the patched string instead of None.
Comment on lines
+638
to
+656
| if call.trigger_type == "cron" { | ||
| for (key, value) in &call.config_values { | ||
| if key == "expression" { | ||
| let fields: Vec<&str> = value.split_whitespace().collect(); | ||
| if fields.len() != 6 { | ||
| diagnostics.push(Diagnostic { | ||
| range: call.config_range, | ||
| severity: Some(DiagnosticSeverity::ERROR), | ||
| source: Some("iii-lsp".to_string()), | ||
| message: format!( | ||
| "Cron expression must have 6 fields (sec min hour day month weekday), got {}", | ||
| fields.len() | ||
| ), | ||
| ..Default::default() | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the standard cron expression format - 5 fields or 6 fields?
💡 Result:
In traditional/“standard” Unix cron (Vixie/ISC cron as documented by crontab(5)), a cron schedule is 5 fields:
minute hour day-of-month month day-of-week (then the command). [1][2]
A 6-field cron expression (adding a leading seconds field) is not standard Unix cron—it’s used by Quartz-style schedulers (often 6 required fields, plus an optional year as a 7th). [3][4]
Also note: in system crontabs like /etc/crontab, there’s often an extra user column, but that’s not an extra time field. [1]
🏁 Script executed:
# First, let's understand the context around the validation code
cd iii-lsp && head -n 700 src/diagnostics.rs | tail -n 100Repository: iii-hq/workers
Length of output: 3776
🏁 Script executed:
# Find the extract_object_string_values and strip_quotes functions
rg -n "extract_object_string_values|strip_quotes" iii-lsp/src --type rust -B 2 -A 5Repository: iii-hq/workers
Length of output: 7877
🏁 Script executed:
# Check for test cases related to cron validation
rg -n "cron" iii-lsp/src --type rust -i -B 2 -A 5Repository: iii-hq/workers
Length of output: 1217
🏁 Script executed:
# Look at the full diagnostic section to understand the context
sed -n '600,700p' iii-lsp/src/diagnostics.rsRepository: iii-hq/workers
Length of output: 3814
🏁 Script executed:
# Search for cron-related tests
rg -n "cron" iii-lsp/src/diagnostics.rs -A 10Repository: iii-hq/workers
Length of output: 717
🏁 Script executed:
# Look for any configuration examples or documentation about cron format
fd -e md -e txt -e json -e toml | xargs rg -l "cron|expression" | head -10Repository: iii-hq/workers
Length of output: 82
🏁 Script executed:
# Check if there are any existing tests for the cron validation
rg -n "test.*cron|cron.*test" iii-lsp/src --type rust -i -B 2 -A 10Repository: iii-hq/workers
Length of output: 40
🏁 Script executed:
# Check the README for cron format documentation
cat iii-lsp-vscode/README.md | head -100Repository: iii-hq/workers
Length of output: 2082
🏁 Script executed:
# Look for any documentation or examples in the main codebase
find . -name "README*" -o -name "*.md" | xargs grep -l "cron" 2>/dev/null | head -5Repository: iii-hq/workers
Length of output: 84
🏁 Script executed:
# Search for where trigger configuration is defined or documented
rg -n "trigger.*type.*cron|cron.*trigger" --type ts --type js --type py -B 2 -A 5 | head -50Repository: iii-hq/workers
Length of output: 40
🏁 Script executed:
# Verify the exact flow of how config_values are created - look at the complete extract_object_string_values function
sed -n '437,455p' iii-lsp/src/diagnostics.rsRepository: iii-hq/workers
Length of output: 772
🏁 Script executed:
# Check if there's any documentation about the expected cron format in comments or elsewhere
rg -n "Quartz|seconds|sec min|6.*field" iii-lsp/src --type rustRepository: iii-hq/workers
Length of output: 195
🏁 Script executed:
# Look for any integration tests or examples showing cron usage
rg -n "expression.*\d|cron.*expression" --type ts --type js --type py | head -20Repository: iii-hq/workers
Length of output: 40
🏁 Script executed:
# Verify if strip_quotes handles all edge cases - test with examples
python3 << 'EOF'
def strip_quotes(s):
return s.strip("'").strip('"')
# Test cases
test_cases = [
'"0 * * * * *"', # Normal quoted cron
"'0 * * * * *'", # Single quoted
'0 * * * * *', # No quotes
'"0 * * \\* * *"', # Escaped char (raw string)
'expression: "0 * * * * *"', # With key (shouldn't reach here)
]
for test in test_cases:
result = strip_quotes(test)
fields = result.split()
print(f"Input: {test!r} → {result!r} → {len(fields)} fields")
EOFRepository: iii-hq/workers
Length of output: 334
🏁 Script executed:
# Check if there are any example configurations or tests showing expected trigger setup
find . -type f \( -name "*.ts" -o -name "*.js" -o -name "*.py" \) -exec grep -l "registerTrigger.*cron\|trigger.*type.*cron" {} \; | head -5Repository: iii-hq/workers
Length of output: 40
🏁 Script executed:
# Look at complete context of cron validation - check if there are comments explaining the 6-field choice
sed -n '625,660p' iii-lsp/src/diagnostics.rsRepository: iii-hq/workers
Length of output: 1319
Document the cron expression format requirement.
The validation enforces a 6-field format (sec min hour day month weekday), which is Quartz-style cron, not standard POSIX cron (5 fields). Add a comment or documentation clarifying which cron format is expected, so users understand why their expressions may be rejected if they follow standard Unix cron conventions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@iii-lsp/src/diagnostics.rs` around lines 638 - 656, The cron validation
enforces a 6-field (sec min hour day month weekday) Quartz-style expression
which can confuse users of standard 5-field POSIX cron; update diagnostics.rs by
adding a brief code comment above the check for call.trigger_type == "cron"
stating that this is Quartz-style cron, and enhance the Diagnostic message built
in the loop (the one using call.config_range and config_values) to explicitly
mention "Quartz-style 6-field cron (sec min hour day month weekday)" so users
know which format is expected.
- Fix missing return in patch_unclosed_string for double quotes - Convert UTF-16 code units to byte offset in position_to_byte_offset - Add publisher field to vscode extension package.json - Store document before running diagnostics to prevent race condition - Use Weak references in engine_client callback to prevent Arc cycle - Add comment documenting Quartz-style cron format
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
iii-lsp/src/diagnostics.rs (1)
660-758: Consider adding Rust language tests for completeness.The test suite covers TypeScript and Python thoroughly, including both dict-style and keyword-argument patterns for Python. However, there are no tests for Rust struct expression parsing (e.g.,
iii.trigger(TriggerArgs { function_id: "foo::bar".to_string(), ... })).📝 Example Rust test to add
fn parse_rs(source: &str) -> tree_sitter::Tree { let mut parser = Parser::new(); parser .set_language(&tree_sitter_rust::LANGUAGE.into()) .unwrap(); parser.parse(source, None).unwrap() } #[test] fn rs_finds_trigger_calls() { let source = r#"iii.trigger(TriggerArgs { function_id: "todos::create".to_string(), payload: Payload { title: "test".into() } })"#; let tree = parse_rs(source); let (calls, _) = find_all_calls(tree.root_node(), source); assert_eq!(calls.len(), 1); assert_eq!(calls[0].function_id, "todos::create"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iii-lsp/src/diagnostics.rs` around lines 660 - 758, Add Rust-language unit tests to mirror the existing TypeScript and Python tests: implement a parse_rs helper (using tree_sitter_rust::LANGUAGE) and add tests like rs_finds_trigger_calls that call find_all_calls on a Rust trigger invocation (e.g., iii.trigger(TriggerArgs { function_id: "...", payload: ... })) to assert detection of function_id, payload presence and payload/config keys; place these alongside parse_ts/parse_py and the other tests in the tests module so parse_rs and the new rs_* tests exercise the same find_all_calls code paths.iii-lsp/src/analyzer.rs (4)
116-117: Quote counting doesn't account for escape sequences.The quote counting logic counts all occurrences of
'and"without considering backslash escapes. For strings like'it\'s', the escaped quote will be counted, producing an incorrect odd count and potentially wrong patching.This is unlikely to affect typical function IDs, but could cause incorrect behavior for edge cases with escaped quotes.
♻️ Sketch of escape-aware counting
- let single_quotes = line_before.chars().filter(|&c| c == '\'').count(); - let double_quotes = line_before.chars().filter(|&c| c == '"').count(); + let single_quotes = count_unescaped_quotes(line_before, '\''); + let double_quotes = count_unescaped_quotes(line_before, '"');fn count_unescaped_quotes(s: &str, quote: char) -> usize { let mut count = 0; let mut chars = s.chars().peekable(); while let Some(ch) = chars.next() { if ch == '\\' { chars.next(); // skip escaped char } else if ch == quote { count += 1; } } count }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iii-lsp/src/analyzer.rs` around lines 116 - 117, The current quote counting in analyzer.rs uses single_quotes and double_quotes by counting all '\'' and '"' characters and thus miscounts escaped quotes (e.g., "it\'s"); replace that logic with an escape-aware routine (e.g., implement and use a function like count_unescaped_quotes(s: &str, quote: char)) that skips characters immediately following a backslash so only unescaped quote characters are counted, and call it instead of the current .filter(...).count() usages for single_quotes and double_quotes.
693-705: Early return?may skip valid pairs.Line 694 uses
?which returnsNoneimmediately ifutf8_textfails for any pair's key. This could skip subsequent pairs that might contain the target key. Consider usingcontinueinstead of?to be more resilient.♻️ More resilient iteration
if child.kind() == "pair" { - if let Some(key) = child.child_by_field_name("key") { - let key_text = strip_quotes(key.utf8_text(source.as_bytes()).ok()?); + let key = match child.child_by_field_name("key") { + Some(k) => k, + None => continue, + }; + let key_text = match key.utf8_text(source.as_bytes()) { + Ok(t) => strip_quotes(t), + Err(_) => continue, + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iii-lsp/src/analyzer.rs` around lines 693 - 705, The current loop uses the try operator on key.utf8_text(...) which causes the function to return None early if utf8_text fails for any child; change the logic in the iteration that checks child.child_by_field_name("key") so that you handle the utf8_text error locally (e.g., match or if let Ok(text) = key.utf8_text(...) { let key_text = strip_quotes(text); ... } else { continue }) rather than using ?, so the loop continues to the next pair when utf8_text fails while still checking subsequent pairs for target_key; apply this change around the calls to child.child_by_field_name("key"), strip_quotes, target_key comparison, and extract_string_content.
92-98: Edge case: surrogate pairs in UTF-16 positions.Decrementing
position.characterby 1 assumes each character is a single UTF-16 code unit. For characters outside the BMP (emoji, some CJK), this could land in the middle of a surrogate pair. This is unlikely in typical code but could cause incorrect cursor positioning.For an LSP analyzer, this is a minor edge case since identifiers rarely contain such characters.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iii-lsp/src/analyzer.rs` around lines 92 - 98, The current decrement of position.character in analyze_source can land inside a UTF-16 surrogate pair for non-BMP characters; update the logic to compute the previous valid UTF-16 code unit boundary instead of blindly doing `position.character - 1`. Use the source line text (the same string passed into analyze_source) and its UTF-16 encoding (e.g., str::encode_utf16) to build the u16 code-unit sequence for the target line, then find the previous code-unit index that corresponds to a character boundary (if the current index points to a low surrogate, step back one more u16) and create the new Position with that adjusted character index before calling analyze_source recursively; keep references to Position, position.character, and analyze_source to locate where to change the decrement.
736-752: Consider adding non-ASCII test cases.The test suite is comprehensive for ASCII content, but lacks tests with non-ASCII characters (e.g., function IDs with accented characters or emoji). Adding such tests would validate the UTF-16 handling and help catch issues like the Point byte offset bug.
💡 Example test with non-ASCII
#[test] fn ts_trigger_function_id_unicode() { // Function ID with non-ASCII: "日本語::関数" let source = r#"iii.trigger({ function_id: '日本語::関数' })"#; // Calculate correct UTF-16 position for inside the string let result = analyze(source, pos(0, 28), Language::TypeScript); assert_eq!(result.context, CompletionContext::FunctionId); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iii-lsp/src/analyzer.rs` around lines 736 - 752, Add tests that include non-ASCII characters to validate UTF-16 handling: extend the tests module (mod tests) by adding a new test function (e.g., ts_trigger_function_id_unicode) similar to ts_trigger_function_id that calls analyze(...) with a source containing non-ASCII function IDs (e.g., "日本語::関数" or emoji) and asserts result.context == CompletionContext::FunctionId and result.current_text matches the non-ASCII string; compute the Position passed to analyze using the correct UTF-16 column index (same mechanism as pos(...) used in ts_trigger_function_id) so the caret falls inside the quoted non-ASCII string to exercise the analyzer's UTF-16 offset logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@iii-lsp/src/analyzer.rs`:
- Around line 222-247: position_to_byte_offset currently assumes '\n' line
endings (it adds line.len() + 1) and under-counts byte offsets for CRLF; fix by
normalizing CRLFs and mapping back to original bytes: inside
position_to_byte_offset, create a normalized string by replacing "\r\n" with
"\n", compute the byte offset against that normalized text (using the existing
logic in position_to_byte_offset), then adjust the computed offset to the
original source by adding back the number of removed '\r' bytes that occur
before that offset in the original source; keep the function signature and use
the same local symbols (source, position, line_num, col_utf16, byte_offset) so
callers of position_to_byte_offset get correct original-source byte offsets.
- Around line 274-280: The code incorrectly constructs tree_sitter::Point using
LSP UTF-16 character units (Point::new(position.line as usize,
position.character as usize)), causing wrong node lookups for non-ASCII text;
replace that with a byte-based column: use the existing position_to_byte_offset
helper (as used by patch_unclosed_string and patch_unclosed_brackets) to get the
absolute byte offset for the LSP Position, compute the line-relative byte column
(byte_offset - line_start_byte_offset), then call Point::new(position.line as
usize, byte_column) and use that Point with root.descendant_for_point_range;
ensure the logic for finding line_start_byte_offset is consistent with how
position_to_byte_offset computes offsets.
In `@iii-lsp/src/diagnostics.rs`:
- Around line 468-481: The to_range function currently converts Tree-sitter byte
column offsets directly to LSP character positions causing misaligned
diagnostics for non-ASCII text; change to_range(node: Node) -> Range to
to_range(node: Node, source: &str) -> Range and compute start/end character as
UTF-16 code units using a helper (e.g., byte_offset_to_utf16 or reuse
position_to_byte_offset logic from analyzer.rs) that iterates the target line's
char_indices and accumulates ch.len_utf16() until the byte column boundary, then
set Position.character to that u32 value; update all call sites (all 13) to pass
the source &str.
---
Nitpick comments:
In `@iii-lsp/src/analyzer.rs`:
- Around line 116-117: The current quote counting in analyzer.rs uses
single_quotes and double_quotes by counting all '\'' and '"' characters and thus
miscounts escaped quotes (e.g., "it\'s"); replace that logic with an
escape-aware routine (e.g., implement and use a function like
count_unescaped_quotes(s: &str, quote: char)) that skips characters immediately
following a backslash so only unescaped quote characters are counted, and call
it instead of the current .filter(...).count() usages for single_quotes and
double_quotes.
- Around line 693-705: The current loop uses the try operator on
key.utf8_text(...) which causes the function to return None early if utf8_text
fails for any child; change the logic in the iteration that checks
child.child_by_field_name("key") so that you handle the utf8_text error locally
(e.g., match or if let Ok(text) = key.utf8_text(...) { let key_text =
strip_quotes(text); ... } else { continue }) rather than using ?, so the loop
continues to the next pair when utf8_text fails while still checking subsequent
pairs for target_key; apply this change around the calls to
child.child_by_field_name("key"), strip_quotes, target_key comparison, and
extract_string_content.
- Around line 92-98: The current decrement of position.character in
analyze_source can land inside a UTF-16 surrogate pair for non-BMP characters;
update the logic to compute the previous valid UTF-16 code unit boundary instead
of blindly doing `position.character - 1`. Use the source line text (the same
string passed into analyze_source) and its UTF-16 encoding (e.g.,
str::encode_utf16) to build the u16 code-unit sequence for the target line, then
find the previous code-unit index that corresponds to a character boundary (if
the current index points to a low surrogate, step back one more u16) and create
the new Position with that adjusted character index before calling
analyze_source recursively; keep references to Position, position.character, and
analyze_source to locate where to change the decrement.
- Around line 736-752: Add tests that include non-ASCII characters to validate
UTF-16 handling: extend the tests module (mod tests) by adding a new test
function (e.g., ts_trigger_function_id_unicode) similar to
ts_trigger_function_id that calls analyze(...) with a source containing
non-ASCII function IDs (e.g., "日本語::関数" or emoji) and asserts result.context ==
CompletionContext::FunctionId and result.current_text matches the non-ASCII
string; compute the Position passed to analyze using the correct UTF-16 column
index (same mechanism as pos(...) used in ts_trigger_function_id) so the caret
falls inside the quoted non-ASCII string to exercise the analyzer's UTF-16
offset logic.
In `@iii-lsp/src/diagnostics.rs`:
- Around line 660-758: Add Rust-language unit tests to mirror the existing
TypeScript and Python tests: implement a parse_rs helper (using
tree_sitter_rust::LANGUAGE) and add tests like rs_finds_trigger_calls that call
find_all_calls on a Rust trigger invocation (e.g., iii.trigger(TriggerArgs {
function_id: "...", payload: ... })) to assert detection of function_id, payload
presence and payload/config keys; place these alongside parse_ts/parse_py and
the other tests in the tests module so parse_rs and the new rs_* tests exercise
the same find_all_calls code paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 599b1d3a-390b-4ff3-bf2a-19d3656f25a5
📒 Files selected for processing (5)
iii-lsp-vscode/package.jsoniii-lsp/src/analyzer.rsiii-lsp/src/diagnostics.rsiii-lsp/src/engine_client.rsiii-lsp/src/main.rs
✅ Files skipped from review due to trivial changes (1)
- iii-lsp-vscode/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- iii-lsp/src/main.rs
- iii-lsp/src/engine_client.rs
sergiofilhowz
approved these changes
Apr 7, 2026
- Add shared helpers utf16_col_to_byte_col and byte_col_to_utf16_col - Fix position_to_byte_offset to handle CRLF line endings - Fix tree-sitter Point construction to use byte column, not UTF-16 - Fix to_range to convert tree-sitter byte columns to LSP UTF-16 units
Merged
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
2 participants
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.
Summary
iii-lsp, a Rust-based Language Server providing completions, hover, and diagnostics for III engine functions and triggersiii-lsp-vscode, a VS Code extension that connects to the LSP serverVS Code Installation
1. Build the LSP binary
cd iii-lsp cargo build --releaseThen add
iii-lsp/target/releaseto yourPATH, or configure the path in VS Code settings.2. Install the extension
Option A — From source:
cd iii-lsp-vscode npm install code --install-extension iii-lsp-vscodeOption B — From VSIX:
3. Configure (optional)
In VS Code
settings.json:{ "iii-lsp.serverPath": "/path/to/iii-lsp", "iii-lsp.engineUrl": "ws://127.0.0.1:49134" }Test plan
cargo testpasses iniii-lsp/andimage-resize/.ts,.py,.rsfiles in VS Codeiii.trigger({ function_id: '...' })Summary by CodeRabbit
New Features
Improvements
Tests
Chores