pallets / click

Python composable command line interface toolkit
https://click.palletsprojects.com
BSD 3-Clause "New" or "Revised" License
15.78k stars 1.4k forks source link

Documentation improvement: Implementing custom types and defaults #750

Open MobiusIQ opened 7 years ago

MobiusIQ commented 7 years ago

First, awesome project! Using click has been a breeze and the docs have been very helpful!

Relates to the "Implementing custom types" section in the docs: http://click-docs-cn.readthedocs.io/zh_CN/latest/parameters.html

Thought this was a bug at first, but when creating a simple example project, I realized its more of a minor omission in the docs.

When describing how to make custom types, it is not mentioned that defaults should be specified as a string, not as the expected final type.

Consider the following example:

import click

class BasedIntParamType(click.ParamType):
    name = 'integer'

    def convert(self, value, param, ctx):
        try:
            return int(value, 0)
        except ValueError:
            self.fail('%s is not a valid integer' % value, param, ctx)

BASED_INT = BasedIntParamType()

@click.group()
def cli():
    pass

@click.command()
@click.option("--val", '-v', type=BASED_INT, default = 0)
def bad(val):
    '''
    Attempting to use this command without a value will result in an error
    '''
    click.echo("all is NOT well!")
    if val:
        click.secho("I got a number: {}".format(val))

@click.command()
@click.option("--val", '-v',  type=BASED_INT, default = '0')
def good(val):
    '''
    Sanity check to make sure everything is working on a basic level
    '''
    click.echo("all is well!")
    if val:
        click.echo("I got a number: {}".format(val))

cli.add_command(bad)
cli.add_command(good)

if __name__ == '__main__':
    cli()

results:

(ClickBug) C:\Users\foo\Documents\GitRepos\ClickBug>clickbug good
all is well!

(ClickBug) C:\Users\foo\Documents\GitRepos\ClickBug>clickbug good -v 0x10
all is well!
I got a number: 16

(ClickBug) C:\Users\foo\Documents\GitRepos\ClickBug>clickbug bad
Traceback (most recent call last):
  File "C:\Users\foo\Documents\GitRepos\ClickBug\ClickBug\Scripts\clickbug-script.py", line 11, in <module>
    load_entry_point('ClickBugSimpleProject', 'console_scripts', 'clickbug')()
  File "c:\users\foo\documents\gitrepos\clickbug\clickbug\lib\site-packages\click\core.py", line 722, in __call__
    return self.main(*args, **kwargs)
  File "c:\users\foo\documents\gitrepos\clickbug\clickbug\lib\site-packages\click\core.py", line 697, in main
    rv = self.invoke(ctx)
  File "c:\users\foo\documents\gitrepos\clickbug\clickbug\lib\site-packages\click\core.py", line 1064, in invoke
    sub_ctx = cmd.make_context(cmd_name, args, parent=ctx)
  File "c:\users\foo\documents\gitrepos\clickbug\clickbug\lib\site-packages\click\core.py", line 621, in make_context
    self.parse_args(ctx, args)
  File "c:\users\foo\documents\gitrepos\clickbug\clickbug\lib\site-packages\click\core.py", line 880, in parse_args
    value, args = param.handle_parse_result(ctx, opts, args)
  File "c:\users\foo\documents\gitrepos\clickbug\clickbug\lib\site-packages\click\core.py", line 1396, in handle_parse_result
    value = self.full_process_value(ctx, value)
  File "c:\users\foo\documents\gitrepos\clickbug\clickbug\lib\site-packages\click\core.py", line 1681, in full_process_value
    return Parameter.full_process_value(self, ctx, value)
  File "c:\users\foo\documents\gitrepos\clickbug\clickbug\lib\site-packages\click\core.py", line 1368, in full_process_value
    value = self.get_default(ctx)
  File "c:\users\foo\documents\gitrepos\clickbug\clickbug\lib\site-packages\click\core.py", line 1636, in get_default
    return Parameter.get_default(self, ctx)
  File "c:\users\foo\documents\gitrepos\clickbug\clickbug\lib\site-packages\click\core.py", line 1312, in get_default
    return self.type_cast_value(ctx, rv)
  File "c:\users\foo\documents\gitrepos\clickbug\clickbug\lib\site-packages\click\core.py", line 1344, in type_cast_value
    return _convert(value, (self.nargs != 1) + bool(self.multiple))
  File "c:\users\foo\documents\gitrepos\clickbug\clickbug\lib\site-packages\click\core.py", line 1342, in _convert
    return self.type(value, self, ctx)
  File "c:\users\foo\documents\gitrepos\clickbug\clickbug\lib\site-packages\click\types.py", line 38, in __call__
    return self.convert(value, param, ctx)
  File "c:\users\foo\documents\gitrepos\clickbug\clickbugproject\ClickBugSimpleExample.py", line 14, in convert
    return int(value, 0)
TypeError: int() can't convert non-string with explicit base
achimnol commented 3 years ago

I also just discovered that Click tries to convert the default value in parameters twice. The documentation suggests that we need to check the type of passed value to skip over or perform conversion, but this may not be feasible in some cases, such as passing a JSON string literal for a JSON parameter type, because it is indistinguishable based on type and there are edge cases like strings of escaped strings, etc. refs) https://github.com/lablup/backend.ai-client-py/pull/180

So I think the documentation should also mention this behavior and let people implement a ratchet like me, or make changes in the Click side. Note that, when using a ratchet attribute in the ParamType instance, the parameter definitions should instantiate the custom ParamType objects individually instead of reusing one singleton.

Rowlando13 commented 1 week ago

@achimnol @MobiusIQ We are happy to have a documentation PR into stable for either of these issues if they are still relevant.