feat: support detecting origin indent and newline to generate pages.json #170
Conversation
WalkthroughThe updates enhance dependency versions, integrate new features for file formatting, and extend testing capabilities. Major improvements include updating package versions in Changes
Sequence Diagram(s)sequenceDiagram
participant Editor as Editor
participant PageContext as PageContext
participant detectIndent as detectIndent
participant detectNewline as detectNewline
participant JSON as JSON
Editor->>PageContext: Update pages.json
PageContext->>detectIndent: Detect indent in pages.json
detectIndent-->>PageContext: Return indent
PageContext->>detectNewline: Detect newline in pages.json
detectNewline-->>PageContext: Return newline style
PageContext->>JSON: Stringify with detected indent & newline
JSON-->>PageContext: Formatted JSON string
PageContext->>Editor: Save formatted pages.json
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (3)
packages/core/src/context.ts (3)
Line range hint
91-91: Optimize array operations by using.flatMap().- this.pagesPath = pageDirFiles.map(page => page.files).flat() + this.pagesPath = pageDirFiles.flatMap(page => page.files)This change simplifies the code and potentially improves performance by reducing the complexity of array operations.
Line range hint
302-302: Usefor...ofinstead offorEachfor better performance and readability.- pages.forEach(v => pagesMap.set(v.path, v)) + for (const v of pages) { + pagesMap.set(v.path, v) + }This change enhances the readability and performance of the loop, especially if asynchronous operations are involved.
Line range hint
119-119: Avoid reassigning function parameters.- watcher.on('add', async (path) => { + watcher.on('add', async (originalPath) => { + let path = slash(originalPath);This change avoids confusion and potential bugs related to the reassignment of function parameters by using a new variable.
Also applies to: 133-133, 155-155
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!pnpm-lock.yaml
Files selected for processing (3)
- packages/core/package.json (1 hunks)
- packages/core/src/context.ts (5 hunks)
- packages/core/src/files.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/core/src/files.ts
Additional context used
Biome
packages/core/src/context.ts
[error] 91-91: The call chain .map().flat() can be replaced with a single .flatMap() call.
[error] 302-302: Prefer for...of instead of forEach.
[error] 119-119: Reassigning a function parameter is confusing.
[error] 133-133: Reassigning a function parameter is confusing.
[error] 155-155: Reassigning a function parameter is confusing.
Additional comments not posted (5)
packages/core/package.json (1)
47-48: Addition ofdetect-indentanddetect-newlinedependencies.These dependencies are crucial for implementing the feature to maintain the original indentation and newline settings in
pages.json. This aligns with the PR's objectives to reduce unnecessary git diffs.packages/core/src/context.ts (4)
10-11: Import statements fordetect-indentanddetect-newline.These imports are necessary for the new functionality to detect and apply the original formatting of
pages.json.
43-45: Initialization of properties for JSON formatting.Proper initialization ensures that the default formatting is set, which is crucial for maintaining consistency if the detection fails.
67-70: Reading and setting the formatting based on the content ofpages.json.This change is essential for dynamically determining the indentation and newline characters based on the existing content of
pages.json, which addresses the issue described in #133.
313-319: Use detected formatting settings when generatingpages.json.This implementation ensures that the generated
pages.jsonrespects the original formatting, thus minimizing unnecessary version control diffs.
|
LGTM |
ModyQyW
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/core/src/files.ts (1 hunks)
- test/files.spec.ts (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/core/src/files.ts
Additional comments not posted (2)
test/files.spec.ts (2)
2-2: The addition ofitfrom 'vitest' is appropriate for the expanded testing capabilities.
16-16: The snapshot test is well-implemented and ensures consistency in the output ofgetPageFiles.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
packages/core/src/context.ts (2)
Line range hint
91-91: Optimize array operations by replacing.map().flat()with.flatMap().- this.pagesPath = pageDirFiles.map(page => page.files).flat() + this.pagesPath = pageDirFiles.flatMap(page => page.files)
Line range hint
302-302: ReplaceforEachwithfor...offor better performance and readability.- pages.forEach(v => pagesMap.set(v.path, v)) + for (const v of pages) { + pagesMap.set(v.path, v) + }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!pnpm-lock.yaml
Files selected for processing (2)
- packages/core/package.json (1 hunks)
- packages/core/src/context.ts (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/core/package.json
Additional context used
Biome
packages/core/src/context.ts
[error] 91-91: The call chain .map().flat() can be replaced with a single .flatMap() call. (lint/complexity/useFlatMap)
Safe fix: Replace the chain with .flatMap().
[error] 302-302: Prefer for...of instead of forEach. (lint/complexity/noForEach)
forEach may lead to performance issues when working with large arrays. When combined with functions like filter or map, this causes multiple iterations over the same type.
Additional comments not posted (5)
packages/core/src/context.ts (5)
10-11: Added imports fordetect-indentanddetect-newlineare appropriate for the new functionality.
24-24: Import ofreadFileSyncaligns with the new functionality to read file content for detecting format.
43-45: Initialization of properties for JSON format detection is clear and concise.
67-70: Proper use ofreadFileSyncand format detection functions within the constructor to set up the initial state based on the existingpages.jsoncontent.
313-319: The logic to conditionally append the newline character based onresolvedPagesJSONEofNewlineis well-implemented.
|
我打算在两到三天内合并发布这个 |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (6)
- package.json (2 hunks)
- packages/core/package.json (1 hunks)
- packages/core/src/context.ts (5 hunks)
- packages/playground/package.json (1 hunks)
- packages/schema/package.json (1 hunks)
- packages/volar/package.json (1 hunks)
Files skipped from review due to trivial changes (4)
- package.json
- packages/playground/package.json
- packages/schema/package.json
- packages/volar/package.json
Additional comments not posted (13)
packages/core/package.json (7)
43-43: Update to dependency@uni-helper/uni-envapproved.The version bump from
^0.1.1to^0.1.3should include improvements or bug fixes that benefit the project.
44-44: Update to dependency@vue/compiler-sfcapproved.The minor version update from
^3.4.27to^3.4.30is likely to include important fixes and optimizations.
46-46: Update to dependencydebugapproved.The patch version update from
^4.3.4to^4.3.5likely contains minor fixes and should not introduce breaking changes.
47-47: Addition ofdetect-indentdependency approved.This new dependency supports the feature of detecting original indentation in
pages.json, aligning with the PR objectives.
48-48: Addition ofdetect-newlinedependency approved.This new dependency supports the feature of detecting original newline format in
pages.json, which is crucial for maintaining consistency and preventing frequent Git changes.
54-54: Update to dependencyyamlapproved.The minor version update from
^2.4.2to^2.4.5should include optimizations or bug fixes enhancing stability or performance.
59-59: Update to development dependency@types/nodeapproved.The version update from
^20.12.11to^20.14.8ensures better type definitions aligning with the latest Node.js API changes, which can improve type safety and developer experience.packages/core/src/context.ts (6)
10-10: Addition ofdetect-indentimport approved.This import is necessary for the implementation of detecting original indentation settings in
pages.json, supporting the PR's functionality.
11-11: Addition ofdetect-newlineimport approved.This import is essential for the new functionality of detecting newline characters, which helps maintain file consistency and minimize Git diffs.
24-24: Addition ofreadFileSyncimport approved.Importing
readFileSyncfrom './files' supports reading thepages.jsoncontent to detect formatting settings, which is crucial for the feature being implemented.
43-45: Initialization of newline and indentation properties approved.These lines correctly initialize the properties that will store the detected indentation and newline characters, which are central to the new feature.
67-70: Logic for setting formatting based on file content approved.The logic here effectively reads the file content and sets the indentation and newline properties based on the actual content. This is a robust approach to ensure that the output matches the original formatting.
313-319: Enhanced JSON stringification logic approved.This modification to the
JSON.stringifyusage, where indentation and newline settings are dynamically applied based on the detected values, is a crucial enhancement that supports the main objective of this PR.
Description 描述
Close #133.
Linked Issues 关联的 Issues
#133
Additional context 额外上下文
As a feature, we don't have to rush for it.
I also want to find a way to handle #133 (comment). I think we can try to detect if
pages.jsonormanifest.jsonexists and generate one if not in the constructor in the next PR. What do you think?Summary by CodeRabbit
Chores
packageManager, dependencies, and devDependencies versions inpackage.json.detect-indentanddetect-newlinedependencies inpackages/core/package.json.New Features
PageContextclass, including indent and newline detection.Tests
itfunction import intest/files.spec.ts.