-
Notifications
You must be signed in to change notification settings - Fork 143
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
Version 0.9.1 seems to have broken prompt handling in some cases #142
Comments
@thomasjm thanks for this report. Could you please provide a minimal example within whichever system you're launching the bash kernel (nix + codedown?), so I can dig in and debug? Using stock jupyter-lab + bash_kernel >=0.9.1 works fine for me, as does execution via papermill, all with bash v5.2, so I can't reproduce this yet sorry. |
Here you go, this reproduces it using the |
@thomasjm on a deadline this week, will take a look on the weekend -- feel free to annoy me if you don't hear from me by monday. best, |
Just checking in, would love to get a fix for this! |
Sorry, on a reporting deadline till later this week -- started debugging on the weekend but a fix will have to wait till later this week or the coming weekend |
Hello, just another ping on this! |
OK, so I've worked out that the issue is with handling the "invisible" characters in the prompt (the Thoughts, @takluyver? I'll continue poking later, but for now @thomasjm you (or the nix pkg maintainer) could patch out the I'm reticent to do that globally, as the code here seems to work everywhere except in nix. I'd prefer a solution that has the nice effect of avoiding breaking |
Note that those 'invisible' But I agree they seem relevant - for some reason, whether it's the changes in that PR or something else, they seem to be showing up in the displayed prompt, which stops it being found as a prompt. |
Thanks for looking into this @kdm9 and @takluyver ! I found a fix, which I've introduced in NixOS/nixpkgs#311507. The fix is to use Nixpkgs' Here's some documentation on the flag that controls this when building Bash. Hopefully this helps clarify the situations where the current prompt approach breaks down. Not sure if it's important to fix this case--for my part I'll be happy once the Nixpkgs PR is merged. |
OK, many thanks for the update -- I suspected it would be something like that! I will add a note to the README the we need proper readline support, but we should really detect if we have readline and only add those characters if we do. |
That makes sense, nice work figuring it out. 👍 |
Starting at 0.9.1, I can no longer start a kernel. I'm using Papermill 2.5.0 and Bash 5.2, both provided by Nixpkgs. I get error output like the below, which seems to involve the prompt regex.
The likely commit seems to be 9049a84
CC @kdm9
When I launch the Bash binary normally with the indicated
rcfile
, it seems fine (the prompt is just a single$
).EDIT: actually, it seems this may be because of a lack of this patch: #69. This PR was previously incorporated into Nixpkgs, but was removed when updating to 0.9.1. Not sure why, perhaps it didn't merge cleanly with 9049a84.
The text was updated successfully, but these errors were encountered: