Closed balejk closed 1 year ago
Perfect -- yes, you should be able to add it to _Config
(with or without a default). I'd be fine with making it required on the _Config
class (so that there's only one place the "2" is specified -- in the Click/cli defaults).
Thanks!
Thank you for merging this!
Perfect -- yes, you should be able to add it to
_Config
(with or without a default). I'd be fine with making it required on the_Config
class (so that there's only one place the "2" is specified -- in the Click/cli defaults).
Sorry, I am not sure that I follow: are you suggesting that the code
length would be None
in _Config
and that it would always get set to
the Click value (defaulting to 2 if unspecified on the command line)
using evolve
(which is really the case now already, except it's
already 2 in _Config
by default - so just changing this to None
is
what you mean?)?
Sorry, I hadn't read the code properly when I wrote that -- I was thinking that it could just be a required argument for _Config
(so no default at all).
However, since --code-length
is only on the "invite" subcommand the evolve
call is probably right.
That said, if you're willing it might be nicer to make that a global option (and hence a required _Config
option). I haven't done it here (yet?) but there's no technical reason why the fowl invite
has to "create" a code and fowl accept
"consumes" the code. (Basically I chose that because this seems easier to understand, and "usually" makes sense for the way I would use it -- but the protocol could allow the "invite" side to consume a code created by the "accept" side).
What I'm saying is: --code-length
could make (technical) sense as a global option, probably.
(Note that I've sort of "cheated" with the _Config
class a little since it's holding options that aren't strictly global ....)
Anyway, thanks for the PR and the evolve()
call is fine too :)
It might be nice to use the
_Config
default for the CLI default automatically, but I have never usedattrs
and frozen classes before so I do not know what the best way to do that would be (is it possible without instantiating the class?) and so I followed the example of--clearnet
and set the CLI default to the same value manually.