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

Bug in regex for lone short flag #2227

Closed
sorawee opened this issue Jul 23, 2024 · 4 comments
Closed

Bug in regex for lone short flag #2227

sorawee opened this issue Jul 23, 2024 · 4 comments

Comments

@sorawee
Copy link

sorawee commented Jul 23, 2024

#1256 added a support for lone short flag.

Unfortunately, this only works correctly on short flags with one character. Short flags with multiple characters (Java-style short flags) will incorrectly create incorrect field names. For example, .option('-name') will result in a field Name, which is not in camelCase.

The reason is that this regex only matches a short flag with one character. This means a short flag with multiple characters will be categorized as a long flag. Consequently, Commander.js attempts to strip -- from -name, which is a no-op, resulting in -name. Then, the camel case conversion of -name results in Name, which is not a camel case as intended.

@shadowspawn
Copy link
Collaborator

Short flags with more than one character are not supported. Short flags are a dash followed by a single character. See #2211 for more details.

(Feel free to ask if you have more questions.)

@sorawee
Copy link
Author

sorawee commented Jul 23, 2024

Thanks @shadowspawn! That makes sense. But if Java-style short flags are intentionally not supported, shouldn't .option('-name') result in an error to let users know that it is not supported (at least for now)? I think we agree that categorizing -name as a long flag should not happen, right?

@shadowspawn
Copy link
Collaborator

But if Java-style short flags are intentionally not supported, shouldn't .option('-name') result in an error to let users know that it is not supported (at least for now)?

For background, I didn't want to immediately break existing programs when I first found that people were specifying multi-character short flags that were not intended. So the first steps we took were to clarify README and saying this was not supported (#937 #1718).

It hasn't come up often since then, but it could be time to add an error since unsupported option syntax has come up 3 times in last month! (#2211 #2222 #2227)

I think we agree that categorizing -name as a long flag should not happen, right?

Correct. POSIX has clear rules for specifying short option and option-value, and -a AAA is the same as -aAAA. Long options allow multiple characters in name and start with two dashes.

The few applications I am familiar with that allow -multi-character-option-after-one-dash date back to when long options were first being introduced. There were a few styles being tried and it was a while before the now common --double-dash-for-long became the clear winner.

@shadowspawn
Copy link
Collaborator

Closing in favour of #2235, adding an error for short flag longer than one character

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants