flask-script icon indicating copy to clipboard operation
flask-script copied to clipboard

Use standard IPython startup instead of embed

Open bendavis78 opened this issue 12 years ago • 5 comments

Fixes #73

bendavis78 avatar Aug 15 '13 18:08 bendavis78

One reason I changed to use embed was it was part of the public API (init.py) and consistent across versions (after 0.10). I agree and understand the reason not to use it (especially coming from an IPython developer). A couple issues I see with the current pull request:

  • IPython 1.x moved the ipapp module from IPython.frontend.terminal to IPython.terminal so we need some more conditional logic to get TerminalIPythonApp imported
  • Issue #36 was opened wanting the default profile loaded. I saw in the Django issue that this goes back and forth, and ultimately in issue #36 the request was to have the confirm_ext = False. I really don't want to expose another argument in Shell()'s constructor so I think this should just always be set to False (I figure 95%+ would prefer this, if not 100% :) ) or expose it as a Flask config parameter (maybe a key like SCRIPT_IPYTHON_CONFIRM_EXIT) but I don't know at this point if it even needs to be exposed.

techniq avatar Aug 15 '13 20:08 techniq

Yeah as for #36 I agree there's no point to add an argument for confirm_ext, when a better way to do that would be via ipython config. The main thing should be to get user configuration loading correctly. I also realized my patch is broken as it doesn't seem to pull in the user context, so I need to figure that out as well. I'll see if I can add conditional support for the various versions also.

bendavis78 avatar Aug 15 '13 20:08 bendavis78

I just got a chance to look to see how Django is handling this and see that IPython 1.x now has a public API IPython.start_ipython() which looks like what we should be using moving forward (in case it's namespace moves again). https://github.com/django/django/blob/master/django/core/management/commands/shell.py

It also seems like you're doing a lot with the TerminalIPythonApp and TerminalInteractiveShell instances. Is all this needed / is there a cleaning way?

techniq avatar Aug 20 '13 14:08 techniq

Unfortunately start_ipython() doesn't take user_ns (yet). See my stackoverflow question here: http://stackoverflow.com/questions/18318257/how-can-i-start-an-ipython-application-instance-while-passing-a-custom-user-ns-a/18345717#18345717

Ideally ipython should be using TerminalIPythonApp. Unfortunately this is the best way I've found to do it while still supporting a custom user_ns and banner.

bendavis78 avatar Aug 23 '13 17:08 bendavis78

Seems to work for me

jstypka avatar Aug 04 '15 11:08 jstypka