refactor: migrate from citty to node's built-in parseArgs#1971
refactor: migrate from citty to node's built-in parseArgs#1971IWANABETHATGUY merged 1 commit intorolldown:mainfrom
parseArgs#1971Conversation
✅ Deploy Preview for rolldown-rs canceled.
|
parseArgsparseArgs
|
Usage output and the current error output of #1852 
|
There was a problem hiding this comment.
Great, and I will take care of matters pertaining to arguments in #1963. cc @IWANABETHATGUY.
| // We need to support both `rolldown -c` and `rolldown -c rolldown.config.js`, | ||
| // the value of the option could be either a boolean or a string in this case, | ||
| // so `strict` needs to be set to `false` | ||
| strict: false, |
There was a problem hiding this comment.
Indeed, it should be false. By the way, I wonder if we should specify the rolldown.config.js file or move it to the constants file (as it may be considered hardcoding if I remove the “constants” file.).
Additionally, I do not believe the automatically ignored parameter is elegant. Once the API becomes stable and independent, I think we should look into a better way to separate the default configuration or the configuration file that’s been specified 🤔.
There was a problem hiding this comment.
Indeed, it should be
false. By the way, I wonder if we should specify therolldown.config.jsfile or move it to the constants file (as it may be considered hardcoding if I remove the “constants” file.).
It should be fine to keep it in constants.ts, as there might be some default values and so on for the CLI in the future.
Additionally, I do not believe the automatically ignored parameter is elegant. Once the API becomes stable and independent, I think we should look into a better way to separate the default configuration or the configuration file that’s been specified 🤔.
Perhaps when the user specifies both the config option and other options, we could log a message to inform them that we will ignore the other options?
There was a problem hiding this comment.
It should be fine to keep it in constants.ts, as there might be some default values and so on for the CLI in the future.
Ok.
Perhaps when the user specifies both the config option and other options, we could log a message to inform them that we will ignore the other options?
I'll do it in #1963.
| } from '../../package.json' assert { type: 'json' } | ||
| import { showHelp } from './commands/help' | ||
| import { DEFAULT_CONFIG_FILENAME } from './constants' | ||
| import { CLI_OPTIONS } from './options' |
There was a problem hiding this comment.
The value should not be static when the automatically generated arguments are implemented, so I suggest using the camelCase method instead of the UPPER SNAKE_CASE method.
There was a problem hiding this comment.
In this PR, the value is still static; you can change it to camelCase in your subsequent PR.
|
|
||
| const HELP_TEMPLATE = `rolldown version ${version} | ||
|
|
||
| ${description} |
There was a problem hiding this comment.
Fast JavaScript/TypeScript bundler in Rust with Rollup-compatible API. (rolldown v0.12.2)
Can you keep the layout as before?
There was a problem hiding this comment.
current output and before:
| } | ||
| return cyan(optionStr.padEnd(30)) + description | ||
| }) | ||
| .join('\n'), |
There was a problem hiding this comment.
Can you add an indentation for each option ?
There was a problem hiding this comment.
Also we should fix the wrong indentation before
There was a problem hiding this comment.
Done, two spaces are added bofore each option
resolves #1954
resolves #1852