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

Inspect continuation prompt signature and pass only viable arguments. #14274

Merged
merged 3 commits into from Jan 8, 2024

Conversation

Carreau
Copy link
Member

@Carreau Carreau commented Dec 27, 2023

Closes #14273

@@ -34,7 +34,7 @@ which defines the defaults. The required interface is like this:
:class:`~.TerminalInteractiveShell` instance.

.. method:: in_prompt_tokens(cli=None)
continuation_prompt_tokens(self, cli=None, width=None)
continuation_prompt_tokens(self, width=None)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While you are at it, maybe remove cli=None from the previous method, and the two lines 46-47 below.

@tornaria
Copy link

LGTM, I actually rebuilt ipython with this PR and tried running sagemath to make sure.

My suggestion above in patch form:

--- a/docs/source/config/details.rst
+++ b/docs/source/config/details.rst
@@ -33,7 +33,7 @@ which defines the defaults. The required interface is like this:
    Prompt style definition. *shell* is a reference to the
    :class:`~.TerminalInteractiveShell` instance.
 
-   .. method:: in_prompt_tokens(cli=None)
+   .. method:: in_prompt_tokens()
                continuation_prompt_tokens(self, width=None)
                rewrite_prompt_tokens()
                out_prompt_tokens()
@@ -43,9 +43,6 @@ which defines the defaults. The required interface is like this:
       For continuation prompts, *width* is an integer representing the width of
       the prompt area in terminal columns.
 
-      *cli*, where used, is the prompt_toolkit ``CommandLineInterface`` instance.
-      This is mainly for compatibility with the API prompt_toolkit expects.
-
 Here is an example Prompt class that will show the current working directory
 in the input prompt:
 
@@ -55,7 +52,7 @@ in the input prompt:
     import os
 
     class MyPrompt(Prompts):
-         def in_prompt_tokens(self, cli=None):
+         def in_prompt_tokens(self):
              return [(Token, os.getcwd()),
                      (Token.Prompt, ' >>>')]
 

Maybe you also want to patch out the examples:

--- a/examples/Embedding/embed_class_long.py
+++ b/examples/Embedding/embed_class_long.py
@@ -21,7 +21,7 @@
 
 class CustomPrompt(Prompts):
 
-    def in_prompt_tokens(self, cli=None):
+    def in_prompt_tokens(self):
 
        return [
             (Token.Prompt, 'In <'),
--- a/examples/utils/cwd_prompt.py
+++ b/examples/utils/cwd_prompt.py
@@ -6,7 +6,7 @@
 
 class MyPrompt(Prompts):
 
-     def in_prompt_tokens(self, cli=None):
+     def in_prompt_tokens(self):
          return [(Token, os.getcwd()),
                  (Token.Prompt, '>>>')]
 

@Carreau Carreau added this to the 8.20 milestone Jan 8, 2024
@Carreau Carreau merged commit 59eccb2 into ipython:main Jan 8, 2024
20 of 21 checks passed
@Carreau Carreau deleted the fix-prompt branch January 8, 2024 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix prompt backward compat.
2 participants