bug#19755: python.el: native completion: more problems (and solutions)

bug#19755: python.el: native completion: more problems (and solutions)

[Carlos Pita]

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

X-Debbugs-CC: galli.87@gmail.com
Tags: patch

Hi Fabian,

besides the ones l have already reported and patched in [1] there are
some remaining problems with the native completion approach:

1) The waiting timeout is too low for import completions, as imports
take a bit more time than simple dirs. But making the timeout larger
is bad for cases when there really is no completion at all for the
current prefix. I want to avoid this trade off.

2) When all the completions share a common prefix, readline completes
inline up to this common prefix, so the delete-line-command sequence
length is wrong. For example, say the current prefix is "x" and that
its completions are "xxy", "xxz". Readline will complete "xx" inline,
but delete-line-command will only delete one "x".

3) A pager could be enabled to show long lists of completions.

My solutions for 1 and 2 simply extend the dummy completion hack I
introduced in [1]:

1') Ensure there is always one completion at least (a fortiori, I'm
ensuring there are at least two completions, to force listing instead
of inline completion). So we can set a larger timeout *just for the
first* accept-process-output without the risk of waiting too much when
there is really no completion available.

2') Ensure there is a completion with a different prefix, by providing
a dummy randomly prefixed completion.

For 3 I just disabled paged completions for readline, but I don't know
how to do that for libedit.

Attached is a patch with all the described changes, applied on top of
my patch for [1]. Anyway, it should be easy to selectively apply the
changes manually.

All in all I find the solution pretty simple and it leverages my
previous trick. The completer is a bit more complex now as it has to
keep track of a couple of states in order to generate the correct
sequence of completions (dummies included):

    def completer(text, state,
                  real_completer=readline.get_completer(),
                  last_state=[None]):
        completion = None
        if state == 0:
            last_state[0] = None
        if last_state[0] is None:
            completion = real_completer(text, state)
            if not completion:
                last_state[0] = state
        if (state == last_state[0] or
            state - 1 == last_state[0] == 0):
            import random
            completion = '%s%d__dummy_completion__' % (
                text, random.randint(1000, 10000))
        return completion
    readline.set_completer(completer)

Cheers
--
Carlos

[1] http://debbugs.gnu.org/cgi/bugreport.cgi?bug=19736

[-- Attachment #2: native-compl.patch --]
[-- Type: text/x-patch, Size: 3970 bytes --]

diff --git a/.emacs.d/lisp/python.el b/.emacs.d/lisp/python.el
index ca69b68..15041a3 100644
--- a/.emacs.d/lisp/python.el
+++ b/.emacs.d/lisp/python.el
@@ -3071,28 +3071,38 @@ When a match is found, native completion is disabled."
   "Try to setup native completion, return non-nil on success."
   (let ((process (python-shell-get-process)))
     (python-shell-send-string
-     (funcall
-      'mapconcat
-      #'identity
-      (list
-       "try:"
-       "    import readline"
-       "    def completer(text, state, c=readline.get_completer()):"
-       "        completion = c(text, state)"
-       "        if not completion and state == 1:"
-       "            return text + '__dummy_completion__'"
-       "        else:"
-       "            return completion"
-       "    readline.set_completer(completer)"
-       "    if readline.__doc__ and 'libedit' in readline.__doc__:"
-       "        readline.parse_and_bind('bind ^I rl_complete')"
-       "    else:"
-       "        readline.parse_and_bind('tab: complete')"
-       "    print ('python.el: readline is available')"
-       "    del completer, readline  # Some cleanup"
-       "except:"
-       "    print ('python.el: readline not available')")
-      "\n")
+     ;; Ensure at least two completions:
+     ;;  - Random prefix avoids common prefix inline completion
+     ;;  - Two completions avoid single inline completion
+     ;;  - One sure completion avoids waiting output timeout
+     "try:
+    import readline
+    def completer(text, state,
+                  real_completer=readline.get_completer(),
+                  last_state=[None]):
+        completion = None
+        if state == 0:
+            last_state[0] = None
+        if last_state[0] is None:
+            completion = real_completer(text, state)
+            if not completion:
+                last_state[0] = state
+        if (state == last_state[0] or
+            state - 1 == last_state[0] == 0):
+            import random
+            completion = '%s%d__dummy_completion__' % (
+                text, random.randint(1000, 10000))
+        return completion
+    readline.set_completer(completer)
+    if readline.__doc__ and 'libedit' in readline.__doc__:
+        readline.parse_and_bind('bind ^I rl_complete')
+    else:
+        readline.parse_and_bind('tab: complete')
+        readline.parse_and_bind('set page-completions off')
+    print('python.el: readline is available')
+    del completer, readline  # Env cleanup
+except:
+    print('python.el: readline not available')"
      process)
     (python-shell-accept-process-output process)
     (when (save-excursion
@@ -3206,9 +3216,7 @@ completion."
                             #'comint-redirect-filter original-filter-fn))
                 (set-process-filter process #'comint-redirect-filter))
               (process-send-string process input-to-send)
-              (accept-process-output
-               process
-               python-shell-completion-native-output-timeout)
+              (accept-process-output process 0.3)
               ;; XXX: can't use `python-shell-accept-process-output'
               ;; here because there are no guarantees on how output
               ;; ends.  The workaround here is to call
@@ -3217,13 +3225,12 @@ completion."
               (while (accept-process-output
                       process
                       python-shell-completion-native-output-timeout))
-              (cl-remove
-               (concat input "__dummy_completion__")
+              (cl-remove-if
+               (lambda (c) (string-match "__dummy_completion__" c))
                (split-string
                 (buffer-substring-no-properties
                  (point-min) (point-max))
-                separators t)
-               :test #'string=)))
+                separators t))))
         (set-process-filter process original-filter-fn)))))
 
 (defun python-shell-completion-get-completions (process import input)

bug#19755: Acknowledgement (python.el: native completion: more problems (and solutions))

[Carlos Pita]

X-Debbugs-CC: galli.87@gmail.com

After investing some hours on the many border cases of native
completion I sadly came to realize that there is a simpler and not so
fragile approach: as we're already assuming readline is available, why
don't we just get the current readline completer and use it directly?
This is not so different from the legacy completion mechanism, except
that we don't create the completer but use the same one that readline
is using. I think the completion code (legacy and native) could be
unified around a completer function and a lot of cruft related to
process interaction trickery (timeouts, control char sequences, output
parsing, etc.) could be wiped out from the code for good. Am I missing
something? Despite having to throw away so many hours of unpleasant
work I would like to simplify the completion code and I'm offering to
do it myself if you want. Just let me know what do you think about
this idea... maybe I'm not seeing an obvious reason why we should go
through readline user interface instead of readline programmer
interface.

On Tue, Feb 3, 2015 at 9:41 AM, GNU bug Tracking System
<help-debbugs@gnu.org> wrote:
> Thank you for filing a new bug report with debbugs.gnu.org.
>
> This is an automatically generated reply to let you know your message
> has been received.
>
> Your message is being forwarded to the package maintainers and other
> interested parties for their attention; they will reply in due course.
>
> As you requested using X-Debbugs-CC, your message was also forwarded to
>   galli.87@gmail.com
> (after having been given a bug report number, if it did not have one).
>
> Your message has been sent to the package maintainer(s):
>  bug-gnu-emacs@gnu.org
>
> If you wish to submit further information on this problem, please
> send it to 19755@debbugs.gnu.org.
>
> Please do not send mail to help-debbugs@gnu.org unless you wish
> to report a problem with the Bug-tracking system.
>
> --
> 19755: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=19755
> GNU Bug Tracking System
> Contact help-debbugs@gnu.org with problems

bug#19755: python.el: native completion: more problems (and solutions)

[Carlos Pita]

Ok, after a conversation with Fabian I understood which are the
shortcomings of the legacy mechanism:

1) While inside a block of code asking a completion to the shell makes
it believe that you're not inside the block anymore.

2) Spurious prompt number increments take place each time a completion
is requested to the shell.

So it's not just a matter of the completer being the preinstalled by
the shell or some other one that python.el instantiates itself during
the setup phase.

Another solution could be to open a simple python helper thread that
listens on a socket. This thread could provide documentation for eldoc
or for help buffers (I'm about to post a RFE for this feature),
besides the completions. Another adventage is that it will be possible
to unify the completion mechanisms again.

What do you think?

Cheers
--
Carlos

bug#19755: python.el: native completion: more problems (and solutions)

[Carlos Pita]

Here is a quick prototype of how the client/server approach would look like.

The setup code could be structured around a single class PythonElHelper, say.

The class will listen on a socket in a dedicated thread.

I have coded its output as json in order to allow to pass structured
information to the elisp side and easily parse this information as
elisp data with json-read-from-string.

I don't see a need to use a full-fledged http server, although that
could be done with url.el if desired.

The server would provide tooltips for eldoc, documentation for the
help buffer and completions for completion-at-point in a way that
doesn't interfere with history, prompt numbers, block editing, etc.
and doesn't require to deal with the shell input/output.

Do you think this approach is sensible? I'm available for coding this
along the next two months or so.

######################## python server #################


class PythonElServer:

    def __init__(self):
        # detect python2/3, ipython, readline, set completer, etc
        pass

    def complete(self, symbol):
        return ["a", "b", "c"]

    def tooltip(self, symbol):
        return "func(a, b, c)"

    def documentation(self, symbol):
        return "A function that does something"

    def serve(self, port):
        import socket, json
        server = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        server.bind(('', port))
        server.listen(1)
        (client, address) = server.accept()
        while True:
            request = client.recv(1024)
            action, symbol = request.split()
            response = getattr(self, action)(symbol)
            client.send(json.dumps(response))

PythonElServer().serve(9090)


######################## elisp client #################

;; -*- lexical-binding: t -*-

(setq python-shell-server
      (open-network-stream "pyserver" "pyserver" "localhost" 9091))

(defun python-shell-server-request (action symbol)
  (set-process-filter
   python-shell-server
   (lambda (proc out)
     (let ((response (json-read-from-string out)))
       (pcase action
     (`complete (python-shell-complete response))
     (`tooltip (python-shell-tooltip response))
     (`documentation (python-shell-documentation response))))))
  (process-send-string python-shell-server
               (concat (symbol-name action) " " symbol)))

(defun python-shell-complete (response)
  (print (concat "complete: " response)))

(defun python-shell-tooltip (response)
  (print (concat "tooltip: " response)))

(defun python-shell-documentation (response)
  (print (concat "documentation: " response)))

(python-shell-server-request 'documentation "xxx")

bug#19755: python.el: native completion: more problems (and solutions)

[Fabián Ezequiel Gallina]

Hola Carlos,

I pushed a fix for this at revno 911ed2e in the master branch.

I'll be slowly catching up, sorry for the long delay.


Cheers,
Fabián.