Conversation
Enable commander to accept extra CLI arguments by adding program.allowExcessArguments(true) to the htmlhint CLI. Updated the source (src/cli/htmlhint.ts) and rebuilt distribution files (dist/cli/htmlhint.js, dist/cli/parse-glob.js). Also includes package.json/package-lock.json updates from the rebuild.
Deploying htmlhint with
|
| Latest commit: |
4281b64
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://405a2756.htmlhint.pages.dev |
| Branch Preview URL: | https://dev-coliff-update-dev-deps-m.htmlhint.pages.dev |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR updates the htmlhint CLI to tolerate additional positional arguments by configuring Commander to allow excess arguments, and refreshes the built dist/ artifacts and lockfiles accordingly.
Changes:
- Enable Commander’s “allow excess arguments” behavior in the
htmlhintCLI. - Update
commander(and rebuild outputs), along with incidental toolchain/lockfile updates. - Rebuild
dist/artifacts to reflect the source/dependency changes.
Reviewed changes
Copilot reviewed 2 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/cli/htmlhint.ts | Enables Commander excess-argument acceptance for the CLI entrypoint. |
| dist/cli/htmlhint.js | Rebuilt CLI bundle reflecting the new Commander setting. |
| dist/cli/parse-glob.js | Rebuilt helper output as part of the updated build/toolchain. |
| package.json | Updates Commander (runtime) and bumps dev tooling versions used in the rebuild. |
| package-lock.json | Lockfile updates to match the dependency/toolchain changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "prettier": "3.8.1", | ||
| "rimraf": "^5.0.0", | ||
| "rimraf": "^6.1.3", | ||
| "rollup": "4.59.1", | ||
| "typescript": "5.4.5" | ||
| "typescript": "5.9.3" |
There was a problem hiding this comment.
rimraf and typescript are also bumped as part of the rebuild. These toolchain upgrades can change emitted dist/ output and are not directly required by the CLI behavior change; consider moving these devDependency upgrades into a separate PR to keep the risk/review surface focused.
| } | ||
|
|
||
| const program = new Command() | ||
| program.allowExcessArguments(true) |
There was a problem hiding this comment.
This change alters how the CLI tolerates additional positional args, but there’s no regression test covering the new behavior. Please add a CLI-level test (e.g., invoking bin/htmlhint with multiple targets or with additional args after --) that would have previously failed and now succeeds, so the intent is locked in.
| "dependencies": { | ||
| "async": "3.2.6", | ||
| "commander": "11.1.0", | ||
| "commander": "14.0.3", |
There was a problem hiding this comment.
commander is bumped from 11.x to 14.x as part of this change. Since this is a runtime dependency for the published CLI, a major-version upgrade can introduce unrelated breaking behavior; consider either keeping the existing major version (if allowExcessArguments is available there) or splitting the commander upgrade into a dedicated PR with explicit compatibility/testing notes.
| "commander": "14.0.3", | |
| "commander": "11.1.0", |
There was a problem hiding this comment.
Code Review
This pull request enables htmlhint to accept excess arguments by calling program.allowExcessArguments(true), which is necessary due to a breaking change in the updated commander library. The change correctly restores the previous behavior. However, I've suggested a more idiomatic approach using .argument('[targets...]') to explicitly declare variadic arguments, which improves code clarity and adheres to modern commander best practices. The PR also includes updates to other dependencies like rimraf and typescript as part of the rebuild process, which look fine.
| } | ||
|
|
||
| const program = new Command() | ||
| program.allowExcessArguments(true) |
There was a problem hiding this comment.
While allowExcessArguments(true) works, a more idiomatic approach with commander since v12 is to explicitly declare that your program accepts variadic arguments. This makes the CLI definition clearer.
I suggest removing this line and instead adding .argument('[targets...]', 'file, folder, pattern, stdin, or url to lint') to the command definition chain (e.g., after .usage()).
// src/cli/htmlhint.ts:56
program
.version(pkg.version)
.usage('<file|folder|pattern|stdin|url ...> [options]')
.argument('[targets...]', 'file, folder, pattern, stdin, or url to lint') // Add this line
.option('-l, --list', 'show all of the rules available')
// ...This will have the same effect of allowing multiple file/path arguments, but it is more explicit about the command's interface.
Enable commander to accept extra CLI arguments by adding program.allowExcessArguments(true) to the htmlhint CLI. Updated the source (src/cli/htmlhint.ts) and rebuilt distribution files (dist/cli/htmlhint.js, dist/cli/parse-glob.js).
Also includes package.json/package-lock.json updates from the rebuild.