sagemath/sage

Followup to #34547: fix emacs sage-shell-mode

Closed this issue · 12 comments

It turns out that sage-shell-mode in emacs uses the variable interfaces, deleted from sage.interfaces.all in #34547. We restore that variable (which is not used anywhere in the Sage library) along with a comment.

Component: interfaces

Keywords: emacs

Author: John Palmieri

Branch/Commit: u/jhpalmieri/interfaces-emacs @ 49d4ff7

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/34935

New commits:

8e76d0dtrac 34935: fix sage-shell mode in emacs

Commit: 8e76d0d

Reviewer: Matthias Koeppe

comment:3

The backslashes are not needed, but positive review nevertheless.

Changed commit from 8e76d0d to 49d4ff7

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:

49d4ff7trac 34935: fix sage-shell mode in emacs
comment:5

Right, sorry, I was just copying the old lines back. Let's get rid of the backslashes.

comment:6

Wow, that's great!

Now that you found the culprit, what do you think about simply removing the offending lines in sage-shell-mode itself?

I admit that I do not understand at all why names in interfaces get a special treatment.

Another special case I do not understand is line 351:

ignore_classes = [sage.interfaces.gap.Gap, sage.misc.lazy_import.LazyImport]

We could actually take the opportunity do (optionally?) remove the underscore and double underscore methods from the completion list. I never understood that the code was so simple!

I would do this, if at least one further person is interested.

--- /home/martin/.emacs.d/elpa/sage-shell-mode-20221020.1012/emacs_sage_shell.py.old	2023-01-25 07:57:39.845564969 +0100
+++ /home/martin/.emacs.d/elpa/sage-shell-mode-20221020.1012/emacs_sage_shell.py	2023-01-25 07:58:33.123905933 +0100
@@ -52,8 +52,6 @@
 except:
     pass
 
-interfaces = ip.ev('interfaces')
-
 _sage_const_regexp = re.compile("_sage_const_")
 
 
@@ -168,12 +166,10 @@
         regexp = re.compile("^[ a-zA-Z0-9._\\[\\]]+$")
         if regexp.match(varname) is None:
             return []
-        if varname in interfaces:
-            ls = ip.ev('dir(%s)' % (varname,))
-        else:
-            ls = _completions_attributes(preparse(varname))
-            ls.extend(ip.ev('dir(%s)' % (preparse(varname),)))
-            ls = list(sorted(set(ls)))
+        name = preparse(varname)
+        ls = _completions_attributes(name)
+        ls.extend(ip.ev('dir(%s)' % (name,)))
+        ls = list(sorted(set(ls)))
         return ls
     except:
         return []
comment:7

Replying to John Palmieri:

Right, sorry, I was just copying the old lines back. Let's get rid of the backslashes.

I confirm that 9.8.beta7 + the present patch runs under sage-shell-mode.

Thanks a lot !

comment:8

Replying to Martin Rubey:

Wow, that's great!

Now that you found the culprit, what do you think about simply removing the offending lines in sage-shell-mode itself?

I don't use sage-shell-mode, so I don't know what users would lose if we got rid of those lines. I think we should keep this change for now and then possibly change sage-shell-mode separately. If we remove this from sage-shell-mode, then some time after that, we can remove the interfaces variable from Sage. (We probably should keep it for a while for people using older versions of sage-shell-mode.)

Merged to Sage 9.8.rc1