bug#51010: 29.0.50; python completion breaks upon non-trivial rl config

bug#51010: 29.0.50; python completion breaks upon non-trivial rl config

[Carlos Pita]

(originally reported in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25753)

Using this readline config:

> cat .inputrc

set completion-ignore-case on
set completion-display-width 80
set completion-prefix-display-length 5
set show-all-if-ambiguous on
set skip-completed-text on
set colored-stats on
set blink-matching-paren on
set menu-complete-display-prefix on

native completions received by python.el look like:

    0__dummy_completion__ [0m [K  1__dummy_completion__ [0m [K

which wreaks havoc with the native completion mechanism.

This mechanism is quite fragile and already complex enough, I don't
feel like approaching the issue on a case by case basis and always
catching up with upstream and with configuration combinatorics.

One alternative is to set the INPUTRC environment variable in order to
reset the user config. Short of doing that, we could just make one
regex less strict:

                  (comint-redirect-finished-regexp
                   "1__dummy_completion__[[:space:]]*\n")

to:
                  (comint-redirect-finished-regexp
                   "1__dummy_completion__.*\n")

I propose this minimal change. A priori I'd expect that the text
"1__dummy_completion__" will always be in the output, no matter the
specific configuration, although the fact that python.el is already
doing `set show-all-if-ambiguous on` is a testimony to how fallible my
expectations are.

Best regards,
Carlos

bug#51010: 29.0.50; python completion breaks upon non-trivial rl config

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 849 bytes --]

Hi,

my apologies for the insistence, but please take a look a this, it’s a very simple change and although wrt python.el it just removes an annoying message ("Your ‘python-shell-interpreter’ doesn’t seem to support readline") and avoids disabling native completion altogether (which, anyway, as I’ve discussed elsewhere, I believe doesn’t add much in terms of features as it adds in terms of problems), wrt org-babel the problem is actually serious since trying to evaluate a python code block without first blacklisting python from native completion freezes emacs to death and seemingly a restart is needed. Of course, org-babel should be more robust in this regard but some help from python.el won’t hurt.

Best regards,
Carlos

PS: perhaps I should have titled this issue as “python completion freezes emacs to death” ;)

[-- Attachment #2: Type: text/html, Size: 1687 bytes --]

bug#51010: 29.0.50; python completion breaks upon non-trivial rl config

[Augusto Stoffel]

Which of the proposed solutions do you actually suggest?  I think they
are all quite small changes, so would you be able to provide a patch?

bug#51010: 29.0.50; python completion breaks upon non-trivial rl config

[Carlos Pita]

> On 10 Oct 2021, at 08:50, Augusto Stoffel <arstoffel@gmail.com> wrote:
> 
> Which of the proposed solutions do you actually suggest?

The minimal change: replacing [[:space:]]* with .*

> would you be able to provide a patch?

Yes, but I don’t have the necessary tooling at hand ATM, so it may take a couple of days.

bug#51010: 29.0.50; python completion breaks upon non-trivial rl config

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 27 bytes --]

Tags: patch

There you go.

[-- Attachment #2: 0001-Match-dummy-output-even-with-non-default-rl-config.patch --]
[-- Type: application/octet-stream, Size: 1189 bytes --]

From c6df93f051ea0d4934a100c925fc6f00aa02fc2a Mon Sep 17 00:00:00 2001
From: Carlos Pita <carlosjosepita@gmail.com>
Date: Wed, 13 Oct 2021 21:09:40 -0300
Subject: [PATCH] Match dummy output even with non default rl config

* lisp/progmodes/python.el
(python-shell-completion-native-get-completions): Match dummy output
even when readline is configured in non default ways.
---
 lisp/progmodes/python.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index b1f61c89a4..203531018e 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -3811,7 +3811,7 @@ python-shell-completion-native-get-completions
                   (comint-redirect-perform-sanity-check nil)
                   (comint-redirect-insert-matching-regexp t)
                   (comint-redirect-finished-regexp
-                   "1__dummy_completion__[[:space:]]*\n")
+                   "1__dummy_completion__.*\n")
                   (comint-redirect-output-buffer redirect-buffer))
               ;; Compatibility with Emacs 24.x.  Comint changed and
               ;; now `comint-redirect-filter' gets 3 args.  This
-- 
2.33.0

bug#51010: 29.0.50; python completion breaks upon non-trivial rl config

[Carlos Pita]

Does the "Tags: patch" magic header work or should I use the control
interface? It has been some time since the last time I did this...

bug#51010: 29.0.50; python completion breaks upon non-trivial rl config

[Eli Zaretskii]

> From: Carlos Pita <carlosjosepita@gmail.com>
> Date: Wed, 13 Oct 2021 21:50:32 -0300
> Cc: 51010@debbugs.gnu.org
> 
> Does the "Tags: patch" magic header work or should I use the control
> interface? It has been some time since the last time I did this...

AFAIU, it only works in the message that creates a new bug report.
See admin/notes/bugtracker for more info.

bug#51010: 29.0.50; python completion breaks upon non-trivial rl config

[Carlos Pita]

> AFAIU, it only works in the message that creates a new bug report.
> See admin/notes/bugtracker for more info.

Ok, I've tagged the issue through the control interface. One more
question related to debbugs if you don't mind: if I reply to
nnn@debbugs.gnu.org as reported in the ack mail but leave the subject
empty, will that be a nuisance for you? If it will, what is the best
way to follow-up an issue for which I haven't yet received any answer?
Copy&paste and edit the subject? Reply to myself even if the address
is bug-gnu-emacs@gnu.org instead of nnn@gnu.org? (I believe that
having #nnn in the subject is enough). Thank you.

bug#51010: 29.0.50; python completion breaks upon non-trivial rl config

[Eli Zaretskii]

> From: Carlos Pita <carlosjosepita@gmail.com>
> Date: Thu, 14 Oct 2021 22:15:01 -0300
> Cc: 51010@debbugs.gnu.org
> 
> > AFAIU, it only works in the message that creates a new bug report.
> > See admin/notes/bugtracker for more info.
> 
> Ok, I've tagged the issue through the control interface. One more
> question related to debbugs if you don't mind: if I reply to
> nnn@debbugs.gnu.org as reported in the ack mail but leave the subject
> empty, will that be a nuisance for you?

Yes, it will.  Subjects are very important, at least for me, because
they are the way I look for related messages when I work on an issue.

> If it will, what is the best way to follow-up an issue for which I
> haven't yet received any answer?

Go to the bug via the Web interface and copy the Subject from there.

bug#51010: 29.0.50; python completion breaks upon non-trivial rl config

[Carlos Pita]

> > Ok, I've tagged the issue through the control interface. One more
> > question related to debbugs if you don't mind: if I reply to
> > nnn@debbugs.gnu.org as reported in the ack mail but leave the subject
> > empty, will that be a nuisance for you?
>
> Yes, it will.  Subjects are very important, at least for me, because
> they are the way I look for related messages when I work on an issue.

I've recently read this it in the CONTRIBUTE document that connects
both subjects above:

    If a message to the bug tracker contains a patch, please
    include the string "[PATCH]" in the subject of the message in order to
    let the bug tracker tag the bug properly.

(I was under the impression that the patch tag was the official way of
doing it.)

Now, supposing that the patch is an addendum to an ongoing thread,
wouldn't that "[PATCH]" marker mess with the organization of your
emails? Or is it automatically removed from the subject? Or do all
maintainers already have rules in place to deal with such variations?

Best regards,
Carlos

bug#51010: 29.0.50; python completion breaks upon non-trivial rl config

[Eli Zaretskii]

> From: Carlos Pita <carlosjosepita@gmail.com>
> Date: Tue, 19 Oct 2021 20:20:08 -0300
> Cc: 51010@debbugs.gnu.org
> 
>     If a message to the bug tracker contains a patch, please
>     include the string "[PATCH]" in the subject of the message in order to
>     let the bug tracker tag the bug properly.
> 
> (I was under the impression that the patch tag was the official way of
> doing it.)
> 
> Now, supposing that the patch is an addendum to an ongoing thread,
> wouldn't that "[PATCH]" marker mess with the organization of your
> emails?

Yes, depending on the MUA.  Although it's not a catastrophe.

So please use that only when the reason for bug report is to suggest a
patch.

bug#51010: 29.0.50; python completion breaks upon non-trivial rl config

[Stefan Kangas]

Carlos Pita <carlosjosepita@gmail.com> writes:

> One alternative is to set the INPUTRC environment variable in order to
> reset the user config. Short of doing that, we could just make one
> regex less strict:
>
>                   (comint-redirect-finished-regexp
>                    "1__dummy_completion__[[:space:]]*\n")
>
> to:
>                   (comint-redirect-finished-regexp
>                    "1__dummy_completion__.*\n")
>
> I propose this minimal change. A priori I'd expect that the text
> "1__dummy_completion__" will always be in the output, no matter the
> specific configuration, although the fact that python.el is already
> doing `set show-all-if-ambiguous on` is a testimony to how fallible my
> expectations are.

Your below patch LGTM, but I would like confirmation from Eli or Lars
if they think it is safe enough for emacs-28, or if they prefer that it
should go to master.

Carlos Pita <carlosjosepita@gmail.com> writes:

> From c6df93f051ea0d4934a100c925fc6f00aa02fc2a Mon Sep 17 00:00:00 2001
> From: Carlos Pita <carlosjosepita@gmail.com>
> Date: Wed, 13 Oct 2021 21:09:40 -0300
> Subject: [PATCH] Match dummy output even with non default rl config
>
> * lisp/progmodes/python.el
> (python-shell-completion-native-get-completions): Match dummy output
> even when readline is configured in non default ways.
> ---
>  lisp/progmodes/python.el | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
> index b1f61c89a4..203531018e 100644
> --- a/lisp/progmodes/python.el
> +++ b/lisp/progmodes/python.el
> @@ -3811,7 +3811,7 @@ python-shell-completion-native-get-completions
>                    (comint-redirect-perform-sanity-check nil)
>                    (comint-redirect-insert-matching-regexp t)
>                    (comint-redirect-finished-regexp
> -                   "1__dummy_completion__[[:space:]]*\n")
> +                   "1__dummy_completion__.*\n")
>                    (comint-redirect-output-buffer redirect-buffer))
>                ;; Compatibility with Emacs 24.x.  Comint changed and
>                ;; now `comint-redirect-filter' gets 3 args.  This

bug#51010: 29.0.50; python completion breaks upon non-trivial rl config

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sat, 30 Oct 2021 09:09:24 -0700
> Cc: 51010@debbugs.gnu.org, Augusto Stoffel <arstoffel@gmail.com>
> 
> Your below patch LGTM, but I would like confirmation from Eli or Lars
> if they think it is safe enough for emacs-28, or if they prefer that it
> should go to master.

I don't know enough to decide whether this change is safe enough.

Is this a recent problem, or did we always behave like that?  If the
latter, I prefer to install this on master.

Thanks.

bug#51010: 29.0.50; python completion breaks upon non-trivial rl config

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> Is this a recent problem, or did we always behave like that?  If the
> latter, I prefer to install this on master.

That makes sense.  I hope that Carlos can answer your question.

bug#51010: 29.0.50; python completion breaks upon non-trivial rl config

[Carlos Pita]

>> Is this a recent problem, or did we always behave like that?

> I hope that Carlos can answer your question.

I'm pretty sure python.el has behaved the same for a long time in this
regard, but readline has been gaining some nifty features over the
years and so my .inputrc.

Best regards,
Carlos

bug#51010: 29.0.50; python completion breaks upon non-trivial rl config

[Stefan Kangas]

close 51010 29.1
thanks

Carlos Pita <carlosjosepita@gmail.com> writes:

>>> Is this a recent problem, or did we always behave like that?
>
>> I hope that Carlos can answer your question.
>
> I'm pretty sure python.el has behaved the same for a long time in this
> regard, but readline has been gaining some nifty features over the
> years and so my .inputrc.

OK, let's play it safe and install it on master.  With python.el, we can
release an updated package on GNU ELPA, so it is not critical that this
is in Emacs 28.1.

Pushed to master (commit a0973fa7a6).