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

Fix missing option on typings #9

Merged
merged 6 commits into from
Oct 13, 2018
Merged

Conversation

csprance
Copy link
Contributor

Fixed typings on missing option

noAliasPropagation?: boolean | 'first-only';

@csprance csprance changed the title Add typings Fix missing option on typings Oct 10, 2018
@Alhadis
Copy link
Owner

Alhadis commented Oct 10, 2018

I played around with this a bit locally and noticed the typings require all three arguments to be passed:

// test.ts
import getOpts from "get-options";
console.log(getOpts(["--help", "--verbose"], {
    "--help, -?": "",
    "--verbose":  "",
}));
λ tsc test.ts
test.ts:2:13 - error TS2554: Expected 3 arguments, but got 2.

2 console.log(getOpts(["--help", "--verbose"], {
              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3     "--help, -?": "",
  ~~~~~~~~~~~~~~~~~~~~~
4     "--verbose":  "",
  ~~~~~~~~~~~~~~~~~~~~~
5 }));
  ~~

  node_modules/get-options/index.d.ts:20:2
    20  config: Config
        ~~~~~~~~~~~~~~
    An argument for 'config' was not provided.

Both the second and third arguments are actually optional (optdef should probably have a default value in the function header to communicate this better...). Basically, if you don't define anything specific, it tries to do the right thing with its input.

Would it be possible to amend the typings to reflect this?

@csprance
Copy link
Contributor Author

I updated the PR

@Alhadis Alhadis merged commit 9132740 into Alhadis:master Oct 13, 2018
@Alhadis
Copy link
Owner

Alhadis commented Oct 13, 2018

Thanks! Sorry for the delay.

I decided I'm going to adjust the behaviour of getOpts a little so it handles string arguments a little better. Currently, getOpts("--help") is decoded as ["-", "-", "h", "e", "l", "p"], whereas a much more realistic interpretation would be {help: true}. What would need to be changed in the typings to reflect this?

The change of semantics is largely inspired by the need to parse environment variables like NODE_OPTIONS and co, which may or may not contain quoted values with spaces (which complicate accurate parsing).

@csprance
Copy link
Contributor Author

Hmm, good question. I guess if you want to start a new branch with your changes I can take a look at it and see what the typings would look like.

Can you create a new version on npm 1.1.3 with the updates for the types?

@Alhadis
Copy link
Owner

Alhadis commented Oct 21, 2018

Shit! Sorry mate, I was meant to do this earlier, but I got sidetracked. 😞

Will cut a release now.

@Alhadis
Copy link
Owner

Alhadis commented Oct 22, 2018

Alright, the string-handling behaviour was implemented in bf7a188. I'm guessing this would be all that's needed for the typings?

diff --git a/index.d.ts b/index.d.ts
index b2a04b3..c8f2e1c 100644
--- a/index.d.ts
+++ b/index.d.ts
@@ -7,7 +7,7 @@
  * Extract command-line options from a list of strings.
  */
 declare function getOpts(
-	input: any[],
+	input: string | any[],
 	optdef?: string | { [key: string]: string },
 	config?: getOpts.Config
 ): getOpts.Options;

Hold off on any PRs though, I intend to add new options for features I've been wanting to implement. I'll ping you for review before cutting the next release. 😉

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.

None yet

2 participants