-
Notifications
You must be signed in to change notification settings - Fork 2
feat: better integration with editor #96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 00eaf73 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis update introduces modularized configuration for ESLint and CSpell by adding new source modules and dictionary files, refactors the lint command to delegate ESLint setup, and updates configuration loading to support URL roots. It also revises package exports, dependencies, and documentation for IPv6 VM network configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant ConfigLoader as loadConfig
participant ESLintConfig as doom (src/eslint.ts)
participant ESLint
CLI->>ConfigLoader: loadConfig(root)
ConfigLoader-->>CLI: { config, filepath }
CLI->>ESLintConfig: doom(config.cspellOptions)
ESLintConfig-->>CLI: ESLint config array
CLI->>ESLint: new ESLint({ overrideConfig: ... })
CLI->>ESLint: lintFiles()
ESLint-->>CLI: results
Possibly related PRs
Suggested labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
eslint.config.jsOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/eslint.js' imported from /eslint.config.js src/cspell.tsOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/eslint.js' imported from /eslint.config.js src/eslint.tsOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/eslint.js' imported from /eslint.config.js 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🧠 Learnings (1)docs/en/start.mdx (1)🪛 LanguageTooldocs/en/start.mdx[uncategorized] ~261-~261: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION) [uncategorized] ~269-~269: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION) ⏰ Context from checks skipped due to timeout of 90000ms (8)
🔇 Additional comments (9)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances integration with the editor by unifying and streamlining configurations for ESLint and CSpell as well as improving the CLI configuration logic.
- Introduced a unified doom function for ESLint and CSpell configuration.
- Refactored load-config to better handle URL roots and configuration resolution.
- Updated package exports and documentation to support the new integration.
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/eslint.ts | Added a unified configuration function ("doom") leveraging multiple ESLint plugins. |
| src/cspell.ts | Implements cspell settings generation with integrated dictionary resolution. |
| src/cli/load-config.ts | Refactored config loading with support for URL-based roots and improved path handling. |
| src/cli/lint.ts | Simplifies lint command by delegating configuration to the unified doom function. |
| package.json | Updated exports to expose new ESLint and CSpell integration modules. |
| fixture-docs/en/test/managedtap.mdx | Added documentation for network binding mode with IPv6 support. |
| eslint.config.js | Merged doom configuration into the ESLint config with additional MDX rules. |
| doom.config.yml | Removed obsolete lint configuration, streamlining the doom config. |
| cspell.config.js | Exports the new cspell configuration from the integrated doom package. |
| .cspell/k8s.txt & .cspell/devops.txt | Added dictionary files for cspell. |
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/eslint.ts (1)
44-47: Potential type safety issue with merge and satisfies.The combination of
mergewith asatisfiesclause might cause TypeScript compilation issues depending on the inferred types. Consider extracting the default options to improve type safety.- '@cspell/spellchecker': [ - 'error', - merge({ autoFix: true } satisfies Partial<Options>, cspellOptions), - ], + '@cspell/spellchecker': [ + 'error', + merge({ autoFix: true }, cspellOptions ?? {}), + ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
.cspell/devops.txt(1 hunks).cspell/k8s.txt(1 hunks).github/workflows/ci.yml(1 hunks)cspell.config.js(1 hunks)doom.config.yml(0 hunks)eslint.config.js(2 hunks)fixture-docs/en/test/managedtap.mdx(1 hunks)package.json(2 hunks)src/cli/lint.ts(2 hunks)src/cli/load-config.ts(6 hunks)src/cspell.ts(1 hunks)src/eslint.ts(1 hunks)
💤 Files with no reviewable changes (1)
- doom.config.yml
✅ Files skipped from review due to trivial changes (1)
- fixture-docs/en/test/managedtap.mdx
🚧 Files skipped from review as they are similar to previous changes (6)
- cspell.config.js
- .cspell/k8s.txt
- eslint.config.js
- src/cli/lint.ts
- package.json
- src/cli/load-config.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/cspell.ts (2)
src/cli/helpers.ts (1)
parseTerms(70-70)src/terms.ts (1)
NormalizedTermItem(40-40)
🪛 LanguageTool
.cspell/devops.txt
[uncategorized] ~1-~1: Das sieht falsch platziert oder unnötig aus. Überlegen Sie, es zu löschen.
Context: katanomi tekton tektonchains tektonconfigs tekton...
(AI_DE_GGEC_UNNECESSARY_SPACE)
[uncategorized] ~2-~2: Das sieht falsch platziert oder unnötig aus. Überlegen Sie, es zu löschen.
Context: katanomi tekton tektonchains tektonconfigs tektonhubs te...
(AI_DE_GGEC_UNNECESSARY_SPACE)
[uncategorized] ~3-~3: Das sieht falsch platziert oder unnötig aus. Überlegen Sie, es zu löschen.
Context: katanomi tekton tektonchains tektonconfigs tektonhubs tektoninstaller...
(AI_DE_GGEC_UNNECESSARY_SPACE)
[uncategorized] ~4-~4: Das sieht falsch platziert oder unnötig aus. Überlegen Sie, es zu löschen.
Context: ...tanomi tekton tektonchains tektonconfigs tektonhubs tektoninstallersets tektonpip...
(AI_DE_GGEC_UNNECESSARY_SPACE)
[uncategorized] ~5-~5: Das sieht falsch platziert oder unnötig aus. Überlegen Sie, es zu löschen.
Context: ...on tektonchains tektonconfigs tektonhubs tektoninstallersets tektonpipelines tekt...
(AI_DE_GGEC_UNNECESSARY_SPACE)
[uncategorized] ~6-~6: Das sieht falsch platziert oder unnötig aus. Überlegen Sie, es zu löschen.
Context: ...onconfigs tektonhubs tektoninstallersets tektonpipelines tektonresults tektontrig...
(AI_DE_GGEC_UNNECESSARY_SPACE)
[uncategorized] ~7-~7: Das sieht falsch platziert oder unnötig aus. Überlegen Sie, es zu löschen.
Context: ...hubs tektoninstallersets tektonpipelines tektonresults tektontriggers
(AI_DE_GGEC_UNNECESSARY_SPACE)
[uncategorized] ~8-~8: Das sieht falsch platziert oder unnötig aus. Überlegen Sie, es zu löschen.
Context: ...tallersets tektonpipelines tektonresults tektontriggers
(AI_DE_GGEC_UNNECESSARY_SPACE)
🔇 Additional comments (10)
src/eslint.ts (3)
12-24: Well-designed function overloads with robust parameter handling.The function overloads and parameter resolution logic correctly handle both direct cspell options and root path/URL scenarios. The type guards and async config loading integration look solid.
56-64: Good TypeScript rule customizations.The TypeScript-specific rule configurations are well-chosen - disabling overly strict rules while maintaining type safety and enforcing consistent coding patterns.
38-38: Verify the specificity of the file pattern.The pattern
**/en/**/*.{js,jsx,md,mdx,ts,tsx}is quite specific, targeting only files underendirectories. Ensure this aligns with your project structure and internationalization strategy.#!/bin/bash # Description: Check if there are files matching this pattern and if there are other language directories echo "Files matching the pattern:" fd -e js -e jsx -e md -e mdx -e ts -e tsx . | grep '/en/' | head -10 echo -e "\nOther potential language directories:" fd -t d -d 2 . | grep -E '/[a-z]{2}(-[A-Z]{2})?$' | head -10.github/workflows/ci.yml (1)
39-41: Improved CI step sequencing.Separating the build step from the parallel execution ensures that build artifacts are available before running dependent tasks like linting and documentation generation. This change improves CI reliability.
.cspell/devops.txt (1)
1-9: Comprehensive DevOps dictionary addition.The dictionary contains relevant DevOps and Tekton-related terms that will improve spell-checking accuracy. The static analysis hints about "unnecessary spaces" are false positives since this is a word list format, not prose text.
src/cspell.ts (5)
15-22: Robust error handling for dictionary directory resolution.The error handling properly catches both non-existent directories and non-directory files at the expected path. The early returns prevent unnecessary processing when the
.cspelldirectory is unavailable.
27-42: Efficient dictionary file processing.The dictionary resolution logic correctly processes all files in the
.cspelldirectory, creating appropriate dictionary definitions withaddWords: truefor dynamic word additions.
52-64: Clever case-sensitive word detection logic.The logic to separate words based on case sensitivity is well-designed. Words with bad cases that differ only by case are handled specially with suggestions, while others are added to the regular word list.
69-79: Well-structured case-sensitive dictionary configuration.The special case-sensitive dictionary with
supportNonStrictSearches: falseand suggestion mappings provides excellent spell-checking experience for terms with specific casing requirements.
81-90: Comprehensive CSpell configuration.The returned configuration includes all necessary settings for effective spell-checking: compound words, multiple English variants, git integration, and both regular and flagged words with custom dictionaries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (9)
docs/en/start.mdx (9)
15-17: Use consistent package manager initialization commands
Based on the project's convention of using Yarn v4 (learned from JounQin#75), consider replacing thenpm init -yexample withyarn init -yor including it alongside npm.
27-40: Include a lint script in package.json
The new documentation introducesdoom lint, but the examplescriptsblock omits alintentry. Consider adding:"lint": "doom lint"
151-152: Use consistent command tabs for Yarn
TheRun \yarn serve`instruction should use the` component for consistency with other commands.
155-155: Use consistent command tabs for scaffolding
Replace the inlineRun \yarn new`with` to maintain consistency across the guide.
178-186: Normalize list formatting in translation examples
Ensure nested list items are consistently indented and each item ends with a period. For example:- 1. `yarn translate -g abc xyz` will translate all documents under `<root>/<source>/abc` and `<root>/<source>/xyz` ... + 1. `yarn translate -g abc xyz` will translate all documents under `<root>/<source>/abc` and `<root>/<source>/xyz` respectively.
191-191: Add period to warning section
The sentence should end with a period.-Specifically, if you use `-g '*'` for full translation, ... +Specifically, if you use `-g '*'` for full translation, ...
195-195: Correct capitalization of "team Leader"
Lowercase "leader" for consistency with common usage.-Please contact your team Leader to obtain it. +Please contact your team leader to obtain it.
235-235: Use consistent command tabs for export
TheRun \yarn export`example should leverage` for uniformity with other package manager commands.
261-273: Add missing colon aftercspell.config.mjs
The bullet item forcspell.config.mjslacks a trailing colon. Apply:- - `cspell.config.mjs` + - `cspell.config.mjs`:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.changeset/wicked-beds-teach.md(1 hunks)docs/en/start.mdx(9 hunks)docs/zh/start.mdx(1 hunks)fixture-docs/en/test/managedtap.mdx(1 hunks)src/cspell.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .changeset/wicked-beds-teach.md
- docs/zh/start.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- fixture-docs/en/test/managedtap.mdx
- src/cspell.ts
🧰 Additional context used
🧠 Learnings (1)
docs/en/start.mdx (1)
Learnt from: JounQin
PR: alauda/doom#75
File: src/cli/load-config.ts:4-7
Timestamp: 2025-06-09T03:10:41.010Z
Learning: The alauda/doom project uses yarn v4 as the package manager, not npm. Always reference yarn commands when suggesting package management operations.
🪛 LanguageTool
docs/en/start.mdx
[uncategorized] ~153-~153: A punctuation mark might be missing here.
Context: ...eview. ### Using Scaffolding Templates {#new} Run yarn new to generate projects, ...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
[uncategorized] ~261-~261: Loose punctuation mark.
Context: ...figuration files: - eslint.config.mjs: ```js import doom from '@alauda/do...
(UNLIKELY_OPENING_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Build and Lint with Node.js 22 on ubuntu-latest
- GitHub Check: Build and Lint with Node.js 24 on ubuntu-latest
- GitHub Check: Build and Lint with Node.js 18 on ubuntu-latest
- GitHub Check: Build and Lint with Node.js 20 on ubuntu-latest
- GitHub Check: Build and Lint with Node.js 24 on ubuntu-latest
- GitHub Check: Build and Lint with Node.js 18 on ubuntu-latest
- GitHub Check: Build and Lint with Node.js 22 on ubuntu-latest
- GitHub Check: Build and Lint with Node.js 20 on ubuntu-latest
🔇 Additional comments (2)
docs/en/start.mdx (2)
2-2: Skip sourceSHA metadata
This metadata line is auto-generated and does not require manual review.
48-48: Skip tsconfig.json creation section
The tsconfig snippet is correctly formatted and needs no review.
13989ec to
00eaf73
Compare
Summary by CodeRabbit