-
-
Notifications
You must be signed in to change notification settings - Fork 144
Refactor args.config processing #103
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
Conversation
pdoc/cli.py
Outdated
| for opt in args.config: | ||
| try: | ||
| config_key, config_value = opt.split('=', 1) | ||
| config_value = ast.literal_eval(config_value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
literal_eval is more restricted than eval and thus prevents certain vulnerabilities. Not sure if anyone is using features supported only by eval?
| raise RuntimeError( | ||
| 'Error evaluating config value {!r}\n' | ||
| 'Make sure string values are quoted?'.format(opt, e) | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switches error message to being for the single bad config option to hopefully be more specific.
pdoc/cli.py
Outdated
| except Exception as e: | ||
| raise RuntimeError( | ||
| 'Error evaluating config value {!r}\n' | ||
| 'Make sure string values are quoted?'.format(opt, e) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Template is missing a placeholder for e.
If opt is repr-ed, don't we end up with too many quotes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If opt is repr-ed, don't we end up with too many quotes?
Possibly, it could be useful for the unquoted configuration value case, but probably better without overall.
>>> print("{!r}".format("'configuration_value'"))
"'configuration_value'"
>>> print("{!r}".format("'configuration value'"))
"'configuration value'"
>>> print("{!r}".format('"configuration value"'))
'"configuration value"'
>>> print("{!r}".format("unquoted configuration value"))
'unquoted configuration value'There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Template is missing a placeholder for
e.
Addressed in 78d3152
|
As of 78d3152, here is the output for a bad --config: pdoc3 --html \
--template-dir doc/pdoc_template \
--force --output-dir doc \
--config "variable=this should be bad" \
pdoc
Traceback (most recent call last):
File "/home/dhimmel/Documents/repos/pdoc/pdoc/cli.py", line 378, in main
config_value = ast.literal_eval(config_value)
File "/home/dhimmel/anaconda3/lib/python3.6/ast.py", line 48, in literal_eval
node_or_string = parse(node_or_string, mode='eval')
File "/home/dhimmel/anaconda3/lib/python3.6/ast.py", line 35, in parse
return compile(source, filename, mode, PyCF_ONLY_AST)
File "<unknown>", line 1
this should be bad
^
SyntaxError: invalid syntax
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/dhimmel/Documents/repos/pdoc/bin/pdoc3", line 11, in <module>
load_entry_point('pdoc3', 'console_scripts', 'pdoc3')()
File "/home/dhimmel/Documents/repos/pdoc/pdoc/cli.py", line 384, in main
.format(error.__class__.__name__, config_str, error)
RuntimeError: SyntaxError evaluating --config variable=this should be bad
invalid syntax (<unknown>, line 1)
Make sure string values are quoted?A bit repetitive and verbose, but it gets the problem across. Another option is to raise a warning rather than an error and proceed with execution. |
|
Too severe for a warning. Made it just a tad less repetitive, as you say. Thanks! |
No description provided.