Release v0.1.0 with MCS syntax diagnostics and autocomplete features - #14
Merged
Merged
Conversation
- Added MCS syntax and import diagnostics via the `minecraft_script lint` command. - Implemented autocomplete for keywords, builtins, TextComponent methods, user-defined functions, and import paths. - Updated README to reflect new features and configuration options. - Enhanced the highlighter extension with new linting capabilities and improved error handling.
|
Warning Review limit reached
More reviews will be available in 23 minutes and 13 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughRelease 0.1.0 adds MCS syntax linting via external Python CLI, context-aware code completions (keywords, methods, imports, user-defined symbols), and supporting configuration. The changes span Python linting implementation and comprehensive test coverage, VS Code bridge integration, and updated documentation. ChangesMCS Linting and Autocomplete v0.1.0
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
highlighter/mcs-language-completion.mjs (1)
102-104: 💤 Low valueConsider refining the import path filter for more predictable completions.
The current filter uses
completionPath.includes(normalizedPartial)as a fallback, which matches any path containing the partial string anywhere. For example, typing"./foo"would match"./bar/foobar/baz.mcs"because it contains "foo". This broad matching might lead to unexpected suggestions.Consider restricting the filter to only prefix matching or path-segment matching for more predictable behavior:
🔍 Alternative filtering approach
- if (normalizedPartial && !completionPath.startsWith(normalizedPartial) && !completionPath.includes(normalizedPartial)) { + if (normalizedPartial && !completionPath.startsWith(normalizedPartial)) { return undefined }This would only show completions that are valid continuations of what the user has typed, which is typically more intuitive.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@highlighter/mcs-language-completion.mjs` around lines 102 - 104, The filter currently allows any completionPath that merely includes normalizedPartial, which yields unexpected matches; update the logic around the normalizedPartial/completionPath check so it only accepts prefix or path-segment matches — e.g., require completionPath.startsWith(normalizedPartial) or a segment boundary match (completionPath === normalizedPartial or completionPath.startsWith(normalizedPartial + "/") or completionPath.includes("/" + normalizedPartial + "/") or completionPath.endsWith("/" + normalizedPartial)); modify the condition that uses normalizedPartial and completionPath (the block with normalizedPartial && !completionPath.startsWith(normalizedPartial) && !completionPath.includes(normalizedPartial)) to implement this stricter matching.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@highlighter/mcs-language-completion.mjs`:
- Around line 71-79: collectUserSymbols() currently returns names from both
FUNCTION_PATTERN and VARIABLE_PATTERN but the completion creation loop always
treats them as functions; update the code so you either (A) make
collectUserSymbols() return symbol objects with a type tag (e.g., {name, kind:
'function'|'variable'}) by checking FUNCTION_PATTERN and VARIABLE_PATTERN, then
in the loop construct vscode.CompletionItem with the appropriate
vscode.CompletionItemKind (Function or Variable) and set
item.detail/item.documentation to "user function" or "user variable"
accordingly, or (B) if you prefer a minimal change, change the static wording to
a generic "user symbol" in the loop and (optionally) set CompletionItemKind to
Variable when the name matches VARIABLE_PATTERN; reference collectUserSymbols,
FUNCTION_PATTERN, VARIABLE_PATTERN, vscode.CompletionItem and
vscode.CompletionItemKind to locate the spots to change.
In `@highlighter/mcs-python-bridge.mjs`:
- Line 120: The Range end calculation uses an unnecessary Math.max that can
conflict with the earlier bounds clamp; replace the expression new
vscode.Range(line, column, line, Math.max(endColumn, column + 1)) with a simple
use of endColumn (i.e., new vscode.Range(line, column, line, endColumn)) so the
computed end comes from the already-clamped endColumn variable; locate the
occurrence in mcs-python-bridge.mjs where Range is constructed using line,
column and endColumn to make this change.
- Around line 114-117: Clamp the computed line index to the document bounds
before calling document.lineAt to avoid RangeError: compute line as
Math.min(Math.max(diagnostic.line - 1, 0), document.lineCount - 1) (using
diagnostic.line and document.lineCount), then continue using
document.lineAt(line) and adjust column/endColumn relative to the retrieved
lineText; update the existing line variable calculation in the code that
currently sets line and calls document.lineAt.
In `@highlighter/README.md`:
- Line 31: The README's sentence "or use the npm CLI after installing
`minecraft-script`" is ambiguous; update the README to either (a) replace that
clause with an explicit npm command and explain how the npm package relates to
the Python package (e.g., whether it wraps/installs the Python tool and what
command to run), or (b) remove the npm reference entirely if there is no
supported npm installation path; locate the sentence in the README that mentions
"python -m minecraft_script lint --json --stdin" and edit the phrase referencing
`minecraft-script`/npm to one clear option with a concrete npm command or a
statement that only the Python pip installation is supported.
In `@minecraft_script/lint.py`:
- Line 48: Replace the overly broad "except BaseException as error" in
minecraft_script/lint.py with "except Exception as error" to avoid catching
system-level exceptions; locate the try/except block that currently uses the
"except BaseException as error" clause and change it to "except Exception as
error". If you need to preserve explicit handling for interrupts or exits, add a
separate clause such as "except (KeyboardInterrupt, SystemExit): raise" before
the Exception handler.
In `@minecraft_script/shell_commands.py`:
- Around line 204-217: When --stdin is used and source_path stays None, replace
direct uses of source_path in result messages with a computed display name (e.g.
display_path = str(source_path) if source_path is not None else "<stdin>") and
use display_path in the print statements that report "no issues found" and the
"file:line:column: message" outputs; update the branch that sets source_path
from filtered_args and the branch that reads from stdin so both paths ensure
display_path is available and used instead of raw source_path (referencing
use_stdin, source_path, filtered_args, and the prints that currently emit
f"{source_path}:..." ).
In `@tests/test_lexer.py`:
- Around line 7-10: The test test_tokenizes_numbers shows "3.14" is split into
"3", ".", "14" (TT_NUMBER, TT_DOT, TT_NUMBER) — fix either the lexer or the
test: if decimals should be numeric literals, update the lexer (function
tokenize in minecraft_script/lexer.py and any helper that emits TT_NUMBER) to
recognize a float pattern (digits optionally followed by '.' and digits) and
emit a single TT_NUMBER token for "3.14"; if splitting is intentional, rename
the test and adjust expected values in test_tokenizes_numbers/token_types to
reflect that decimals are not numbers (or add a new test test_tokenizes_floats
asserting a single TT_NUMBER for floats). Ensure references to TT_DOT are
updated only if you change tokenization behavior.
---
Nitpick comments:
In `@highlighter/mcs-language-completion.mjs`:
- Around line 102-104: The filter currently allows any completionPath that
merely includes normalizedPartial, which yields unexpected matches; update the
logic around the normalizedPartial/completionPath check so it only accepts
prefix or path-segment matches — e.g., require
completionPath.startsWith(normalizedPartial) or a segment boundary match
(completionPath === normalizedPartial or
completionPath.startsWith(normalizedPartial + "/") or
completionPath.includes("/" + normalizedPartial + "/") or
completionPath.endsWith("/" + normalizedPartial)); modify the condition that
uses normalizedPartial and completionPath (the block with normalizedPartial &&
!completionPath.startsWith(normalizedPartial) &&
!completionPath.includes(normalizedPartial)) to implement this stricter
matching.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 180cfa96-f0c2-4efc-95e5-7a9331bb720a
📒 Files selected for processing (14)
highlighter/CHANGELOG.mdhighlighter/README.mdhighlighter/extension.mjshighlighter/hover-docs.mjshighlighter/mcs-language-completion.mjshighlighter/mcs-python-bridge.mjshighlighter/package.jsonminecraft_script/lint.pyminecraft_script/shell_commands.pytests/_parse_helpers.pytests/test_lexer.pytests/test_lint.pytests/test_parser.pytodo.md
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 14 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
- Improved user symbol collection to differentiate between functions and variables in autocomplete suggestions. - Updated completion item details and documentation to reflect the type of user-defined symbols. - Refined diagnostic line calculation in the Python bridge for better accuracy. - Enhanced README to clarify installation instructions for the Python package and npm wrapper. - Improved error handling in linting functions to manage exceptions more effectively. - Updated shell command outputs to display the correct source path for diagnostics. - Added tests for tokenization of integer literals and decimal-like sequences.
Code Review SummaryStatus: 0 New Issues Found | Recommendation: Address existing flagged issues before merge Other Observations (not in diff)Existing review comments have already identified issues in this PR, including a critical line/column swap in Files Reviewed (14 files)
Reviewed by laguna-m.1-20260312:free · 312,085 tokens |
- Updated the import path completion function to include a range for more accurate text replacement. - Improved the regex pattern for position extraction in linting to accommodate various error messages. - Added new tests for diagnostic parsing from exceptions to ensure accurate line and column reporting.
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="highlighter/mcs-language-completion.mjs">
<violation number="1" location="highlighter/mcs-language-completion.mjs:34">
P2: Method completion detection regressed: it checks the character before the cursor instead of the character before the current word, so `obj.met...` falls back to general completions after the first typed letter.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
|
|
||
| const wordRange = document.getWordRangeAtPosition(position, /[A-Za-z_][A-Za-z0-9_-]*/) | ||
| const currentWord = wordRange ? document.getText(wordRange) : '' | ||
| const charBefore = position.character > 0 ? linePrefix[position.character - 1] : '' |
There was a problem hiding this comment.
P2: Method completion detection regressed: it checks the character before the cursor instead of the character before the current word, so obj.met... falls back to general completions after the first typed letter.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At highlighter/mcs-language-completion.mjs, line 34:
<comment>Method completion detection regressed: it checks the character before the cursor instead of the character before the current word, so `obj.met...` falls back to general completions after the first typed letter.</comment>
<file context>
@@ -18,15 +18,20 @@ export function createMcsLanguageCompletionProvider() {
- const charBefore = wordRange && wordRange.start.character > 0
- ? linePrefix[wordRange.start.character - 1]
- : ''
+ const charBefore = position.character > 0 ? linePrefix[position.character - 1] : ''
const items = charBefore === '.'
</file context>
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
1 participant
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.
minecraft_script lintcommand.Summary by CodeRabbit
New Features
Documentation
Chores
pythonPathfor Python executable override andlintSourcePathfor import resolution