chore: willboosterify this repo#133
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates several dependencies, including oxfmt and oxlint-tsgolint, and modifies configurations in .idea/watcherTasks.xml, lefthook.yml, and package.json to exclude package.json from formatting. Review feedback identifies several opportunities to eliminate redundant exclusion patterns where files are already filtered by extension or variable definition. Additionally, suggestions were made to simplify regex patterns for file matching and to improve the robustness of shell commands when handling file lists containing spaces.
| <component name="ProjectTasksOptions"> | ||
| <TaskOptions isEnabled="true"> | ||
| <option name="arguments" value="node_modules/.bin/oxfmt --write --no-error-on-unmatched-pattern $FilePathRelativeToProjectRoot$" /> | ||
| <option name="arguments" value="node_modules/.bin/oxfmt --write --no-error-on-unmatched-pattern !**/package.json $FilePathRelativeToProjectRoot$" /> |
There was a problem hiding this comment.
The exclusion pattern !**/package.json is redundant for this task because it only triggers on .astro files (as specified in line 9). This redundancy is repeated in several other task options for non-JSON extensions throughout this file. According to the repository style guide, code should be simplified as much as possible to eliminate redundancy.
| <option name="arguments" value="node_modules/.bin/oxfmt --write --no-error-on-unmatched-pattern !**/package.json $FilePathRelativeToProjectRoot$" /> | |
| <option name="arguments" value="node_modules/.bin/oxfmt --write --no-error-on-unmatched-pattern $FilePathRelativeToProjectRoot$" /> |
References
- Simplify code as much as possible to eliminate redundancy. (link)
| # Lefthook expands {staged_files} as shell-escaped args, so paths with spaces stay intact. | ||
| oxlint_files="$(printf '%s\n' {staged_files} | grep -E '(\.astro$|\.cjs$|\.cts$|\.js$|\.jsx$|\.mjs$|\.mts$|\.svelte$|\.ts$|\.tsx$|\.vue$)' || true)" | ||
| oxfmt_files="$(printf '%s\n' {staged_files} | grep -E '(\.astro$|\.cjs$|\.css$|\.cts$|\.gql$|\.graphql$|\.hbs$|\.htm$|\.html$|\.js$|\.json$|\.json5$|\.jsonc$|\.jsx$|\.less$|\.md$|\.mdx$|\.mjs$|\.mts$|\.scss$|\.svelte$|\.toml$|\.ts$|\.tsx$|\.vue$|\.yaml$|\.yml$)' || true)" | ||
| oxfmt_files="$(printf '%s\n' {staged_files} | grep -E '(\.astro$|\.cjs$|\.css$|\.cts$|\.gql$|\.graphql$|\.hbs$|\.htm$|\.html$|\.js$|\.json$|\.json5$|\.jsonc$|\.jsx$|\.less$|\.md$|\.mdx$|\.mjs$|\.mts$|\.scss$|\.svelte$|\.toml$|\.ts$|\.tsx$|\.vue$|\.yaml$|\.yml$)' | grep -v -E '(^|/)package\.json$' || true)" |
There was a problem hiding this comment.
The regex for matching file extensions can be simplified for better readability and maintainability. Instead of repeating the anchor $ for each extension, you can group them and use a single anchor at the end.
oxfmt_files="$(printf '%s\n' {staged_files} | grep -E '\.(astro|cjs|css|cts|gql|graphql|hbs|htm|html|js|json|json5|jsonc|jsx|less|md|mdx|mjs|mts|scss|svelte|toml|ts|tsx|vue|yaml|yml)$' | grep -v -E '(^|/)package\.json$' || true)"References
- Simplify code as much as possible to eliminate redundancy. (link)
|
|
||
| if [ -n "$oxfmt_files" ]; then | ||
| node node_modules/.bin/oxfmt --write --no-error-on-unmatched-pattern $oxfmt_files | ||
| node node_modules/.bin/oxfmt --write --no-error-on-unmatched-pattern '!**/package.json' $oxfmt_files |
There was a problem hiding this comment.
The exclusion pattern '!**/package.json' is redundant here because package.json is already filtered out from the $oxfmt_files variable in line 19. Additionally, note that $oxfmt_files is used without quotes, which will cause the command to fail if any staged filenames contain spaces. While this is an existing pattern in the file, it's worth considering a more robust way to handle the file list (e.g., using xargs).
node node_modules/.bin/oxfmt --write --no-error-on-unmatched-pattern $oxfmt_filesReferences
- Simplify code as much as possible to eliminate redundancy. (link)
1 participant
No description provided.