Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a cspell configuration file and registers cspell as a devDependency, extending the lint script. Renames the test service import from MarkoLangaugeService to MarkoLanguageService and updates test callsites (doHover, doValidate, command lookups). Updates fixture HTML classes from "subnote" to "sub-note". Corrects identifier spelling for an attribute constant (ATTR_UNNAMED) across script extractor code. Adds a cspell disable comment in a regex file and fixes typographical errors in NOTICE files. Changes are limited to tests, configs, comments, and identifier/name fixes; no behavioral or public API changes. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 30: The pre-commit wiring is incomplete: the "lint" script in
package.json now runs cspell, but .lintstagedrc.json still only invokes eslint
and prettier on staged files; update .lintstagedrc.json to add rules for the
file patterns you want spell-checked (e.g., "*.md", "*.ts" or the existing
staged globs) that run the cspell command (matching the same cspell invocation
in package.json), ensuring the cspell task is executed for staged files at
pre-commit alongside eslint and prettier.
In
`@packages/language-server/src/__tests__/fixtures/script/class-api-basic/index.marko`:
- Around line 57-60: The emitted HTML for the class-api-basic fixture changed
(the <span class="sub-note"> lines with isSmartPhone: ${isSmartOnly} and
phoneType: ${type}), so the Jest snapshots are out of date; run the test suite
for packages/language-server (or run jest for the class-api-basic test) with
snapshot update (e.g., jest -u or your repo's test:update script), verify the
updated output in __tests__/__snapshots__ for the class-api-basic fixture, and
commit the regenerated __snapshots__ file so CI uses the new snapshot.
In
`@packages/language-server/src/__tests__/fixtures/script/tags-api-basic/index.marko`:
- Around line 26-29: The fixture HTML changed in the tags-api-basic fixture
(index.marko) causing snapshot failures; regenerate the Jest/fixture snapshots
for the tags-api-basic test suite (run the test command with snapshot update,
e.g., jest -u or your project's equivalent) so the updated rendered output (the
modified class/name lines) are captured, then commit the updated snapshot files
alongside the modified fixture; ensure the snapshot files corresponding to the
tags-api-basic fixture are included in the commit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 28f832b9-b593-4bad-b456-3945e093edad
⛔ Files ignored due to path filters (6)
package-lock.jsonis excluded by!**/package-lock.jsonand included by**packages/language-server/src/__tests__/fixtures/script/class-api-basic/__snapshots__/class-api-basic.expected/index.htmlis excluded by!**/__snapshots__/**and included by**packages/language-server/src/__tests__/fixtures/script/class-api-basic/__snapshots__/class-api-basic.expected/index.tsis excluded by!**/__snapshots__/**and included by**packages/language-server/src/__tests__/fixtures/script/tags-api-basic/__snapshots__/tags-api-basic.expected/index.htmlis excluded by!**/__snapshots__/**and included by**packages/language-server/src/__tests__/fixtures/script/tags-api-basic/__snapshots__/tags-api-basic.expected/index.mdis excluded by!**/__snapshots__/**and included by**packages/language-server/src/__tests__/fixtures/script/tags-api-basic/__snapshots__/tags-api-basic.expected/index.tsis excluded by!**/__snapshots__/**and included by**
📒 Files selected for processing (11)
cspell.jsonpackage.jsonpackages/language-server/NOTICEpackages/language-server/src/__tests__/fixtures/script/class-api-basic/index.markopackages/language-server/src/__tests__/fixtures/script/tags-api-basic/index.markopackages/language-server/src/__tests__/index.test.tspackages/language-tools/src/extractors/html/keywords.tspackages/language-tools/src/extractors/script/index.tspackages/language-tools/src/extractors/script/util/attach-scopes.tspackages/language-tools/src/processors/marko.tspackages/vscode/NOTICE
| "change": "npm exec changeset add", | ||
| "format": "npm run build && eslint -f unix --fix . && prettier \"./**/*{.ts,.js,.json,.md,.yml,rc}\" --write", | ||
| "lint": "npm run build && eslint -f unix . && prettier \"./**/*{.ts,.js,.json,.md,.yml,rc}\" --check --log-level=warn", | ||
| "lint": "npm run build && eslint -f unix . && prettier \"./**/*{.ts,.js,.json,.md,.yml,rc}\" --check --log-level=warn && cspell \"**/*.{md,ts,marko}\"", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== package.json hook-related config ==="
jq '{scripts, "lint-staged": .["lint-staged"]}' package.json
echo
echo "=== .husky contents (if present) ==="
if [ -d .husky ]; then
fd . .husky -t f -x sh -c 'echo "--- $1"; sed -n "1,160p" "$1"' sh {}
else
echo "No .husky directory found"
fi
echo
echo "=== references to cspell / lint / pre-commit ==="
rg -n --glob '!**/node_modules/**' --glob '!**/dist/**' 'pre-commit|lint-staged|npm run lint|cspell'Repository: marko-js/language-server
Length of output: 24177
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Looking for lint-staged configuration files ==="
fd -H '\.(lintstagedrc|lintstagedrc\.(js|json|mjs|cjs))$|lint-staged\.config\.(js|json|mjs|cjs)$' . --type f
echo
echo "=== Checking if cspell.json exists ==="
if [ -f cspell.json ]; then
head -20 cspell.json
fi
echo
echo "=== Full .husky/pre-commit content ==="
if [ -f .husky/pre-commit ]; then
cat .husky/pre-commit
fiRepository: marko-js/language-server
Length of output: 662
🏁 Script executed:
cat .lintstagedrc.jsonRepository: marko-js/language-server
Length of output: 170
Pre-commit wiring is incomplete.
Adding cspell to npm run lint doesn't enable pre-commit checks without corresponding changes to the lint-staged configuration. The .lintstagedrc.json file only runs eslint and prettier on staged files—cspell is not included. Update .lintstagedrc.json to trigger spell checking on the appropriate file patterns (e.g., add "*.md" and "*.ts" rules that include the cspell command) so spell checking actually runs on pre-commit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 30, The pre-commit wiring is incomplete: the "lint"
script in package.json now runs cspell, but .lintstagedrc.json still only
invokes eslint and prettier on staged files; update .lintstagedrc.json to add
rules for the file patterns you want spell-checked (e.g., "*.md", "*.ts" or the
existing staged globs) that run the cspell command (matching the same cspell
invocation in package.json), ensuring the cspell task is executed for staged
files at pre-commit alongside eslint and prettier.
| <span class="sub-note">isSmartPhone: ${isSmartOnly}</span> | ||
| </if> | ||
|
|
||
| <span class="subnote">phoneType: ${type}</span> | ||
| <span class="sub-note">phoneType: ${type}</span> |
There was a problem hiding this comment.
Regenerate the matching snapshots.
Line 57 and Line 60 change the emitted HTML for this fixture, and CI is already failing on the class-api-basic snapshot. Please include the updated __snapshots__ output in this PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/language-server/src/__tests__/fixtures/script/class-api-basic/index.marko`
around lines 57 - 60, The emitted HTML for the class-api-basic fixture changed
(the <span class="sub-note"> lines with isSmartPhone: ${isSmartOnly} and
phoneType: ${type}), so the Jest snapshots are out of date; run the test suite
for packages/language-server (or run jest for the class-api-basic test) with
snapshot update (e.g., jest -u or your repo's test:update script), verify the
updated output in __tests__/__snapshots__ for the class-api-basic fixture, and
commit the regenerated __snapshots__ file so CI uses the new snapshot.
| <span class="sub-note">isSmartPhone: ${isSmartOnly}</span> | ||
| </if> | ||
|
|
||
| <span class="subnote">phoneType: ${type}</span> | ||
| <span class="sub-note">phoneType: ${type}</span> |
There was a problem hiding this comment.
Update the fixture snapshots with this HTML change.
Line 26 and Line 29 modify the rendered class name, and CI is already failing on the tags-api-basic snapshot. The regenerated snapshot files should be committed alongside this fixture update.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/language-server/src/__tests__/fixtures/script/tags-api-basic/index.marko`
around lines 26 - 29, The fixture HTML changed in the tags-api-basic fixture
(index.marko) causing snapshot failures; regenerate the Jest/fixture snapshots
for the tags-api-basic test suite (run the test command with snapshot update,
e.g., jest -u or your project's equivalent) so the updated rendered output (the
modified class/name lines) are captured, then commit the updated snapshot files
alongside the modified fixture; ensure the snapshot files corresponding to the
tags-api-basic fixture are included in the commit.
1 participant
Add spell checking as a pre-commit hook, and correct some spelling mistakes