bash_kernel icon indicating copy to clipboard operation
bash_kernel copied to clipboard

Fixes for modern readline

Open kdm9 opened this issue 2 years ago • 10 comments

This will automatically disable bracketed paste, which seems to break bash_kernel. I have verified that this fixes the problem with the latest released version of bash_kernel.

It should address issues described in #117, #116, & #115 (which are all the same AFAICT).

~~Additionally, I've added code to set the TERM_PROGRAM and TERM_PROGRAM_VERSION env vars so others can use this to perform customisation of bashrc as needed. This might not be ideal, as perhaps bashrc has executed before these are set. Please advise if that's the case.~~ (removed this)

To use this code:

python3 -m pip uninstall bash_kernel
python3 -m pip install -U git+https://github.com/kdm9/bash_kernel.git@patch-1

kdm9 avatar May 24 '22 18:05 kdm9

Quite likely fixes #119's problem, and also #107 too

kdm9 avatar May 24 '22 18:05 kdm9

I've tested this code in my environment but it freezes bash execution. Env details:

$ jupyter-lab --version
3.2.5
$ python --version
Python 3.9.5
$ uname -a
Linux my-ubuntu 5.13.0-41-generic #46-Ubuntu SMP Thu Apr 14 20:06:04 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
$ bash --version
GNU bash, version 5.1.8(1)-release (x86_64-pc-linux-gnu)

So it didn't help to fix #107 to me

hcz avatar Jun 02 '22 11:06 hcz

@hcz Thanks for testing. That's strange, as I get no hang here across a quite wide diversity of linuxes and bashes. does manually running bind 'set enable-bracketed-paste off' hang for you, either in Jupyter or outside it?

kdm9 avatar Jun 02 '22 19:06 kdm9

@kdm9 I've tried separately solution from the issue #107 : bind 'set enable-bracketed-paste off'

It worked.

hcz avatar Jun 03 '22 06:06 hcz

@hcz how about the latest commit? does that hang still? (run python3 -m pip install -U git+https://github.com/kdm9/bash_kernel.git@patch-1 again)

I removed the part about TERM_PROGRAM, so this now should be identical to manually running bind 'set enable-bracketed-paste off'.

kdm9 avatar Jun 03 '22 07:06 kdm9

@kdm9 thanks for the try, but unfortunately it doesn't hang and doesn't work

It looks like bind isn't applied immediately: Screenshot from 2022-06-03 17-47-58

hcz avatar Jun 03 '22 14:06 hcz

@hcz thanks for testing! Now I'm really confused :)

  1. Can you confirm that your venv/conda env's bash_kernel/kernel.py file has this line in it? (i.e there was no installation SNAFU that meant you got e.g. the main branch or the released version)
  2. Can you confirm that it no longer hangs without the line that sets TERM_PROGRAM? (in which case I'll keep the TERM_PROGRAM part excluded)

Unless anything obvious is wrong in 1. above, I'll have to defer to @takluyver as to why kernel.run_command("XXX") behaves differently from running the identical command in the first cell.

Best, Kevin

kdm9 avatar Jun 03 '22 15:06 kdm9

@kdm9

  1. There was no that line, really sorry. I missed the first necessary step: bash_kernel uninstall. I've tested your latest commits: a4c81c9f35926b7625b11e4c016c5e67edac522f and d6a048bbafdc994172787ec625c9e3937e19359b Both revisions worked really nice and fixed these problems:

    • ugly red alerts disappeared, exit codes became fine
    • bracketed input was disabled, additional symbol sequences disappeared
  2. I confirm. Looks like it was lags of my local environment

hcz avatar Jun 03 '22 18:06 hcz

@hcz Wonderful news! Thanks so much for testing, i'm glad it fixes it too. I'll amend the instructions above to ensure folks uninstall it if they want to test/use this version until @takluyver has time to review/merge.

kdm9 avatar Jun 03 '22 19:06 kdm9

@takluyver a friendly ping about this PR. I'm teaching a workshop using jupyter & bash_kernel soon, and I'd much prefer the instructions to be pip install bash_kernel than the hack above to install this fix from my dev branch. Could you please review/merge soon?

If you need help maintaining bash_kernel, I'd be happy to step in as a co-maintainer, even if temporarily until you have more time. If so, just add me to the repo.

kdm9 avatar Jul 06 '22 07:07 kdm9

As a cheap simple workaround, I found just unsetting my TERM when calling jupyter lab gets around the issue somehow:

TERM= jupyter lab ...

andyneff avatar Aug 18 '22 22:08 andyneff

Thanks @kdm9 et al for diagnosing and figuring out a fix for this, and sorry it's taken me so long to look at it.

I think the fix basically looks good, and I'll merge it. If I was being picky, I might suggest that this belongs 'upstream' in pexpect (this file tries to set bash up for being wrapped in a programmatic interface), but there's a working fix ready here, and I'd probably also need to review the Pexpect PR (though there are other maintainers there), so let's not hold this up for that.

Further in the future, I wonder if bash_kernel (or the pexpect replwrap API it uses) could make use of bracketed paste, rather than disabling it. Currently, if you have more than one line in a cell, bash_kernel sends them to bash one at a time, waiting for the prompt after each one. With bracketed paste, it should be possible to send the whole cell in one go. On the other hand, it probably doesn't simplify the code in replwrap unless we can assume that every REPL to be wrapped supports bracketed paste. :thinking:

takluyver avatar Aug 22 '22 10:08 takluyver

I've released version 0.8 with this change - hopefully that fixes a lot of the issues people have been seeing.

takluyver avatar Aug 22 '22 11:08 takluyver