Skip to content

Conversation

@shadowspawn
Copy link
Collaborator

@shadowspawn shadowspawn commented Jul 7, 2024

Problem

By default, if is not an error to pass excess command-arguments. This may sometimes be as intended but
will often mean a command-line error by the user will be silently ignored. A nice example from #2149

One particular case is when an argument is a name/title/label, in which the user may include spaces while forgetting quotes.

I originally intended excess arguments to be an error by default, but made it opt-in for the first release to reduce breakage at the time:

See: #2149

Solution

  • Make allowExcessArguments false by default.
  • Fix lots of broken tests where passing undeclared arguments!
  • Add migration tips to CHANGELOG for the breaking change.

@shadowspawn shadowspawn changed the base branch from develop to release/13.x July 7, 2024 04:35
@shadowspawn shadowspawn changed the title Feature/disallow excess arguments Disallow excess arguments Jul 7, 2024
@shadowspawn shadowspawn added the semver: major Releasing requires a major version bump, not backwards compatible label Jul 7, 2024
@shadowspawn shadowspawn marked this pull request as ready for review July 7, 2024 04:35
@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Jul 8, 2024
@shadowspawn shadowspawn merged commit 58c28eb into tj:release/13.x Jul 8, 2024
@shadowspawn shadowspawn deleted the feature/disallow-excess-arguments branch July 8, 2024 03:28
marcindulak added a commit to marcindulak/asciidoc-link-check that referenced this pull request Dec 30, 2024
@shadowspawn shadowspawn removed the pending release Merged into a branch for a future release, but not released yet label Dec 30, 2024
@shadowspawn
Copy link
Collaborator Author

Released in Commander v13.0.0

jelmore1674 pushed a commit to jelmore1674/build-changelog that referenced this pull request Mar 19, 2025
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [commander](https://github.com/tj/commander.js) | devDependencies | major | [`12.1.0` -> `13.0.0`](https://renovatebot.com/diffs/npm/commander/12.1.0/13.0.0) |

---

### Release Notes

<details>
<summary>tj/commander.js (commander)</summary>

### [`v13.0.0`](https://github.com/tj/commander.js/blob/HEAD/CHANGELOG.md#1300-2024-12-30)

[Compare Source](tj/commander.js@v12.1.0...v13.0.0)

##### Added

-   support multiple calls to `.parse()` with default settings (\[[#&#8203;2299](tj/commander.js#2299)])
-   add `.saveStateBeforeParse()` and `.restoreStateBeforeParse()` for use by subclasses (\[[#&#8203;2299](tj/commander.js#2299)])
-   style routines like `styleTitle()` to add color to help using `.configureHelp()` or Help subclass (\[[#&#8203;2251](tj/commander.js#2251)])
-   color related support in `.configureOutput()` for `getOutHasColors()`, `getErrHasColors()`, and `stripColor()` (\[[#&#8203;2251](tj/commander.js#2251)])
-   Help property for `minWidthToWrap` (\[[#&#8203;2251](tj/commander.js#2251)])
-   Help methods for `displayWidth()`, `boxWrap()`, `preformatted()` et al (\[[#&#8203;2251](tj/commander.js#2251)])

##### Changed

-   *Breaking*: excess command-arguments cause an error by default, see migration tips (\[[#&#8203;2223](tj/commander.js#2223)])
-   *Breaking*: throw during Option construction for unsupported option flags, like multiple characters after single `-` (\[[#&#8203;2270](tj/commander.js#2270)])
-   *Breaking*: throw on multiple calls to `.parse()` if `storeOptionsAsProperties: true` (\[[#&#8203;2299](tj/commander.js#2299)])
-   TypeScript: include implicit `this` in parameters for action handler callback (\[[#&#8203;2197](tj/commander.js#2197)])

##### Deleted

-   *Breaking*: `Help.wrap()` refactored into `formatItem()` and `boxWrap()` (\[[#&#8203;2251](tj/commander.js#2251)])

##### Migration Tips

**Excess command-arguments**

It is now an error for the user to specify more command-arguments than are expected. (`allowExcessArguments` is now false by default.)

Old code:

```js
program.option('-p, --port <number>', 'port number');
program.action((options) => {
  console.log(program.args);
});
```

Now shows an error:

```console
$ node example.js a b c
error: too many arguments. Expected 0 arguments but got 3.
```

You can declare the expected arguments. The help will then be more accurate too. Note that declaring
new arguments will change what is passed to the action handler.

```js
program.option('-p, --port <number>', 'port number');
program.argument('[args...]', 'remote command and arguments'); // expecting zero or more arguments
program.action((args, options) => {
  console.log(args);
});
```

Or you could suppress the error, useful for minimising changes in legacy code.

```js
program.option('-p, --port', 'port number');
program.allowExcessArguments();
program.action((options) => {
  console.log(program.args);
});
```

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS44NC4wIiwidXBkYXRlZEluVmVyIjoiMzkuODQuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->

Reviewed-on: https://git.justinelmore.dev/jelmore1674/build-changelog/pulls/38
Reviewed-by: Justin Elmore <[email protected]>
Co-authored-by: Renovate Bot <[email protected]>
Co-committed-by: Renovate Bot <[email protected]>
kevinoid added a commit to kevinoid/git-branch-is that referenced this pull request May 19, 2025
As a result of tj/commander.js#2223
commander@^13 and later print an error and exit for excess arguments.
This is already tested by 'exit code 1 with extra args works when
executed'.

Signed-off-by: Kevin Locke <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver: major Releasing requires a major version bump, not backwards compatible

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants