| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
Whoooo ! Will review ! |
Sorry, something went wrong.
|
So before reviewing I'll still ask: It seem like PTShell make it more difficult (impossible) to us this in things like emacs inferior shell. It will likely not be the only one, so should we really get rid of the readline one ?Or leave it as an option ? |
Sorry, something went wrong.
+1 otherwise. |
Sorry, something went wrong.
I'm concerned that if we have two sets of terminal interaction code and are mainly using one, it will bit rot. Maybe we could split it out and have it as a separate package for emacs users to maintain? But the APIs the InteractiveShell subclasses use are not very well defined, so I'm not really keen on that either. |
Sorry, something went wrong.
ping @tacaswell
Fair enough. We can always resurrect the code from git right ? That's what the git dig command from the git-archaeologist plugin is for ? |
Sorry, something went wrong.
|
I am motivated to look in to this (and suspect that it can be fixed on the emacs side) and if not then on the pt side to not emit some escapes. Emacs chokes in a function that converts ansi escapes into emacs font faces. This is 90% conjecture. |
Sorry, something went wrong.
|
Great, I'm glad someone has the motivation to investigate. We'll put this on hold for the moment then to see if you can work the issue out. If it's not possible, we'll work out what we're going to do. |
Sorry, something went wrong.
|
@tacaswell have you had a chance to look at this? |
Sorry, something went wrong.
|
A bit but not enough to resolve it. On Tue, May 10, 2016, 06:18 Thomas Kluyver notifications@github.com wrote:
|
Sorry, something went wrong.
|
@tacaswell any more updates? I'd like to get this code out before we release 5.0. |
Sorry, something went wrong.
I would be +1 for merging, we can revert if really necessary, but I don't see us reverting PTK for now. |
Sorry, something went wrong.
|
between day-job and mpl 1.5.2/2.0 I have had no time 😞 When is 5.0 slated to be out? attn @jorgenschaefer who might be able to tell me that is is just a minor configuration thing. |
Sorry, something went wrong.
|
I do not use ipython, so no idea about any of this, sorry. Emacs' python.el has some hardcoded stuff in python-shell-completion-native-setup that tries to use readline or rlcompleter in turn. I don't know at what point it fails, but the correct fix here would be (IMHO) to fix Emacs. Though, considering the release cycle, that can take a while. For the curious, the Python code Emacs uses: https://github.com/emacs-mirror/emacs/blob/master/lisp/progmodes/python.el#L3206-L3254 |
Sorry, something went wrong.
I hear you.
I'd like to have it out for end of June, to have pushed in conad for SciPy.
Thanks for pitching in. Will try to get a few emacs-users to pitch in, will ping people on ML, and twitter. |
Sorry, something went wrong.
|
Does this remove the dependency on gnureadline on Macs? That would be nice :-) |
Sorry, something went wrong.
|
Yep, and the dependency on pyreadline on Windows :-) |
Sorry, something went wrong.
|
@jorgenschaefer The issue is not with calls into ipython, it is with colorizing the string that comes back from ptprompt Debugger entered--Lisp error: (args-out-of-range "�[8D�[0m�[J�[0;38;5;28mIn [�[0;38;5;10;1m1�[0;38;5;28m]: �[8D�[0m
�[J�[0m" 80)
string-match("�" "�[8D�[0m�[J�[0;38;5;28mIn [�[0;38;5;10;1m1�[0;38;5;28m]: �[8D�[0m
\n�[J�[0m" 80)
ansi-color-filter-apply("�[?25l�[8D�[0m�[J�[0;38;5;28mIn [�[0;38;5;10;1m1�[0;38;5;28m]: �[8D�[0m
\n�[J�[0m�[?7h�[?12l�[?25h")
python-shell-comint-watch-for-first-prompt-output-filter("�[?25l�[8D�[0m�[J�[0;38;5;28mIn [�[0;38;5;10;1m1�[0;38;5;28m]: �[8D�[0m
\n�[J�[0m�[?7h�[?12l�[?25h")
run-hook-with-args(python-shell-comint-watch-for-first-prompt-output-filter "�[?25l�[8D�[0m�[J�[0;38;5;28mIn [�[0;38;5;10;1m1�[0;38;5;28m]: �[8D�[0m
\n�[J�[0m�[?7h�[?12l�[?25h")
comint-output-filter(#<process Python> "�[?25l�[8D�[0m�[J�[0;38;5;28mIn [�[0;38;5;10;1m1�[0;38;5;28m]: �[8D�[0m
\n�[J�[0m�[?7h�[?12l�[?25h")
Which given how badly gh is mangling this copy/paste makes me assume it is an encoding issue. |
Sorry, something went wrong.
|
Looks like a bug in ansy-color-filter-apply, again something to report to the Emacs maintainers :-) |
Sorry, something went wrong.
|
@tacaswell As a workaround, you can probably use %colors NoColor before execution the code which has the problem... Or is that problem happinging in lisp code? Can't actually read/parse it :-) |
Sorry, something went wrong.
|
I think we add the discussion on twitter, and as far as I can remember it's a question of inferior-shell, vs ansi-terminal: M-X ansi-term I know there is a reason why you can't use ansi-term, but I'm not sure I remember why. Also as we see on the screenshots, the color does works, it's so extra escape sequences that are broken, so I'm unsure NoColor is a solution. [edit] I have no clue what I am doing above. |
Sorry, something went wrong.
|
The reason I use shell instead of ansi-term is that in what elpy-shell-switch-to-shell (eventually) delegates to (hence why I tried to get @jorgenschaefer interested as I suspect that elpy will be getting this bug report in the near future). |
Sorry, something went wrong.
Ah, yes I'm starting to remember. Then could elpy be made configurable to use ansi-term ? Or is there a reason not to ? (https://www.masteringemacs.org/article/running-shells-in-emacs-overview) |
Sorry, something went wrong.
|
@takluyver would one option be to "extract" the readline TIS thing into a separate package and "offer it to the community" [Yes I know this solution sucks is not optimal]. |
Sorry, something went wrong.
|
It is difficult to interact with ansi-term from Emacs proper. All the REPL modes use comint. (In theory, ipython should check termcap/terminfo capabilities and not send colors or any terminal commands at all – os.environ["TERM"] == "dumb".) |
Sorry, something went wrong.
|
So technically it's not color, it's the cursor movement sequences and blinking (plus a few other I haven't tracked down yet) , and as this is mostly how prompt_toolkit does it thing, it will likely be super hard to avoid. Looking a bit at it, it should be (relatively) straightforward to export the "old" terminal interactive shell out of there into a separate package. Tell me if I'm wrong but M-x shell seem to not handle forward some things like tab completion (do elpy does it through readline ? ) in which case we should be able to reduce that even further. That would (at least) give some fallback for the time being until we figure out a better plan. As I'm unsure about how elpy-shell-switch-to-shell is used, I have difficulties understanding what feature is needed, what behavior is expected. |
Sorry, something went wrong.
|
It's certainly possible to split out the old TIS as a separate package, but I'm wary of doing that, because:
I don't know what the answer is here, but I'm really not keen on the idea of a separate readline TIS package. There is a very dumb fallback mode used for tests, which just uses Python's raw_input. But you're no better off with that than with the vanilla Python shell. |
Sorry, something went wrong.
That untrue for magics, ls, and all the syntax on top of Python. |
Sorry, something went wrong.
|
And most importantly (from my point of view) the gui event-loop integration management. |
Sorry, something went wrong.
|
Have a go with the environment variable IPY_TEST_SIMPLE_PROMPT=1, and see how that goes. The event-loop integration has all been rewritten (and greatly simplified) for prompt_toolkit, though, so the old inputhook machinery is part of what we'd really like to drop. |
Sorry, something went wrong.
|
One option that might work for a community maintained package is to clean up jupyter_console 4.x, rename that and push it out for whoever's interested in maintaining it - that way it's built on a clearly defined interface (the messaging protocol), and it can take advantage of the event-loop integration in the kernel. jupyter_console 5.x is also based on prompt_toolkit, but 4.x uses readline. |
Sorry, something went wrong.
|
(we could call it 'jupyter inferior shell', which would be accurate in more ways than one ;-) |
Sorry, something went wrong.
|
Using IPY_TEST_SIMPLE_PROMPT=1 appears to fix things for me. Colors are off from what they were, but I suspect that I can find the right On Thu, May 26, 2016 at 5:37 PM Thomas Kluyver notifications@github.com
|
Sorry, something went wrong.
|
Have you tested with event loop integration? I'd be surprised if that works with IPY_TEST_SIMPLE_PROMPT. Prompts won't have colour at all, and there's probably no tab completion, unless emacs is doing some magic to provide completions separately. |
Sorry, something went wrong.
|
It looked like the event loop integration worked (had prompt back, could pan/zoom). emacs does it's own colorization, history, and if set up correctly, completion (via jedi). |
Sorry, something went wrong.
Does multiline input works ? (Sidenote, thanks all for pitching in and helping with this) |
Sorry, something went wrong.
|
multiline does not work, but it never worked very well before (did not cope with the indents right and multi-line scroll back did not work). For multi-line stuff it is better to write it in another buffer and ship it over to the interpreter anyway. Completion seems to work better now. |
Sorry, something went wrong.
It is not a terminal emulator. It treats the subprocess as something that simply emits line-based text to stdout and reads line-based text back. The subprocess should not treat it as a terminal emulator, and should not send terminal commands. This is why you check $TERM before doing these sorts of things.
It's just a wrapper around standard Emacs' run-python, which is the default way of handling interactive programs from within Emacs (same thing for Scheme, Ruby, JavaScript, etc. REPLs). For many of those, Emacs adds features ontop of the underlying REPL. IPython is very featureful by itself, and I don't think it really matches the use case. You are right that using IPython within this environment does not add a lot ontop of standard CPython, but it seems quite a few users like it. There's an IPython/Jupyter notebook implementation for Emacs which might be more useful for some use cases. (Again, I do not use IPython at all, so I am just trying to answer questions by guesswork here :-)) |
Sorry, something went wrong.
As promised, I am surprised. "I wrote that code and I don't know how it's doing what you say it's doing" surprised ;-) Unless it was using the tk backend, as Python automatically integrates with the tkinter event loop without us needing to do anything. If it does what you want and you don't need our completions or multiline input, maybe we should make that a more official feature rather than an internal testing flag. |
Sorry, something went wrong.
|
Now that #9506 is "merged" I think we can get that in. @takluyver, need some help to rebase ? |
Sorry, something went wrong.
IPython now uses the prompt_toolkit based TerminalInteractiveShell instead, so this removes the readline code we're no longer using.
|
Rebased |
Sorry, something went wrong.
| InteractiveShellABC.register(TerminalInteractiveShell) | ||
| class TerminalInteractiveShell(PromptToolkitShell): | ||
| def __init__(self, *args, **kwargs): | ||
| warn("This is a deprecated alias for IPython.terminal.ptshell.TerminalInteractiveShell. " |
There was a problem hiding this comment.
Want to also warn on import ? Can be done in separate PR + since when it is deprecated.
Sorry, something went wrong.
|
Merged ! I'll open a small extra issue to improve deprecation warning and add them on import as well. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
IPython now uses the prompt_toolkit based TerminalInteractiveShell instead, so this removes the readline code we're no longer using.