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

Change "--config foo:bar" to "--setting foo bar" #992

Closed
simonw opened this issue Oct 5, 2020 · 6 comments
Closed

Change "--config foo:bar" to "--setting foo bar" #992

simonw opened this issue Oct 5, 2020 · 6 comments

Comments

@simonw
Copy link
Owner

simonw commented Oct 5, 2020

I designed the config format before I had a good feel for CLI design using Click. --config max_page_size 2000 is better than --config max_page_size:2000.

@simonw simonw added this to the Datasette 1.0 milestone Oct 5, 2020
@simonw
Copy link
Owner Author

simonw commented Oct 5, 2020

If I rename metadata to config in #493 then this should be --setting instead.

@simonw simonw changed the title Change "--config foo:bar" to "--config foo bar" Change "--config foo:bar" to "--setting foo bar" Oct 6, 2020
@simonw
Copy link
Owner Author

simonw commented Oct 6, 2020

I can add this well in advance of Datasette 1.0 as an alias to the existing --config option (but taking two arguments instead of one).

@simonw
Copy link
Owner Author

simonw commented Nov 24, 2020

This is blocking progress on other metadata tickets like #860 because I want to split the concept of concrete metadata (source, license, etc) from configuration that currently lives in metadata (default sort order, default facets).

I'm going to solve this next to unblock that stuff.

@simonw
Copy link
Owner Author

simonw commented Nov 24, 2020

I'm going to keep --config for the moment but show a deprecation warning that it will be gone in Datasette 1.0.

@simonw
Copy link
Owner Author

simonw commented Nov 24, 2020

Need to figure out the --setting foo bar alternative for this --config foo:bar logic:

class Config(click.ParamType):
name = "config"
def convert(self, config, param, ctx):
if ":" not in config:
self.fail(f'"{config}" should be name:value', param, ctx)
return
name, value = config.split(":", 1)
if name not in DEFAULT_CONFIG:
self.fail(
f"{name} is not a valid option (--help-config to see all)",
param,
ctx,
)
return
# Type checking
default = DEFAULT_CONFIG[name]
if isinstance(default, bool):
try:
return name, value_as_boolean(value)
except ValueAsBooleanError:
self.fail(f'"{name}" should be on/off/true/false/1/0', param, ctx)
return
elif isinstance(default, int):
if not value.isdigit():
self.fail(f'"{name}" should be an integer', param, ctx)
return
return name, int(value)
elif isinstance(default, str):
return name, value
else:
# Should never happen:
self.fail("Invalid option")

@simonw
Copy link
Owner Author

simonw commented Nov 24, 2020

Part of #1105

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant