takluyver / bash_kernel

A bash kernel for IPython
BSD 3-Clause "New" or "Revised" License
692 stars 144 forks source link

silent failure in case of unbalanced quotes #141

Open abbbe opened 5 months ago

abbbe commented 5 months ago

The following screenshot illustrates the issue:

image

When this notebook is executed, cell#3 remains marked '[*]' while in reality it has failed silently and execution of the next cells can continue. Expected behavior - execution of cell#3 should fail with some kind of an error message.

kdm9 commented 5 months ago

ah, yes, this is basically the same root cause as #132, except we don't have a "you wrote half a string" error message yet. I must admit I am very busy at the minute so won't get a fix out very soon, but pull requests are welcome. However, it will be reasonably tricky to fix, given the bidirectional nesting of quotes, and also heredocs: these are valid: '"', "'", but these aren't: ""', "'' etc, let alone quoting, heredocs or other features of bash that allow further nesting not supported by shlex.

abbbe commented 5 months ago

I have tested the scenario I have countered and #132. Both seem to use PS2 prompt.

On my MacOS:

bash-3.2$ bash --version
GNU bash, version 3.2.57(1)-release (arm64-apple-darwin23)
Copyright (C) 2007 Free Software Foundation, Inc.
bash-3.2$ echo '
> ^C
bash-3.2$ echo "
> ^C
bash-3.2$ echo \
> ^C
bash-3.2$ set | grep ^PS2
PS2='> '

On Debian-based system:

root@elon33:~# bash --version
GNU bash, version 5.2.15(1)-release (x86_64-pc-linux-gnu)
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
root@elon33:~# echo '
> ^C
root@elon33:~# echo "
> ^C
root@elon33:~# echo \
> ^C
root@elon33:~# set | grep ^PS2
PS2='> '

If you ask me, a correct way to handle this situation is to treat appearance of PS2 as a fault. This is assuming you agree with the statement that in bash_kernel a shell command must fit inside of a single cell.

kdm9 commented 5 months ago

ah, of course, brilliant suggestion @abbbe! that should be pretty easy to detect, you're right.

kdm9 commented 4 months ago

This is actually a duplicate of #118

abbbe commented 4 months ago

Seems to be, yes.

Just as a side node — I can’t shake the feeling all the fixes we come up with will be dirty hacks (of varying degree of dirtiness) until bash adds proper support for REPLs. Like use “\0” instead of “\n” to indicate the end of the command.