add pshell --list-shells and default_shell ini options #2012
Conversation
|
|
||
| $ $VENV/bin/pshell --list-shells | ||
| Available shells: | ||
| bpython [not available] |
There was a problem hiding this comment.
I think it would be better just to omit listing any shell whose factory returns None in the default listing.
There was a problem hiding this comment.
Most factories should not return None because their bindings should depend on their reqs. This is mainly for bpython and ipython and I wanted some way to indicate to the user that things are good, they just need to install bpython.
There was a problem hiding this comment.
Given that we've gone to the trouble to factor this with entry points, wouldn't it be better to split out pyramid_bpython and pyramid_ipython packages, each with appropriate dependencies, and registering the relevant entrypoint? Then, anybody who wants one of them just installs that (mostly-empty) package, just like any other shell.
There was a problem hiding this comment.
Conversely pyramid ships a cherrypy wsgi entry point for no apparent reason.
|
LGTM |
|
👍 |
mmerickel
changed the title
|
@sontek please provide some thoughts before I merge as it's building on your work. |
|
Great, now we can choose the shell for pshell. |
| @@ -77,6 +96,8 @@ def pshell_file_config(self, filename): | |||
| for k, v in items: | |||
| if k == 'setup': | |||
| self.setup = v | |||
| elif k == 'default_shell': | |||
There was a problem hiding this comment.
would preferred_shells be a better name for this in the config?
There was a problem hiding this comment.
I started with that and changed to default_shell but I can change it back. I don't really care. Naming is hard. Next someone is going to ask to load the option from an environment variable anyway.
|
@mmerickel I love it =) I wanted to rip the shells out but didn't think we'd want to break backwards compatibility. Only thing I saw was the config |
|
This is probably too much but I realized that none of the shells themselves depend on pyramid so maybe the entrypoint doesn't need to be called something pyramid-specific. For example my apps tend to have their own |
| myshell=my_app:ptpython_shell_factory | ||
| """ | ||
| entry_points={ | ||
| 'pyramid.pshell': [ |
There was a problem hiding this comment.
That has to be pyramid.pshell_runner. Everything else looks good to me 👍
There was a problem hiding this comment.
Nice catch, thank you.
|
|
add pshell --list-shells and default_shell ini options
``pyramid.pshell`` is now ``pyramid.pshell_runner`` see: Pylons/pyramid#2012
I'm malleable on the naming of
default_shell. I consideredpreferred_shellsas well.