bug#19736: 24.4; python.el: native completion breaks ipython magic completion

bug#19736: 24.4; python.el: native completion breaks ipython magic completion

[Carlos Pita]

Hi Fabian,

I think you should be using [1] as the ipython completer. In a sense,
when you inherit from rlcompleter.Completer and replace the current
completer you're somehow going against the goal of native completion,
aren't you? Wouldn't it be better to just monkey patch the completer
returned by readline.get_completer() with your _callable_postfix in case
the completer indeed defines a _callable_postfix (it seems ipython
completer don't do that)?

Cheers
--
Carlos

[1] http://ipython.org/ipython-doc/dev/api/generated/IPython.core.completer.html

bug#19736:

[Carlos Pita]

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

Tags: patch

Hi Fabian,

I've fixed this bug. It was harder than I'd initially thought as
several issues converged to screw ipython completion up:

1) The native completion setup code was importing rlcompleter. Just
importing this overrides the current readline completer (the import
calls set_completer to set the standard python completer, which is not
quite the same as the completer ipython had previously set up).

2) The fallback completion setup code was also importing rlcompleter
unconditionally, thus replacing the ipython completer too.

3) The _callable_postfix + triple-tab workaround was not available for
the ipython completer AFAICS, because it has no _callable_postfix
method. I opted for another (IMO a bit cleaner) workaround, see below.

So, here is what I've done to fix the issues above:

3') I'm wrapping the current complete function (as returned by
readline.get_completer) so that the wrapper never returns just one
completion. In case the original completer returned one completion, a
'__dummy_completion__' is added to the completion list by the wrapper.
This avoids the need to define a _callable_postifx (unavailable in
ipython) and also avoids the need for the triple-tab hack and
duplicate filtering (obviously, you still have to filter
'__dummy_completion__'). All in all I think the approach is more
robust although still a bit hackish. IMO, it should be documented
better in the code, I spent quite a time figuring out WTF the triple
tabs and _callable_postfix were supposed to do! :)

1') Because of 3', there is no need to import rlcompleter in the
native completion setup code anymore.

2') I've done some refactoring of the fallback completion setup code
in order to import rlcompleter only in case the current shell is not
ipython. In general, I think this setup code is a bit more clear now,
but you're the one to judge that.

I'm attaching a patch against my version of python.el which is a
little outdated and has some patches of mine applied on top. But I
think you won't have any problems figuring out the needed changes as
they closely track the description above.

Cheers
--
Carlos

[-- Attachment #2: complete.diff --]
[-- Type: text/plain, Size: 4826 bytes --]

diff --git a/.emacs.d/lisp/python.el b/.emacs.d/lisp/python.el
index b959fc4..332ca44 100644
--- a/.emacs.d/lisp/python.el
+++ b/.emacs.d/lisp/python.el
@@ -2950,43 +2962,38 @@ This function takes the list of setup code to send from the
 ;;; Shell completion
 
 (defcustom python-shell-completion-setup-code
-  "try:
-    import __builtin__
-except ImportError:
-    # Python 3
-    import builtins as __builtin__
-try:
-    import readline, rlcompleter
-except:
-    def __PYTHON_EL_get_completions(text):
-        return []
-else:
-    def __PYTHON_EL_get_completions(text):
-        builtins = dir(__builtin__)
-        completions = []
-        try:
+  "def __PYTHON_EL_get_completions(text):
+    try:
+        import __builtin__
+    except ImportError:  # Python 3
+        import builtins as __builtin__
+    builtins = dir(__builtin__)
+    is_ipython = ('__IPYTHON__' in builtins or
+                  '__IPYTHON__active' in builtins)
+    completions = []
+    try:
+        if is_ipython:
             splits = text.split()
             is_module = splits and splits[0] in ('from', 'import')
-            is_ipython = ('__IPYTHON__' in builtins or
-                          '__IPYTHON__active' in builtins)
             if is_module:
                 from IPython.core.completerlib import module_completion
                 completions = module_completion(text.strip())
-            elif is_ipython and '__IP' in builtins:
+            elif '__IP' in builtins:
                 completions = __IP.complete(text)
-            elif is_ipython and 'get_ipython' in builtins:
+            elif 'get_ipython' in builtins:
                 completions = get_ipython().Completer.all_completions(text)
-            else:
-                i = 0
-                while True:
-                    res = readline.get_completer()(text, i)
-                    if not res:
-                        break
-                    i += 1
-                    completions.append(res)
-        except:
-            pass
-        return completions"
+        else:  # Never do this in IPython as it overrides the completer!
+            import readline, rlcompleter
+            i = 0
+            while True:
+                res = readline.get_completer()(text, i)
+                if not res:
+                   break
+                i += 1
+                completions.append(res)
+    except:
+        pass
+    return completions"
   "Code used to setup completion in inferior Python processes."
   :type 'string
   :group 'python)
@@ -3053,18 +3060,20 @@ When a match is found, native completion is disabled."
       #'identity
       (list
        "try:"
-       "    import readline, rlcompleter"
-       ;; Remove parens on callables as it breaks completion on
-       ;; arguments (e.g. str(Ari<tab>)).
-       "    class Completer(rlcompleter.Completer):"
-       "        def _callable_postfix(self, val, word):"
-       "            return word"
-       "    readline.set_completer(Completer().complete)"
+       "    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")
@@ -3146,7 +3155,7 @@ completion."
                              python-shell-completion-native-redirect-buffer))
            (separators (python-rx
                         (or whitespace open-paren close-paren)))
-           (trigger "\t\t\t")
+           (trigger "\t")
            (new-input (concat input trigger))
            (input-length
             (save-excursion
@@ -3192,11 +3201,13 @@ completion."
               (while (accept-process-output
                       process
                       python-shell-completion-native-output-timeout))
-              (cl-remove-duplicates
+              (cl-remove
+               (concat input "__dummy_completion__")
                (split-string
                 (buffer-substring-no-properties
                  (point-min) (point-max))
-                separators t))))
+                separators t)
+               :test #'string=)))
         (set-process-filter process original-filter-fn)))))
 
 (defun python-shell-completion-get-completions (process import input)

bug#19736:

[Carlos Pita]

Just in case, here is the modified code for both completion setups
(I'm not listing the minor changes to
python-shell-completion-native-get-completions below):


(defcustom python-shell-completion-setup-code
  "def __PYTHON_EL_get_completions(text):
    try:
        import __builtin__
    except ImportError:  # Python 3
        import builtins as __builtin__
    builtins = dir(__builtin__)
    is_ipython = ('__IPYTHON__' in builtins or
                  '__IPYTHON__active' in builtins)
    completions = []
    try:
        if is_ipython:
            splits = text.split()
            is_module = splits and splits[0] in ('from', 'import')
            if is_module:
                from IPython.core.completerlib import module_completion
                completions = module_completion(text.strip())
            elif '__IP' in builtins:
                completions = __IP.complete(text)
            elif 'get_ipython' in builtins:
                completions = get_ipython().Completer.all_completions(text)
        else:  # Never do this in IPython as it overrides the completer!
            import readline, rlcompleter
            i = 0
            while True:
                res = readline.get_completer()(text, i)
                if not res:
                   break
                i += 1
                completions.append(res)
    except:
        pass
    return completions"
  "Code used to setup completion in inferior Python processes."
  :type 'string
  :group 'python)


(defun python-shell-completion-native-setup ()
  "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")
     process)
    (python-shell-accept-process-output process)
    (when (save-excursion
            (re-search-backward
             (regexp-quote "python.el: readline is available") nil t 1))
      (python-shell-completion-native-try))))

bug#19736:

[Carlos Pita]

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

Patch updated against the current master.

[-- Attachment #2: complete.patch --]
[-- Type: text/x-patch, Size: 5203 bytes --]

From d3ceebd6ea08475b7f72090e0a219a51aaac21fd Mon Sep 17 00:00:00 2001
From: memeplex <carlosjosepita@gmail.com>
Date: Mon, 2 Feb 2015 14:59:24 -0300
Subject: [PATCH] python.el: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=19736

---
 .emacs.d/lisp/python.el | 79 ++++++++++++++++++++++++-------------------------
 1 file changed, 39 insertions(+), 40 deletions(-)

diff --git a/.emacs.d/lisp/python.el b/.emacs.d/lisp/python.el
index b959fc4..5f8a9e8 100644
--- a/.emacs.d/lisp/python.el
+++ b/.emacs.d/lisp/python.el
@@ -2950,43 +2950,38 @@ This function takes the list of setup code to send from the
 ;;; Shell completion
 
 (defcustom python-shell-completion-setup-code
-  "try:
-    import __builtin__
-except ImportError:
-    # Python 3
-    import builtins as __builtin__
-try:
-    import readline, rlcompleter
-except:
-    def __PYTHON_EL_get_completions(text):
-        return []
-else:
-    def __PYTHON_EL_get_completions(text):
-        builtins = dir(__builtin__)
-        completions = []
-        try:
+  "def __PYTHON_EL_get_completions(text):
+    try:
+        import __builtin__
+    except ImportError:  # Python 3
+        import builtins as __builtin__
+    builtins = dir(__builtin__)
+    is_ipython = ('__IPYTHON__' in builtins or
+                  '__IPYTHON__active' in builtins)
+    completions = []
+    try:
+        if is_ipython:
             splits = text.split()
             is_module = splits and splits[0] in ('from', 'import')
-            is_ipython = ('__IPYTHON__' in builtins or
-                          '__IPYTHON__active' in builtins)
             if is_module:
                 from IPython.core.completerlib import module_completion
                 completions = module_completion(text.strip())
-            elif is_ipython and '__IP' in builtins:
+            elif '__IP' in builtins:
                 completions = __IP.complete(text)
-            elif is_ipython and 'get_ipython' in builtins:
+            elif 'get_ipython' in builtins:
                 completions = get_ipython().Completer.all_completions(text)
-            else:
-                i = 0
-                while True:
-                    res = readline.get_completer()(text, i)
-                    if not res:
-                        break
-                    i += 1
-                    completions.append(res)
-        except:
-            pass
-        return completions"
+        else:  # Never do this in IPython as it overrides the completer!
+            import readline, rlcompleter
+            i = 0
+            while True:
+                res = readline.get_completer()(text, i)
+                if not res:
+                   break
+                i += 1
+                completions.append(res)
+    except:
+        pass
+    return completions"
   "Code used to setup completion in inferior Python processes."
   :type 'string
   :group 'python)
@@ -3053,18 +3048,20 @@ When a match is found, native completion is disabled."
       #'identity
       (list
        "try:"
-       "    import readline, rlcompleter"
-       ;; Remove parens on callables as it breaks completion on
-       ;; arguments (e.g. str(Ari<tab>)).
-       "    class Completer(rlcompleter.Completer):"
-       "        def _callable_postfix(self, val, word):"
-       "            return word"
-       "    readline.set_completer(Completer().complete)"
+       "    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")
@@ -3146,7 +3143,7 @@ completion."
                              python-shell-completion-native-redirect-buffer))
            (separators (python-rx
                         (or whitespace open-paren close-paren)))
-           (trigger "\t\t\t")
+           (trigger "\t")
            (new-input (concat input trigger))
            (input-length
             (save-excursion
@@ -3192,11 +3189,13 @@ completion."
               (while (accept-process-output
                       process
                       python-shell-completion-native-output-timeout))
-              (cl-remove-duplicates
+              (cl-remove
+               (concat input "__dummy_completion__")
                (split-string
                 (buffer-substring-no-properties
                  (point-min) (point-max))
-                separators t))))
+                separators t)
+               :test #'string=)))
         (set-process-filter process original-filter-fn)))))
 
 (defun python-shell-completion-get-completions (process import input)
-- 
2.2.1

bug#19736: python.el: native completion breaks ipython magic completion

[Fabián Ezequiel Gallina]

Revno ab9252a@master fixes this.


Cheers,
Fabián.