bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Augusto Stoffel]

If I start a Python shell on Emacs -Q and type, say

```
x = []
x.c<tab>
```

then I see, as expected,

```
Possible completions are:
x.clear
x.copy
x.count
```

Now, if I (setq completion-styles '(flex)), then no completions as
reported in the same situation.  Same thing with the `orderless' or
`substring' completion styles.

Moreover, the same observation holds for native completion on or off.

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Augusto Stoffel]

On Tue,  7 Sep 2021 at 19:52, Augusto Stoffel <arstoffel@gmail.com> wrote:

> If I start a Python shell on Emacs -Q and type, say
>
> ```
> x = []
> x.c<tab>
> ```
>
> then I see, as expected,
>
> ```
> Possible completions are:
> x.clear
> x.copy
> x.count
> ```
>
> Now, if I (setq completion-styles '(flex)), then no completions as
> reported in the same situation.  Same thing with the `orderless' or
> `substring' completion styles.
>
> Moreover, the same observation holds for native completion on or off.

On closer inspection (and discussion with the orderless people), the
completion-at-point function in pyhton.el conforms to the completion API.

The issue lies in the fancier completion styles, which always send the
empty string as the STRING argument to any completion table.

There's a workaround for this, which I could provide, but this doesn't
seem advisable.

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Michael Albinus]

Augusto Stoffel <arstoffel@gmail.com> writes:

Hi Augusto,

> On closer inspection (and discussion with the orderless people), the
> completion-at-point function in pyhton.el conforms to the completion API.
>
> The issue lies in the fancier completion styles, which always send the
> empty string as the STRING argument to any completion table.

FTR, a similar issue has been discussed recently for remote file names
in bug#50387.

Best regards, Michael.

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Gregory Heytings]

>
> Now, if I (setq completion-styles '(flex)), then no completions as 
> reported in the same situation.  Same thing with the `orderless' or 
> `substring' completion styles.
>

You shouldn't use (setq completion-styles '(flex)), you should use 
(add-to-list 'completion-styles 'flex).  Otherwise the default completion 
styles are not used anymore.

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Gregory Heytings]

>> The issue lies in the fancier completion styles, which always send the 
>> empty string as the STRING argument to any completion table.
>
> FTR, a similar issue has been discussed recently for remote file names 
> in bug#50387.
>

Absolutely not.  With (add-to-list 'completion-styles 'substring) or 
(add-to-list 'completion-styles 'flex), completion works as expected in 
Python shells, and not in Tramp.

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Augusto Stoffel]

On Thu,  9 Sep 2021 at 07:30, Gregory Heytings <gregory@heytings.org> wrote:

> You shouldn't use (setq completion-styles '(flex)), you should use
> (add-to-list 'completion-styles 'flex).  Otherwise the default
> completion styles are not used anymore.

This doesn't change the issue described in the subject line.  I would
still not see any flex completions if I did what you suggest.

Granted, completion wouldn't be totally broken.  But I don't mind
letting the brokenness manifest itself.  Therefore I use
(setq completion-styles '(orderless)).

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Gregory Heytings]

>> You shouldn't use (setq completion-styles '(flex)), you should use 
>> (add-to-list 'completion-styles 'flex).  Otherwise the default 
>> completion styles are not used anymore.
>
> This doesn't change the issue described in the subject line.  I would 
> still not see any flex completions if I did what you suggest.
>

Because the flex completion mechanism returns no completions, and the next 
completion mechanism is called.  What kind of flex completions would you 
expect to see after x.t TAB in your example?

>
> Granted, completion wouldn't be totally broken.  But I don't mind 
> letting the brokenness manifest itself.  Therefore I use (setq 
> completion-styles '(orderless)).
>

It's not broken, it works as designed.  Instead of asking each user of the 
completion mechanism to implement a specific function for substring / flex 
completion, these mechanisms tell them "please return all possible 
completions, I'll do the filtering job for you".

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Augusto Stoffel]

On Thu,  9 Sep 2021 at 07:49, Gregory Heytings <gregory@heytings.org> wrote:

>>> You shouldn't use (setq completion-styles '(flex)), you should use
>>> (add-to-list 'completion-styles 'flex).  Otherwise the default
>>> completion styles are not used anymore.
>>
>> This doesn't change the issue described in the subject line.  I
>> would still not see any flex completions if I did what you suggest.
>>
>
> Because the flex completion mechanism returns no completions, and the
> next completion mechanism is called.  What kind of flex completions
> would you expect to see after x.t TAB in your example?

For sure 'x.count' should be a candidate, just like in Elisp 'set' shows
up as a possible completion after typing '(t TAB'.

Whether or not your example should allow 'fix.it' as a completion is up
to debate.

>
>>
>> Granted, completion wouldn't be totally broken.  But I don't mind
>> letting the brokenness manifest itself.  Therefore I use (setq
>> completion-styles '(orderless)).
>>
>
> It's not broken, it works as designed.  Instead of asking each user of
> the completion mechanism to implement a specific function for
> substring / flex completion, these mechanisms tell them "please return
> all possible completions, I'll do the filtering job for you".

Right, this is not a problem with the flex style, it's a problem with
the Python completion table.  More specifically, with its notion of "all
completions".

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Gregory Heytings]

>> It's not broken, it works as designed.  Instead of asking each user of 
>> the completion mechanism to implement a specific function for substring 
>> / flex completion, these mechanisms tell them "please return all 
>> possible completions, I'll do the filtering job for you".
>
> Right, this is not a problem with the flex style, it's a problem with 
> the Python completion table.  More specifically, with its notion of "all 
> completions".
>

Exactly.

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Augusto Stoffel]

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

After more discussions elsewhere [1], I noticed another problem with
the dynamic completion table from python.el.

Each call to the completion table requires communication with the
inferior process.  If called frequently enough, the comint input
filters can get confused, and some garbage gets printed to the shell.
If the inferior is running over Tramp, the problem gets amplified.

Now, this is not likely to cause much trouble when calling
`completion-at-point' manually, but it is a problem with any of
continuously-updating completion UIs like Company or Corfu.

To alleviate this, the completion-at-point function could implement some
sort of caching.  The difficult question is when to invalidate the
cache.  I've attached one possiblility as a draft patch.  If the
approach seems reasonable, then I'll format it properly.

As a side effect, the patch also solves the original issue described in
this ticket.

Please let me know what you think.

[1] https://github.com/oantolin/orderless/issues/79


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: python-capf.diff --]
[-- Type: text/x-patch, Size: 1471 bytes --]

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 4f222b4cf5..3239722981 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -3841,6 +3841,8 @@ python-shell-completion-get-completions
         (split-string completions
                       "^'\\|^\"\\|;\\|'$\\|\"$" t)))))
 
+(defvar-local python-shell--capf-cache nil)
+
 (defun python-shell-completion-at-point (&optional process)
   "Function for `completion-at-point-functions' in `inferior-python-mode'.
 Optional argument PROCESS forces completions to be retrieved
@@ -3895,11 +3897,13 @@ python-shell-completion-at-point
                        #'ignore
                      #'python-shell-completion-get-completions))
                   (t #'python-shell-completion-native-get-completions)))))
-    (list start end
-          (completion-table-dynamic
-           (apply-partially
-            completion-fn
-            process import-statement)))))
+    (let  ((re (or (car python-shell--capf-cache) regexp-unmatchable))
+           (prefix (buffer-substring-no-properties start end)))
+      (unless (string-match re prefix)
+        (setq python-shell--capf-cache
+              (cons (concat "\\`" (regexp-quote prefix) "\\(?:\\sw\\|\\s_\\)*\\'")
+                    (funcall completion-fn process import-statement prefix)))))
+    (list start end (cdr python-shell--capf-cache))))
 
 (define-obsolete-function-alias
   'python-shell-completion-complete-at-point

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Lars Ingebrigtsen]

Augusto Stoffel <arstoffel@gmail.com> writes:

> To alleviate this, the completion-at-point function could implement some
> sort of caching.  The difficult question is when to invalidate the
> cache.  I've attached one possiblility as a draft patch.  If the
> approach seems reasonable, then I'll format it properly.

Would it be possible to do caching at a lower level instead of in
python-mode?

> As a side effect, the patch also solves the original issue described in
> this ticket.

[...]

> -    (list start end
> -          (completion-table-dynamic
> -           (apply-partially
> -            completion-fn
> -            process import-statement)))))
> +    (let  ((re (or (car python-shell--capf-cache) regexp-unmatchable))
> +           (prefix (buffer-substring-no-properties start end)))
> +      (unless (string-match re prefix)
> +        (setq python-shell--capf-cache
> +              (cons (concat "\\`" (regexp-quote prefix) "\\(?:\\sw\\|\\s_\\)*\\'")
> +                    (funcall completion-fn process import-statement prefix)))))
> +    (list start end (cdr python-shell--capf-cache))))

I'm not sure I understand this patch -- it's not using
`completion-table-dynamic' at all now?  (But my understanding of the
completion functions in Emacs is pretty lacking.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Augusto Stoffel]

On Fri, 10 Sep 2021 at 13:37, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Augusto Stoffel <arstoffel@gmail.com> writes:
>
>> To alleviate this, the completion-at-point function could implement some
>> sort of caching.  The difficult question is when to invalidate the
>> cache.  I've attached one possiblility as a draft patch.  If the
>> approach seems reasonable, then I'll format it properly.
>
> Would it be possible to do caching at a lower level instead of in
> python-mode?

I'm not an expert in this either, but I think the caching mechanism
would be pretty particular to the circumstances of each completion
table, so it indeed belongs here.  Maybe João can say more?

Since this comint-based completion is not super smart (it's not
context-dependent at all), the rather naive caching invalidation used
here seems sufficient to me.

>
>> As a side effect, the patch also solves the original issue described in
>> this ticket.
>
> [...]
>
>> -    (list start end
>> -          (completion-table-dynamic
>> -           (apply-partially
>> -            completion-fn
>> -            process import-statement)))))
>> +    (let  ((re (or (car python-shell--capf-cache) regexp-unmatchable))
>> +           (prefix (buffer-substring-no-properties start end)))
>> +      (unless (string-match re prefix)
>> +        (setq python-shell--capf-cache
>> + (cons (concat "\\`" (regexp-quote prefix)
>> "\\(?:\\sw\\|\\s_\\)*\\'")
>> + (funcall completion-fn process import-statement prefix)))))
>> +    (list start end (cdr python-shell--capf-cache))))
>
> I'm not sure I understand this patch -- it's not using
> `completion-table-dynamic' at all now?  (But my understanding of the
> completion functions in Emacs is pretty lacking.)

That's true: the logic here is that the completions are computed
eagerly, and then cached until still valid.  So if you type

  fo<tab>o.bar.baz

the inferior process is contacted 3 times: after the <tab>, and after
each dot.  Before, a lazy table was returned, but the inferior would be
contacted after each character anyway (if using Company or similar).

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[João Távora]

Augusto Stoffel <arstoffel@gmail.com> writes:

> On Fri, 10 Sep 2021 at 13:37, Lars Ingebrigtsen <larsi@gnus.org> wrote:
>
>> Augusto Stoffel <arstoffel@gmail.com> writes:
>>
>>> To alleviate this, the completion-at-point function could implement some
>>> sort of caching.  The difficult question is when to invalidate the
>>> cache.  I've attached one possiblility as a draft patch.  If the
>>> approach seems reasonable, then I'll format it properly.
>>
>> Would it be possible to do caching at a lower level instead of in
>> python-mode?
>
> I'm not an expert in this either, but I think the caching mechanism
> would be pretty particular to the circumstances of each completion
> table, so it indeed belongs here.  Maybe João can say more?

I'm sorry, I'm quite overloaded lately and can't read this long thread.
I've been participating in a few discussions about this and all I can
add here are generic comments, like the intepretation that I make from
the docstring of completion-at-point-functions

    Special hook to find the completion table for the entity at point.
    Each function on this hook is called in turn without any argument and
    should return either nil, meaning it is not applicable at point,
    or a function of no arguments to perform completion (discouraged),
    or a list of the form (START END COLLECTION . PROPS), where:
     START and END delimit the entity to complete and should include point,
                           ^^^^^^^^^^^^^^^^^^^^^^     
     COLLECTION is the completion table to use to complete the entity, and
                                                  ^^^^^^^^^^^^^^^^^^^
     PROPS is a property list for additional information.

As I've underlined, it's _that_ entity, not some other entity that the
"backend" aka "capf" should complete.  So:

a) while the entity is "the same" (START and END are the same and POINT
   is somewhere in between) then the backend can do caching inside
   COLLECTION (Eglot does caching between calls to try-completion,
   all-completions, and others, for example).

b) if the entity changes because the buffer has been changed, by any
   means -- including the means of an completion UI -- then the
   completion UI should re-invoke the capf to get a the new entity to
   complete and the new COLLECTION to complete it.  Emacs's default
   completion UIs do this, and so does Company, if I'm not mistaken.  If
   the backend is super smart and can be faster about responding to this
   new capf invocation validly given the previous respose that's fine.
   But my point is this other caching is much more difficult to do
   accurately because of buffer changes and assumptions about the
   filtering strategy.  For example, a cache that assumes that adding a
   character to the end of entity will always produce a subset of
   previously obtained completions will fail if the completion style is
   some kind of "regexp-based" thing, or if that character is '.', for
   example But it might work decently in some situations.

Anyway, another generic point that I've been trying to make is that
filtering/completion-styling should be done as close to the source of
completions as possible.  In Elisp this is easy to do (completions are
basically lists of strings or the big cheap-to-access obarray).  If the
source of the completions is removed from Emacs by some latency, the
experience is never going to be as good as Elisp.

- If you want to fully honour `completion-styles` which is an Elisp
  facility, you need to know most complete set of completions possible at
  all necessary times.  That's going to be slow (but read my final
  paragraph).

- If you're OK with letting the server do the filtering and the
  highlighting, you can make a "backend" style like I did for SLY, for
  example.  It's going to be faster, but `completion-styles` won't be
  honoured.  That's doesn't mean you give up 100% on "flex".  In SLY,
  there is flex implemented on the Common Lisp side, and for Eglot, many
  LSP server do their own flex matching.

In any case, it is generally possible to design responsive backends
systems that let the user interact with Emacs while the "slow requests"
are ongoing, by having these backends cancel themselves when input is
available.  This uses while-no-input and sit-for.  See jsonrpc.el's
jsonrpc-request for an example.  I've been using these kinds of systems
very successfully over the past 4-5 years with SLY, Eglot as backends
and Company UI as a frontend.

Hope this helped,
João

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Dmitry Gutov]

On 10.09.2021 16:14, João Távora wrote:
> - If you're OK with letting the server do the filtering and the
>    highlighting, you can make a "backend" style like I did for SLY, for
>    example.  It's going to be faster, but `completion-styles` won't be
>    honoured.  That's doesn't mean you give up 100% on "flex".  In SLY,
>    there is flex implemented on the Common Lisp side, and for Eglot, many
>    LSP server do their own flex matching.

You can't really do that with python-shell completion. Nor do you need 
do: the basic pcmpl mechanism should work just fine with it, and for 
performance the completion table just needs some smarter caching.

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Dmitry Gutov]

On 09.09.2021 19:46, Augusto Stoffel wrote:
> To alleviate this, the completion-at-point function could implement some
> sort of caching.  The difficult question is when to invalidate the
> cache.  I've attached one possiblility as a draft patch.  If the
> approach seems reasonable, then I'll format it properly.

It's much simpler than what we talked about, but given that 
python-shell-completion-at-point doesn't really look past the current 
line, your approach should work fine.

I'm not very familiar with the code, so I cannot approve the exact 
patch, though, sorry.

As a bonus, though, maybe add the position of prompt on the shell-buffer 
to the invalidation key? Like, if a user imports some new lib in there, 
that can bring in new completions.

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[João Távora]

On Fri, Sep 10, 2021 at 2:28 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 10.09.2021 16:14, João Távora wrote:
> > - If you're OK with letting the server do the filtering and the
> >    highlighting, you can make a "backend" style like I did for SLY, for
> >    example.  It's going to be faster, but `completion-styles` won't be
> >    honoured.  That's doesn't mean you give up 100% on "flex".  In SLY,
> >    there is flex implemented on the Common Lisp side, and for Eglot, many
> >    LSP server do their own flex matching.
>
> You can't really do that with python-shell completion.

Probably not unless you write some python, no.  I don't see that
as being that dirty.

> Nor do you need
> do: the basic pcmpl mechanism should work just fine with it, and for
> performance the completion table just needs some smarter caching.

Yes, as I said in b), with "sufficiently smart caching" (and infinite
memory space) you can do everything, indeed.  It's one of the
famous "two hard problems" though, so good luck.

João Távora

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Dmitry Gutov]

On 10.09.2021 17:06, João Távora wrote:
> On Fri, Sep 10, 2021 at 2:28 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>>
>> On 10.09.2021 16:14, João Távora wrote:
>>> - If you're OK with letting the server do the filtering and the
>>>     highlighting, you can make a "backend" style like I did for SLY, for
>>>     example.  It's going to be faster, but `completion-styles` won't be
>>>     honoured.  That's doesn't mean you give up 100% on "flex".  In SLY,
>>>     there is flex implemented on the Common Lisp side, and for Eglot, many
>>>     LSP server do their own flex matching.
>>
>> You can't really do that with python-shell completion.
> 
> Probably not unless you write some python, no.  I don't see that
> as being that dirty.

I didn't say it was dirty, just not very fitting for the current 
approach: when you do completion by piping code for evaluation through 
inferior shell, you generally like that code to be simple. And 
reimplementing every completion style in Python seems like anything but.

>> Nor do you need
>> do: the basic pcmpl mechanism should work just fine with it, and for
>> performance the completion table just needs some smarter caching.
> 
> Yes, as I said in b), with "sufficiently smart caching" (and infinite
> memory space) you can do everything, indeed.  It's one of the
> famous "two hard problems" though, so good luck.

Completion backends do caching anyways, whether it's on the Emacs side, 
or somewhere inside a language server.

Since completion logic is defined on "our" side here (despite being 
written in Python), caching logic can reside here too.

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[João Távora]

On Fri, Sep 10, 2021 at 3:22 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 10.09.2021 17:06, João Távora wrote:
> > On Fri, Sep 10, 2021 at 2:28 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
> >>
> >> On 10.09.2021 16:14, João Távora wrote:
> >>> - If you're OK with letting the server do the filtering and the
> >>>     highlighting, you can make a "backend" style like I did for SLY, for
> >>>     example.  It's going to be faster, but `completion-styles` won't be
> >>>     honoured.  That's doesn't mean you give up 100% on "flex".  In SLY,
> >>>     there is flex implemented on the Common Lisp side, and for Eglot, many
> >>>     LSP server do their own flex matching.
> >>
> >> You can't really do that with python-shell completion.
> >
> > Probably not unless you write some python, no.  I don't see that
> > as being that dirty.
>
> I didn't say it was dirty, just not very fitting for the current
> approach: when you do completion by piping code for evaluation through
> inferior shell, you generally like that code to be simple. And
> reimplementing every completion style in Python seems like anything but.

I was just volunteering an opinion on how to solve _most_ of the problem
(hence I wrote "don't give up 100%") Never did I suggest to implement
"every completion style".  SLY implements only two in Common Lisp.
They're pretty sufficient for most users it seems.  Most language servers
seem to implement flex, some only prefix, some may be configurable
I suppose.

> Completion backends do caching anyways, whether it's on the Emacs side,
> or somewhere inside a language server.

There are many types of caching, as I tried to explain.
Inter-capf-invocation caching (if you can understand what I mean)
is possible, but is probably harder to get right than the "intra" version.
I'm really just stating the obvious: the more removed you are from
the source of truth, the harder it becomes to cache. But also, it's
true that are definitely valuable, if implemented correctly or if a
degree of inaccuracy is tolerated. Fast and accurate is hard.

João Távora

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Dmitry Gutov]

On 10.09.2021 17:39, João Távora wrote:
>> Completion backends do caching anyways, whether it's on the Emacs side,
>> or somewhere inside a language server.
> There are many types of caching, as I tried to explain.
> Inter-capf-invocation caching (if you can understand what I mean)
> is possible, but is probably harder to get right than the "intra" version.

The proposed patch provides the "intra" version.

bug#50459: 28.0.50; [PATCH] Python shell completion is incompatible with flex, orderless, etc.

[Augusto Stoffel]

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

Okay, I've attached a patch which seems to work fine.

On Fri, 10 Sep 2021 at 16:32, Dmitry Gutov <dgutov@yandex.ru> wrote:

> On 09.09.2021 19:46, Augusto Stoffel wrote:
>> To alleviate this, the completion-at-point function could implement some
>> sort of caching.  The difficult question is when to invalidate the
>> cache.  I've attached one possiblility as a draft patch.  If the
>> approach seems reasonable, then I'll format it properly.
>
> It's much simpler than what we talked about, but given that
> python-shell-completion-at-point doesn't really look past the current
> line, your approach should work fine.

Yes, and you're right --- I've did it that way because this
comint-based completion basically only works for globals anyway.

>
> I'm not very familiar with the code, so I cannot approve the exact
> patch, though, sorry.
>
> As a bonus, though, maybe add the position of prompt on the
> shell-buffer to the invalidation key? Like, if a user imports some new
> lib in there, that can bring in new completions.

Good idea.  I've added this to the patch.  It still won't show the new
completions until the user evaluates the import, but this is the best
you can get by querying an interpreter for the completions.

I've also changed a bit the way the "native completion" setup code is
sent, so that it doesn't print a message directly in the shell buffer
(there is still an echo area message).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Implement-caching-for-python-shell-completion-at-poi.patch --]
[-- Type: text/x-patch, Size: 5953 bytes --]

From 3f27a63fe5be884464a6a68407e0b6990186c4a6 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Fri, 10 Sep 2021 20:44:10 +0200
Subject: [PATCH] Implement caching for 'python-shell-completion-at-point'

* lisp/progmodes/python.el (python-shell-completion-at-point): cache
results, since computing them involves talking with the inferior
process and, potentially, network communications
(python-shell--capf-cache): new variable, for cache
(python-shell-completion-get-completions,
python-shell-completion-native-get-completions): 'import' argument is
not needed anymore.
(python-shell-completion-native-setup,
python-shell-completion-native-try): pass the setup code
synchronously, to avoid printing a message in the shell.
---
 lisp/progmodes/python.el | 59 ++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 29 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 2eef52de0c..e71a8102df 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -3577,13 +3577,12 @@ python-shell-completion-native-try
          python-shell-completion-native-try-output-timeout))
     (python-shell-completion-native-get-completions
      (get-buffer-process (current-buffer))
-     nil "_")))
+     "_")))
 
 (defun python-shell-completion-native-setup ()
   "Try to setup native completion, return non-nil on success."
-  (let ((process (python-shell-get-process)))
-    (with-current-buffer (process-buffer process)
-      (python-shell-send-string "
+  (let* ((process (python-shell-get-process))
+         (output (python-shell-send-string-no-output "
 def __PYTHON_EL_native_completion_setup():
     try:
         import readline
@@ -3693,14 +3692,10 @@ python-shell-completion-native-setup
         print ('python.el: native completion setup failed, %s: %s'
                % sys.exc_info()[:2])
 
-__PYTHON_EL_native_completion_setup()" process)
-      (when (and
-             (python-shell-accept-process-output
-              process python-shell-completion-native-try-output-timeout)
-             (save-excursion
-               (re-search-backward
-                (regexp-quote "python.el: native completion setup loaded") nil t 1)))
-        (python-shell-completion-native-try)))))
+__PYTHON_EL_native_completion_setup()" process)))
+    (when (string-match-p "python\\.el: native completion setup loaded"
+                          output)
+      (python-shell-completion-native-try))))
 
 (defun python-shell-completion-native-turn-off (&optional msg)
   "Turn off shell native completions.
@@ -3760,13 +3755,10 @@ python-shell-completion-native-toggle
       (python-shell-completion-native-turn-on msg))
     python-shell-completion-native-enable))
 
-(defun python-shell-completion-native-get-completions (process import input)
-  "Get completions using native readline for PROCESS.
-When IMPORT is non-nil takes precedence over INPUT for
-completion."
+(defun python-shell-completion-native-get-completions (process input)
+  "Get completions of INPUT using native readline for PROCESS."
   (with-current-buffer (process-buffer process)
-    (let* ((input (or import input))
-           (original-filter-fn (process-filter process))
+    (let* ((original-filter-fn (process-filter process))
            (redirect-buffer (get-buffer-create
                              python-shell-completion-native-redirect-buffer))
            (trigger "\t")
@@ -3818,11 +3810,8 @@ python-shell-completion-native-get-completions
                  :test #'string=))))
         (set-process-filter process original-filter-fn)))))
 
-(defun python-shell-completion-get-completions (process import input)
-  "Do completion at point using PROCESS for IMPORT or INPUT.
-When IMPORT is non-nil takes precedence over INPUT for
-completion."
-  (setq input (or import input))
+(defun python-shell-completion-get-completions (process input)
+  "Get completions of INPUT using PROCESS."
   (with-current-buffer (process-buffer process)
     (let ((completions
            (python-util-strip-string
@@ -3836,6 +3825,9 @@ python-shell-completion-get-completions
         (split-string completions
                       "^'\\|^\"\\|;\\|'$\\|\"$" t)))))
 
+(defvar-local python-shell--capf-cache nil
+  "Variable to store cached completions and invalidation keys.")
+
 (defun python-shell-completion-at-point (&optional process)
   "Function for `completion-at-point-functions' in `inferior-python-mode'.
 Optional argument PROCESS forces completions to be retrieved
@@ -3889,12 +3881,21 @@ python-shell-completion-at-point
                        ;; it during a multiline statement (Bug#28051).
                        #'ignore
                      #'python-shell-completion-get-completions))
-                  (t #'python-shell-completion-native-get-completions)))))
-    (list start end
-          (completion-table-dynamic
-           (apply-partially
-            completion-fn
-            process import-statement)))))
+                  (t #'python-shell-completion-native-get-completions))))
+         (prev-prompt (car python-shell--capf-cache))
+         (re (or (cadr python-shell--capf-cache) regexp-unmatchable))
+         (prefix (buffer-substring-no-properties start end)))
+    ;; To invalidate the cache, we check if the prompt position or the
+    ;; completion prefix changed.
+    (unless (and (equal prev-prompt (car prompt-boundaries))
+                 (string-match re prefix))
+      (setq python-shell--capf-cache
+            `(,(car prompt-boundaries)
+              ,(if (string-empty-p prefix)
+                   regexp-unmatchable
+                 (concat "\\`" (regexp-quote prefix) "\\(?:\\sw\\|\\s_\\)*\\'"))
+              ,@(funcall completion-fn process (or import-statement prefix)))))
+    (list start end (cddr python-shell--capf-cache))))
 
 (define-obsolete-function-alias
   'python-shell-completion-complete-at-point
-- 
2.31.1

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Augusto Stoffel]

On Fri, 10 Sep 2021 at 17:43, Dmitry Gutov <dgutov@yandex.ru> wrote:

> On 10.09.2021 17:39, João Távora wrote:
>>> Completion backends do caching anyways, whether it's on the Emacs side,
>>> or somewhere inside a language server.
>> There are many types of caching, as I tried to explain.
>> Inter-capf-invocation caching (if you can understand what I mean)
>> is possible, but is probably harder to get right than the "intra" version.
>
> The proposed patch provides the "intra" version.

I just wanted to emphasize that the approach of evaluating code and
asking an interpreter for completions is obviously a dead end.

I guess it is still the best available at the moment for the REPL, so it
makes sense to ensure it's reasonably correct and doesn't hang your
comint.  But further optimizations are not really worth the effort.

The LSP stuff is much more interesting, and there the caching question
is pretty hairy...

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[João Távora]

On Fri, Sep 10, 2021 at 8:27 PM Augusto Stoffel <arstoffel@gmail.com> wrote:
>
> On Fri, 10 Sep 2021 at 17:43, Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> > On 10.09.2021 17:39, João Távora wrote:
> >>> Completion backends do caching anyways, whether it's on the Emacs side,
> >>> or somewhere inside a language server.
> >> There are many types of caching, as I tried to explain.
> >> Inter-capf-invocation caching (if you can understand what I mean)
> >> is possible, but is probably harder to get right than the "intra" version.
> >
> > The proposed patch provides the "intra" version.
>
> I just wanted to emphasize that the approach of evaluating code and
> asking an interpreter for completions is obviously a dead end.

I wouldn't put it that strongly, but for sure not as easy to leverage as a
ready to use LSP server.  That's the promise of LSP, at least.  Also,
in my experience, I've found that completions are just one aspect
of IDEs, and perhaps not even the most important.  "Find definition"
and diagnostics are perhaps more important for me.

So yak-shaving and obsessing over optimizations to this reasonably
hard problem (implementing responsive and fast UIs with arbitrarily
sophisticated filtering styles over data that is some tenths of seconds
away) might not be worth it. OTOH, yak-shaving is what Emacs is
about, in some measure ,and if someone shaves the yak for me, I'll
gladly take a shaved yak.

João

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Lars Ingebrigtsen]

Augusto Stoffel <arstoffel@gmail.com> writes:

> That's true: the logic here is that the completions are computed
> eagerly, and then cached until still valid.  So if you type
>
>   fo<tab>o.bar.baz
>
> the inferior process is contacted 3 times: after the <tab>, and after
> each dot.  Before, a lazy table was returned, but the inferior would be
> contacted after each character anyway (if using Company or similar).

Ah, I see, then that sounds like a definite improvement.  My other
question is then how the cache is flushed...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Augusto Stoffel]

On Sat, 11 Sep 2021 at 14:09, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Augusto Stoffel <arstoffel@gmail.com> writes:
>
>> That's true: the logic here is that the completions are computed
>> eagerly, and then cached until still valid.  So if you type
>>
>>   fo<tab>o.bar.baz
>>
>> the inferior process is contacted 3 times: after the <tab>, and after
>> each dot.  Before, a lazy table was returned, but the inferior would be
>> contacted after each character anyway (if using Company or similar).
>
> Ah, I see, then that sounds like a definite improvement.  My other
> question is then how the cache is flushed...

The cache will be flushed, at the earliest, when a new call to
'python-shell-completion-at-point' is made.  And the criterium is the
following:

- If the prompt moved, then the cache is flushed.
- If the current completion prefix (the text between 'start' and 'end')
  is not identical to the original completion prefix, except for the
  addition of some word or symbol characters at the end, then the cache
  is flushed.

That's it in a nutshell.  I can give more details if you want.

bug#50459: 28.0.50; Python shell completion is incompatible with flex, orderless, etc.

[Lars Ingebrigtsen]

Augusto Stoffel <arstoffel@gmail.com> writes:

> The cache will be flushed, at the earliest, when a new call to
> 'python-shell-completion-at-point' is made.  And the criterium is the
> following:
>
> - If the prompt moved, then the cache is flushed.
> - If the current completion prefix (the text between 'start' and 'end')
>   is not identical to the original completion prefix, except for the
>   addition of some word or symbol characters at the end, then the cache
>   is flushed.

Ah, I got the last bit, but I missed the first part.  Well, then I think
this sounds like a good approach?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#50459: 28.0.50; [PATCH] Python shell completion is incompatible with flex, orderless, etc.

[Lars Ingebrigtsen]

Augusto Stoffel <arstoffel@gmail.com> writes:

> Good idea.  I've added this to the patch.  It still won't show the new
> completions until the user evaluates the import, but this is the best
> you can get by querying an interpreter for the completions.
>
> I've also changed a bit the way the "native completion" setup code is
> sent, so that it doesn't print a message directly in the shell buffer
> (there is still an echo area message).

Looks good to me; pushed to Emacs 28 now.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no