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

Rework default program name, and searching for subcommands #1571

Merged
merged 23 commits into from Aug 6, 2021

Conversation

shadowspawn
Copy link
Collaborator

@shadowspawn shadowspawn commented Jul 18, 2021

Pull Request

The classic approach for scripts to find how they are called is to check the passed arguments, which often start with the interpreter and the script name. e.g. ['/usr/local/bin/node', '/Users/developer/test.js', '--print']. However, this is not always available or consistent for Commander use cases.

The implied script path found in .parse() affects the default program name, and searching for subcommands if stand-alone executable commands are used.

Problem

  • default program name gives different results depending on how script launched
  • default stand-alone executable subcommand search gives different results for npm global commands on Windows and non-Windows
  • naming the program (or any command) is under-documented in the README
  • searching for stand-alone executable subcommands is under-documented in the README
  • looking for stand-alone executable subcommand using script name is a bit subtle, and using command name would be a simpler and more consistent model
  • adding the require.main.filename fallback has introduced different (arguably better) behaviours than script name, but can't be made primary behaviour because not available in es6
  • if script name not available, must use executableFile per command

Note: we do have .name() and executableFile, so there is technically a way for user to get things working.

Deep dive: #1569

Related (including some issues for which executableFile is a solution): #481 #521 (comment) #532 #714 #785 #786 #826 #830 #875 #890 #912 #1044 #1417 #1439

Solution

Node.js itself does not offer universal solutions for finding a reference script name or a directory which we can use from inside Commander. There are some approaches that client can use. e.g. __filename, __dirname, process.argv[1], require.main.filename, import.meta.url.

Keep it simple. The script name is the classic mechanism, and Commander has always used it as the primary implicit context. Stick with it, and improve the documentation, and improve custom control.

Expand documentation about naming commands, and especially program. Rework documentation about finding subcommands.

Remove fallback to require.main.filename since it adds yet more behaviours, and only a partial solution as not available in es6.

Use program (command) name rather than script name as prefix for looking for standalone executable subcommands. Include a backwards compatible fallback subcommand lookup using the script name.

Add .executableDir() for simple custom configuration, including when script name not supplied. (c.f. .executableFile)

(Includes some additional minor markdown and example code fixes.)

ChangeLog

  • changed: use command name as prefix for subcommand stand-alone executable name (with fallback to script name for backwards compatibility)
  • added .executableDir() for custom search for subcommands
  • allow absolute path with executableFile
  • removed internal fallback to require.main.filename when script not known from arguments passed to .parse() (can supply details using .name(), and .executableDir() or executableFile)
  • removed restriction that nested subcommands must specify executableFile

@shadowspawn shadowspawn added the semver: major Releasing requires a major version bump, not backwards compatible label Jul 18, 2021
@shadowspawn shadowspawn marked this pull request as ready for review August 1, 2021 02:00
Copy link
Collaborator

@abetomo abetomo left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@shadowspawn shadowspawn changed the base branch from develop to release/9.x August 6, 2021 22:47
@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Aug 6, 2021
@shadowspawn shadowspawn merged commit b1b7aca into tj:release/9.x Aug 6, 2021
@shadowspawn shadowspawn deleted the feature/discovering-context branch August 6, 2021 22:48
@shadowspawn shadowspawn added this to the Commander v9.0.0 milestone Sep 6, 2021
@shadowspawn shadowspawn removed the pending release Merged into a branch for a future release, but not released yet label Jan 29, 2022
@shadowspawn
Copy link
Collaborator Author

Commander v9 has been released.

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.

None yet

2 participants