Skip to content
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

fix: 生成的 manifest.json 文件最后增加空白换行符 #24

Closed
wants to merge 4 commits into from
Closed

Conversation

waset
Copy link

@waset waset commented Apr 12, 2024

Description 描述

生成的 manifest.json 文件最后增加空白换行符

** 问题 **
在使用 eslint 时会报错:
image

Summary by CodeRabbit

  • Refactor
    • Enhanced the file writing process for manifest JSON to improve formatting and content structure.
    • Updated the WriteManifestJSON method in the ManifestContext class for better configuration content generation.
    • Improved writing logic by appending a newline character at the end of the manifest JSON file.
    • Added an optional newline property to the Options interface in src/types.ts.
  • Documentation
    • Updated README with new configuration options for UniManifest regarding minification and newline characters in manifest.json.

Copy link

coderabbitai bot commented Apr 12, 2024

Walkthrough

The update improves the handling of manifest JSON file generation by adding a newline character at the end of the file in the ManifestContext class and introduces a new optional newline property in the Options interface in src/types.ts, enhancing configuration flexibility.

Changes

File Path Change Summary
src/context.ts Updated WriteManifestJSON to include a newline at the end of the file and improve configuration content string generation.
src/types.ts Added a new optional property newline of type boolean to the Options interface.

Possibly related issues

🐇✨
A little tweak here, a small fix there,
New lines at the end, now files are fair.
Hop, hop, hop, on code we improve,
In the digital burrow, we make our move.
🌟📝


Recent Review Details

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 4abb6e9 and 3163952.
Files selected for processing (2)
  • README.md (1 hunks)
  • src/types.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • src/types.ts

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@waset
Copy link
Author

waset commented Apr 12, 2024

@KeJunMao 麻烦处理一下

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

src/context.ts Outdated
Comment on lines 31 to 36
// 生成配置内容字符串
let body = JSON.stringify(config, null, minify ? 0 : 2)
// 文件最后增加空白换行符
const r = body + "\n"
// 写入文件
writeFileSync(manifestJsonPath, r)
Copy link

@coderabbitai coderabbitai bot Apr 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic to append a newline character to the manifest JSON content is correctly implemented. However, consider renaming the variable r to something more descriptive, like finalContent, to enhance code readability.

- const r = body + "\n"
+ const finalContent = body + "\n"

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
// 生成配置内容字符串
let body = JSON.stringify(config, null, minify ? 0 : 2)
// 文件最后增加空白换行符
const r = body + "\n"
// 写入文件
writeFileSync(manifestJsonPath, r)
// 生成配置内容字符串
let body = JSON.stringify(config, null, minify ? 0 : 2)
// 文件最后增加空白换行符
const finalContent = body + "\n"
// 写入文件
writeFileSync(manifestJsonPath, finalContent)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@waset, thank you for your feedback! If there's anything else you need help with, feel free to ask. Happy coding! 🐇


If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!

…,增加对 options 的支持,并优化写入 manifest.json 文件的功能。
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

src/types.ts Outdated
Comment on lines 8 to 12
/**
* `manifest.json` 最后增加换行符
* @default false
*/
newline?: boolean
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The addition of the newline property to the Options interface is well-implemented and aligns with the PR's objectives. However, consider enhancing the documentation comment to explicitly state that this setting controls the addition of a newline at the end of the manifest.json file, which might help others understand its purpose more quickly.

8,12c8,12
   /**
-   * `manifest.json` 最后增加换行符
+   * Controls whether a newline character is added at the end of the `manifest.json` file.
    * @default false
    */
   newline?: boolean

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
/**
* `manifest.json` 最后增加换行符
* @default false
*/
newline?: boolean
/**
* Controls whether a newline character is added at the end of the `manifest.json` file.
* @default false
*/
newline?: boolean

@waset
Copy link
Author

waset commented Apr 12, 2024

@KeJunMao
查看了#133 后修改代码,增加用户配置以启用文件换行,望审批~

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Comment on lines +20 to +34
plugins: [
UniManifest({
/**
* minify the `manifest.json`
* @default false
*/
minify: true,
/**
* Controls whether a newline character is added at the end of the `manifest.json` file.
* @default false
*/
newline: true
}),
Uni()
],
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

Codebase Verification

The verification process has confirmed that the default value for the minify option in the UniManifest plugin is indeed set to false, as indicated by the entries in src/context.ts and src/options.ts. However, there was no output confirming the default value for the newline option, suggesting that it might not be set to false by default or it is not explicitly defined in the TypeScript files we searched.

Given this information, the review comment about ensuring that the default values mentioned in the README (@default false) are consistent with the actual default behavior in the codebase is partially correct. The minify option's default is verified as false, but the newline option's default remains unverified.

Analysis chain

The updated plugin configuration in defineConfig correctly introduces the UniManifest plugin with new options for minify and newline. This aligns with the PR's objective to manage the format of the manifest.json file more effectively. However, ensure that the default values mentioned in the comments (@default false) are consistent with the actual default behavior of these options in the codebase.

Consider adding a brief explanation in the README about why these options might be useful for users, especially the newline option which addresses specific linting issues.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify that the default values for minify and newline in UniManifest are indeed set to false.
rg --type ts "minify: false" ./src/types.ts
rg --type ts "newline: false" ./src/types.ts

Length of output: 92



Script:

#!/bin/bash
# Broaden the search for default values of minify and newline across all TypeScript files
rg --type ts "minify: false" 
rg --type ts "newline: false" 

Length of output: 204

@KeJunMao KeJunMao requested a review from ModyQyW April 13, 2024 04:38
@waset
Copy link
Author

waset commented Apr 15, 2024

可以使用 @waset/uni-reset-manifest

@waset waset closed this Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant