Skip to content
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

prompt_toolkit 3.0.25+ breaks Ctrl-C #33428

Closed
tornaria opened this issue Feb 28, 2022 · 18 comments
Closed

prompt_toolkit 3.0.25+ breaks Ctrl-C #33428

tornaria opened this issue Feb 28, 2022 · 18 comments

Comments

@tornaria
Copy link
Contributor

For the record, Ctrl-C is broken starting with version 3.0.25 of prompt_toolkit (current version 3.0.28 still broken, sage-the-distribution bundles 3.0.22 so it's ok).

$ xbps-query -s python3-prompt_toolkit
[*] python3-prompt_toolkit-3.0.28_1 Python3 library for building powerful interactive command lines
$ sage
┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 9.5, Release Date: 2022-01-30                     │
│ Using Python 3.10.2. Type "help()" for help.                       │
└────────────────────────────────────────────────────────────────────┘
sage: factor(54853908712446157179434453567343931596301333824416758692208077566613224454501131209)
^C^C^C^C

If one waits the half hour or so that it takes to factor the number above, then the ^C gets through, but waiting kind of defeats the purpose of hitting ^C. This is not specific to factoring -- anything that depends on cysignals for Ctrl-C working is broken.

Upstream issue: prompt-toolkit/python-prompt-toolkit#1576

In the GH issue there are two different fixes suggested (with patches), IMHO the second one is better for sage (the downside is that introduces a dependency on cysignals, but sage already depends on cysignals).

No action is needed now, but one of the patches will be necessary if/when prompt_toolkit is updated, or for distro packages on systems with a recent version of prompt_toolkit (void linux issue and discussion in void-linux/void-packages#35730)

Upstream: Reported upstream. No feedback yet.

CC: @antonio-rojas @orlitzky @jhpalmieri

Component: packages: standard

Keywords: prompt_toolkit

Author: Matthias Koeppe

Branch/Commit: bd1bc04

Reviewer: John Palmieri

Issue created by migration from https://trac.sagemath.org/ticket/33428

@slel

This comment has been minimized.

@mkoeppe
Copy link
Member

mkoeppe commented Feb 28, 2022

comment:2

Let's put in an upper version bound in build/pkgs/prompt_toolkit/install-requires.txt to document this

@mkoeppe
Copy link
Member

mkoeppe commented Mar 3, 2022

@mkoeppe
Copy link
Member

mkoeppe commented Mar 3, 2022

Commit: bd1bc04

@mkoeppe
Copy link
Member

mkoeppe commented Mar 3, 2022

New commits:

bd1bc04build/pkgs/prompt_toolkit/install-requires.txt: Reject versions 3.0.25+

@mkoeppe
Copy link
Member

mkoeppe commented Mar 3, 2022

Author: Matthias Koeppe

@mkoeppe
Copy link
Member

mkoeppe commented Apr 30, 2022

comment:5

Upstream has merged prompt-toolkit/python-prompt-toolkit#1582 - do we know if it fixes the issue?

@tornaria
Copy link
Contributor Author

tornaria commented May 1, 2022

comment:6

Replying to @mkoeppe:

Upstream has merged prompt-toolkit/python-prompt-toolkit#1582 - do we know if it fixes the issue?

I don't think so, we are patching 3.0.29 in void linux, see here:
https://github.com/void-linux/void-packages/blob/master/srcpkgs/python3-prompt_toolkit/patches/dont-handle-sigint.patch

@jhpalmieri
Copy link
Member

comment:8

Confirmed: I just downloaded and installed 3.0.30 (OS X) and it does not seem to have fixed the interrupt problems.

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@jhpalmieri
Copy link
Member

comment:9

Is this going to cause problems with the IPython upgrade (#33530)? That branch uses version 3.0.29.

@mkoeppe
Copy link
Member

mkoeppe commented Jul 16, 2022

comment:10

Thanks for catching this.

@vbraun
Copy link
Member

vbraun commented Jul 18, 2022

comment:11

Merge failure on top of:

1c98f0a Trac #31563: Upgrade giac to 1.9.0-15

83a3df1 Trac #34131: some doc polishing in symbolic

dee5229 Trac #34130: some doc polishing in groups

fd80fa1 Trac #34126: some doc polishing in categories

718d356 Trac #34125: some doc polishing in modular

49390d8 Trac #34124: some doc polishing in combinat

af94c25 Trac #34096: Fix typo: enviroment -> environment

d04c189 Trac #34050: add some space around == in paths and rational

1634bb6 Trac #34047: Typo in _repr_Expression in symbolic/expression.pyx

dc8086f Trac #34154: Fix repeated word typos

17b1c17 Trac #34148: fix and activate pycodestyle E306

785bdbd Trac #34136: modernize super() in rings (part one)

9882001 Trac #34129: Dummy packages should write proper log files

dbc6b9e Trac #34109: pep8 for sagedoc.py

5e6a331 Trac #34103: randomize infinite recursion

17ba76f Trac #34093: Wrong evaluation of elements of CBF[x] on polynomials from other rings

4657052 Trac #34122: Bug in is_planar() method for directed graphs

c9cfb53 Trac #34087: inverse reciprocal trig functions not (back)translated to/from Mathematica

1d6d055 Trac #34076: pycodestyle cleanup in src/sage/graphs/genus.pyx

043d862 Trac #34071: pycodestyle cleanup in asteroidal_triples.pyx, chrompoly.pyx, cliquer.pyx, convexity_properties.pyx

b9b25dc Trac #34070: pycodestyle cleanup in src/sage/graphs/centrality.pyx

4ef2c65 Trac #34069: pycodestyle cleanup in src/sage/graphs/comparability.pyx

5b11467 Trac #34066: Tachyon help message

798adaa Trac #34065: pycodestyle cleanup in src/sage/graphs/bliss.pyx

ce62be2 Trac #34063: pycodestyle cleanup in src/sage/graphs/base/

a0eadb3 Trac #34059: Fix trivial case in conversion of list to number field element

fea0ac5 Trac #34057: changes about avoiding double dieses

9eefd5c Trac #34145: modernize super in geometry/

01e117d Trac #34137: modernize super in categories/

6c79334 Trac #33819: build.yml: In workflow_dispatch, make container and base tag selectable; add doc

6a64ab8 Trac #33760: do not update _pos, _pos3d, _embedding on vertex/edge deletions

dbf091d Trac #34139: fix the linter

625ac58 Updated SageMath version to 9.7.beta5

ticket milestone is not intended to be merged:

@vbraun
Copy link
Member

vbraun commented Jul 28, 2022

Changed branch from u/mkoeppe/prompt_toolkit_3_0_25__breaks_ctrl_c to bd1bc04

@mkoeppe
Copy link
Member

mkoeppe commented Mar 9, 2023

@tornaria Do you know if there's a new prompt-toolkit version that does not need to be patched?
IPython 8.11 wants a prompt-toolkit >= 3.0.30

@tornaria
Copy link
Contributor Author

tornaria commented Mar 9, 2023

@tornaria Do you know if there's a new prompt-toolkit version that does not need to be patched? IPython 8.11 wants a prompt-toolkit >= 3.0.30

We are shipping 3.0.38 in void linux, and I'm still patching it (see https://github.com/void-linux/void-packages/blob/master/srcpkgs/python3-prompt_toolkit/patches/dont-handle-sigint.patch)

But I haven't tested it without the patch so... maybe something else changed somewhere (in ipython?). My suggestion would be to upgrade ipython to 8.11 and prompt-toolkit to 3.0.38, and if the bug is still present you can apply the patch I linked above.

@mkoeppe
Copy link
Member

mkoeppe commented Mar 9, 2023

Thanks. I've just tested it; still broken (and prompt-toolkit/python-prompt-toolkit#1576 is still open)

@mkoeppe mkoeppe mentioned this issue Mar 9, 2023
4 tasks
@tornaria
Copy link
Contributor Author

tornaria commented Mar 9, 2023

@mkoeppe Here is a possible workaround ( "if the mountain won't come to sagemath..." )

What do you think?

Note that the patch I linked above is a different workaround which is not so good (completely disabling the sigint handler in prompt_toolkit carries other problems: they added this handler in 3.0.25 for a reason, see prompt-toolkit/python-prompt-toolkit#1538).

--- a/src/sage/repl/interpreter.py
+++ b/src/sage/repl/interpreter.py
@@ -153,6 +153,8 @@ from IPython.terminal.embed import InteractiveShellEmbed
 from IPython.terminal.ipapp import TerminalIPythonApp, IPAppCrashHandler
 from IPython.core.crashhandler import CrashHandler
 
+from cysignals import pysignals
+import signal
 
 # TODO: This global variable _do_preparse should be associated with an
 # IPython InteractiveShell as opposed to a global variable in this
@@ -287,6 +289,13 @@ class SageTerminalInteractiveShell(SageShellOverride, TerminalInteractiveShell):
         backend = BackendIPythonCommandline()
         backend.get_display_manager().switch_backend(backend, shell=self)
 
+    def prompt_for_code(self):
+        _sigint = signal.getsignal(signal.SIGINT)
+        _sigint_os = pysignals.getossignal(signal.SIGINT)
+        text = TerminalInteractiveShell.prompt_for_code(self)
+        pysignals.setsignal(signal.SIGINT, _sigint, _sigint_os)
+        return text
+
 
 class SageTestShell(SageShellOverride, TerminalInteractiveShell):
     """

orlitzky added a commit to orlitzky/sage that referenced this issue Aug 26, 2023
Sage isn't ready for ipython-8.1.x yet because it requires a newer
prompt_toolkit (sagemath#33428).
orlitzky added a commit to orlitzky/sage that referenced this issue Aug 26, 2023
Sage isn't ready for ipython-8.11 yet because it requires a newer
prompt_toolkit (sagemath#33428).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants