feat(flags): infer argument types, names and defaults #2180

c4spar · 2022-05-02T22:42:03Z

CLAassistant · 2022-05-02T22:42:07Z

All committers have signed the CLA.

kt3k · 2022-05-04T10:51:16Z

flags/test.ts

+import {
+  assert,
+  IsExact,
+} from "https://deno.land/x/conditional_type_checks@1.0.5/mod.ts";


We prefer to avoid depending on deno.land/x/ from std. Can you copy the necessary part of the module in std as test utility?

Yep i know, it was only temporarily 😉 will do 👍

kt3k · 2022-05-04T10:57:39Z

flags/test.ts

+
+/** ---------------------- TYPE TESTS ---------------------- */
+
+Deno.test("typesOfAllBoolean", function (): void {


Nice test cases! These illustrate what's done in this change very well

kt3k · 2022-05-04T10:58:50Z

@c4spar Can you fix the type error in CI?

error: TS2322 [ERROR]: Type 'unknown' is not assignable to type 'boolean | undefined'.
      showDirListing: serverArgs["dir-listing"],
      ~~~~~~~~~~~~~~
    at file:///Users/runner/work/deno_std/deno_std/http/file_server.ts:810:7

c4spar · 2022-05-04T17:39:22Z

Oops, I assumed dashed parameters are camelcased 😅 is there a reason why we don't do that?

c4spar · 2022-05-23T16:32:14Z

flags/mod.ts

+
+/** Same as `Record` but also supports dotted and negatable options. */
+type MapTypes<T extends ArgType, V> = undefined extends T ? Record<never, never>
+  : T extends `no-${infer Name}` ? MapTypes<Name, V | false>


I think the last thing that is missing is the handling of negatable options. How do we want to proceed with them? do we want to make them configurable by prefixing them with no- or with a separate option?

@c4spar
Sorry for the delay in response.

do we want to make them configurable by prefixing them with no- or with a separate option?

I don't think we should change it optional. That feels like an unnecessary breaking change to me. In my view --no-foo shorthand is always useful for boolean args. So I suggest we just keep the existing behavior

For negation of non-boolean args, I can't think of reasonable usages of it. So I'd suggest we should leave it undefined behavior and let's ignore it in typings. What do you think?

I still think we should keep support for negation of non boolean args.
A good example is deno run --no-config 😉 It's a string arg with a default behavior that can be disabled.

But if we don't make it configurable than all string and collectable args can be also false, which is a bit annoying.

Ok. --no-config looks like a useful example.

But if we don't make it configurable than all string and collectable args can be also false, which is a bit annoying.

I agree with this. What API do you suggest for configurable negatable args?

Is the following example what you suggest for configurable negatable args? (I think this is not implemented though)

boolean: ["foo", "no-bar", "no-dotted.tick", "dotted.tock"], string: ["beep", "no-boop", "no-dotted.zig", "dotted.zag"],

Is the following example what you suggest for configurable negatable args? (I think this is not implemented though)

This would be the easiest solution and the typings for this are already implemented in this PR. I have also implemented the logic but only locally, i can open another PR for this.

Another solution would be to add a new negate option, same like the collect option. But the typings would be more complex for this solution.

@c4spar

This would be the easiest solution and the typings for this are already implemented in this PR. I have also implemented the logic but only locally, i can open another PR for this.

Thank you for your efforts on this, but this syntax (boolean: ["no-bar"]) for indicating negatable arg looks confusing/too implicit to me.

Another solution would be to add a new negate option, same like the collect option. But the typings would be more complex for this solution.

This solution looks much better to me as it's more explicit and probably easier to document. Also this feels consistent with collect option for configuring arg behaviors.

Thank you for your efforts on this, but this syntax (boolean: ["no-bar"]) for indicating negatable arg looks confusing/too implicit to me.

I agree with you, I also don't really like it to activate --foo and --no-foo only with no-foo.

I would like it more if you could explicitly activate both separately like this string: ["foo", "no-foo"]. foo would add the type string and no-foo would add the type false. But this doesn't really work if only no-foo is defined, because foo is always a valid argument. A workaround could be to set foo to unknown if only no-foo is defined.

But i'm also fine with a new option to align with the collect option.

I tried both solutions already, but with both solutions I had some issues with union types, but I can give it a second try.

ok i got the types for the negatable option working now 😉

dsherret · 2022-06-01T14:14:15Z

flags/mod.ts

+
+type CollectType = string | undefined;
+
+type NoTypes<B, S, C> = undefined extends (


Instead of this being a negative name ("No"), maybe instead use a positive name ("UseTypes" maybe?).

dsherret · 2022-06-01T14:17:13Z

flags/mod.ts

+
+type CollectType = string | undefined;
+
+type NoTypes<B, S, C> = undefined extends (


Maybe also add the type parameter constraints here to make it a little more clear what these type parameters could possibly be.

Though not necessary, it actually might be good to move away from single letter type parameter names and instead use longer more descriptive names because of how complex this is (I usually prefix type parameter names with a T to distinguish them from other types)

kt3k

Great improvement of the usability of flags. Nice work!

LGTM

kt3k · 2022-06-14T09:12:39Z

I think this PR is ready to land now.

@bartlomieju Do you have any concern about landing this?

dcdunkan · 2022-06-20T04:50:43Z

flags/mod.ts


  /** When `true`, populate the result `_` with everything after the first
   * non-option. */
  stopEarly?: boolean;

  /** A string or array of strings argument names to always treat as strings. */
-  string?: string | string[];
+  string?: S | ReadonlyArray<Extract<S, string>>;

  /** A string or array of strings argument names to always treat as arrays.
   * Collectable options can be used multiple times. All values will be
   * colelcted into one array. If a non-collectable option is used multiple


There's a typo here: colelcted to collected?

fixed. Thx @dcdunkan 👍

bartlomieju · 2022-06-20T11:38:52Z

I think this PR is ready to land now.

@bartlomieju Do you have any concern about landing this?

No, this looks good to me!

kt3k · 2022-06-21T06:36:34Z

Ok. Let's land now!

Thanks again for the series of works for landing this PR @c4spar!

feat(flags): infer argument types and defaults

7b5dd5f

c4spar requested review from bartlomieju and kt3k as code owners May 2, 2022 22:42

c4spar changed the title ~~feat(flags): infer argument types and defaults~~ feat(flags): infer argument types, names and defaults May 2, 2022

c4spar added 10 commits May 3, 2022 19:25

some fixes

02803d8

move types to bottom

bd40fab

remove underscore check from CamelCase type

1e40808

add temporarily conditional_type_checks lib for testing generics

b59779f

use unknown for default values that don't have a defined type

56fc7a1

fix test

3d8c794

add tests for dotted options

aee7406

fix test names

6cd12c5

fix tests

b156b77

fix all boolean

e9b14fd

kt3k reviewed May 4, 2022

View reviewed changes

c4spar added 11 commits May 4, 2022 22:05

add assertType, IsExact and IsAny from conditional_type_checks

8843456

remove camelcase type

d28135f

add test case for default options

ba659ba

add partially support for dotted options

f322bde

add test cases for nested dotted options

1d78786

fix dotted options with defaults

57ef1dd

add test cases for dashed params

53d7331

fix all boolean disabled and add tests

33d124a

add generics for double dash option

3710f79

fix nullish default values

5dd8f96

fmt

6042576

c4spar added 4 commits May 23, 2022 14:33

fix ArgType

86f0127

remove types for collectAll

8caab31

update docs

ddd3fe9

minor

0d2ba0f

c4spar commented May 23, 2022

View reviewed changes

c4spar and others added 6 commits May 24, 2022 22:09

update readme

e9f2393

Merge remote-tracking branch 'origin/main' into flags/generic-types

e1a44d0

fix merge conflict

a282a1f

use hasOwn instead of in operator in hasKey

3d25345

remove unnecessary change

d25657f

Merge branch 'denoland:main' into flags/generic-types

88db884

dsherret reviewed Jun 1, 2022

View reviewed changes

c4spar added 3 commits June 1, 2022 23:11

add types for negatable option and some refactorings

e8f9618

update UseTypes

645a93b

add comma

664780a

c4spar mentioned this pull request Jun 2, 2022

Flags: Make negatable options configurable #2300

Closed

kt3k mentioned this pull request Jun 9, 2022

BREAKING(flags): introduce negatable option #2301

Merged

c4spar added 2 commits June 9, 2022 18:37

Merge remote-tracking branch 'origin/main' into flags/generic-types

afb3585

merge conflict fixes

948b701

kt3k approved these changes Jun 10, 2022

View reviewed changes

dcdunkan reviewed Jun 20, 2022

View reviewed changes

c4spar and others added 2 commits June 20, 2022 22:14

Merge branch 'denoland:main' into flags/generic-types

97d75cf

fix typo

f7f8b9a

kt3k merged commit 45cf771 into denoland:main Jun 21, 2022

cjihrig pushed a commit to bartlomieju/deno_std that referenced this pull request Jun 28, 2022

feat(flags): infer argument types, names and defaults (denoland#2180)

fec921f

c4spar mentioned this pull request Dec 23, 2022

refactor(flags): use meaningful names instead of single chars for generic params #3044

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(flags): infer argument types, names and defaults #2180

feat(flags): infer argument types, names and defaults #2180

c4spar commented May 2, 2022

CLAassistant commented May 2, 2022 •

edited

Loading

kt3k May 4, 2022

c4spar May 4, 2022 •

edited

Loading

kt3k May 4, 2022

kt3k commented May 4, 2022

c4spar commented May 4, 2022

c4spar May 23, 2022

kt3k May 30, 2022

c4spar May 30, 2022

kt3k May 31, 2022

c4spar May 31, 2022

kt3k Jun 1, 2022

c4spar Jun 1, 2022

c4spar Jun 1, 2022

dsherret Jun 1, 2022

dsherret Jun 1, 2022

kt3k left a comment

kt3k commented Jun 14, 2022

dcdunkan Jun 20, 2022

c4spar Jun 20, 2022

bartlomieju commented Jun 20, 2022

kt3k commented Jun 21, 2022


		/** ---------------------- TYPE TESTS ---------------------- */

		Deno.test("typesOfAllBoolean", function (): void {


		type CollectType = string \| undefined;

		type NoTypes<B, S, C> = undefined extends (

feat(flags): infer argument types, names and defaults #2180

feat(flags): infer argument types, names and defaults #2180

Conversation

c4spar commented May 2, 2022

CLAassistant commented May 2, 2022 • edited Loading

Choose a reason for hiding this comment

c4spar May 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kt3k commented May 4, 2022

c4spar commented May 4, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kt3k left a comment

Choose a reason for hiding this comment

kt3k commented Jun 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bartlomieju commented Jun 20, 2022

kt3k commented Jun 21, 2022

CLAassistant commented May 2, 2022 •

edited

Loading

c4spar May 4, 2022 •

edited

Loading