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

Support case insensitive command line options #116

Merged
merged 5 commits into from
Jul 29, 2021

Conversation

peazncheez
Copy link

Adds a caseSensitive field to OptionDefinition to support parsing certain arguments in a case insensitive manner. The default behavior continues to treat all command line arguments as case sensitive.

The UNIX-based command line is traditionally case sensitive, but the Windows command line is not. I'm working on a Windows application and we're currently using this library, but we'd like our args to be parsed in a case-insensitive manner.

Adds a caseSensitive field to OptionDefinition to support parsing
certain arguments in a case insensitive manner. The UNIX-based command
line is traditionally case sensitive, but the Windows command line is
not. The default behavior continues to treat all command line arguments
as case sensitive.
@peazncheez
Copy link
Author

peazncheez commented Jul 20, 2021

@75lb , mind reviewing / allowing the CI to run? And let me know if I shouldn't check in those dist files if they're generated at a later stage or something.

@75lb
Copy link
Owner

75lb commented Jul 21, 2021

morning, thanks for the new feature - sounds good to me.. I will review it later today.. Yes, the dist should be checked in but don't worry about that - I'll take it from here as I might add some futher updates to freshen the library up..

@peazncheez
Copy link
Author

hey @75lb , any updates? 🙂

@75lb
Copy link
Owner

75lb commented Jul 24, 2021

Will you need the granularity of being able to set caseSensitive at the option level or can we set the flag once when invoking the parse function?? E.g.

commandLineArgs(optionDefinitions, { caseInsensitive: true })

Also, the general convention is to enable an option flag by setting it to true, not false, so I will probably change the option name to caseInsensitive..

@75lb
Copy link
Owner

75lb commented Jul 24, 2021

Why are aliases always case-sensitive? Wouldn't it be more consistent to enable or disable case-sensitivity across the board?

- Set caseInsensitive flag at parse-level rather than at option-level
- Make aliases respect caseInsensitive flag
@peazncheez peazncheez force-pushed the peazncheez/case-insensitive-arg branch from 86af4bf to e98c7be Compare July 26, 2021 21:52
Removes the usage of default parameter values
@peazncheez
Copy link
Author

Hey @75lb , thanks for the feedback. I had originally implemented the per-option case sensitivity granularity since that would be most flexible, but it's not necessary for my use case and I agree the additional complexity likely isn't worth it.

Pushed up some commits which:

  • switch over to a single parse-level case insensitivity flag rather than the previous per-option-definition flags
  • makes the case sensitivity flag apply to aliases
  • renames the flag to be caseInsensitive as you recommended

@75lb 75lb merged commit f7df343 into 75lb:master Jul 29, 2021
@75lb
Copy link
Owner

75lb commented Jul 29, 2021

Merged and released in v5.2.0, thanks. 👍

@peazncheez
Copy link
Author

Awesome, thank you!

peazncheez pushed a commit to peazncheez/DefinitelyTyped that referenced this pull request Aug 6, 2021
Case insensitive support was added to the command-line-args library in
75lb/command-line-args#116 and was released in
5.2.0. This commit updates the corresponding types.
typescript-bot pushed a commit to DefinitelyTyped/DefinitelyTyped that referenced this pull request Aug 6, 2021
…@peazncheez

* Add types for case insensitive command line arg parsing

Case insensitive support was added to the command-line-args library in
75lb/command-line-args#116 and was released in
5.2.0. This commit updates the corresponding types.

* Remove tsconfig changes

Co-authored-by: Bryan Williams <bwilliams@palantir.com>
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

Successfully merging this pull request may close these issues.

2 participants