[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: completion-at-point + semantic : erroneous error

From: Eric Ludlam
Subject: Re: completion-at-point + semantic : erroneous error
Date: Fri, 11 Oct 2019 21:06:04 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0

On 10/11/19 1:34 PM, Stefan Monnier wrote:
PS: It would also be good to replace uses of
define-overloadable-function with cl-defmethod, although the mapping
from one to the other is not immediate.
The patch below is an attempt to do that for
semantic-analyze-possible-completions.  I'd be interested to hear your
opinion on this (especially the with-mode-local part).

Thanks for looking into this Stefan.

For the part of the patch that only issues the error if called interactively, that makes sense to me.  I used that fcn interactively when debugging the tool, or debugging new language support, but most users probably won't.

I don't know enough about how far and wide it is used these days to know if that error is needed by anyone.  I would suspect not, since returning empty happens for other reasons too.

For the part of the patch replacing `define-overloadable-function' with methods, I was surprised by the &context syntax.  I've been away from emacs devel too long to be familiar with it, but the doc on it was helpful. mode-local.el was always a hack to overcome the mess that was managing behavior changes between modes while providing useful base functionality.  Updating to methods using &context seems like a fine way to solve the same problem.

I went back to mode-local to remind myself about it.  One of the things it handled was derived modes.  I vaguely recall this being important for Makefiles as there are several makefile-mode variants? (looks in make-mode.el) - yes, there are a bunch.  Not sure what a "&context (major-mode ...) " does to know if it is derived from a parent mode or not.  There is a mode-local-equivalent-mode-p predicate for handling that.

Also in your patch is a TODO about `with-mode-local'.  That with-mode-local did more than just allow the next method call to dispatch correctly.  It also sets up mode local variables, and other mode-local dispatchers for anything the dispatched call might need.  In this case, the completion call uses lots of other mode local dispatched functions.  with-mode-local doesn't set the major-mode though.  Since all of CEDET is presumably still using mode-local features, you will need both.

I hope this helps.



diff --git a/lisp/cedet/semantic/analyze/complete.el 
index b471c0d1a1..193c975c47 100644
--- a/lisp/cedet/semantic/analyze/complete.el
+++ b/lisp/cedet/semantic/analyze/complete.el
@@ -63,7 +63,7 @@ semantic-analyze-tags-of-class-list
  ;;; MAIN completion calculator
-(define-overloadable-function semantic-analyze-possible-completions (context 
&rest flags)
+(cl-defgeneric semantic-analyze-possible-completions (context &optional flags)
    "Return a list of semantic tags which are possible completions.
  CONTEXT is either a position (such as point), or a precalculated
  context.  Passing in a context is useful if the caller also needs
@@ -83,7 +83,9 @@ semantic-analyze-possible-completions
    * Argument to a function with type constraints.
  When called interactively, displays the list of possible completions
  in a buffer."
-  (interactive "d")
+  (semantic-analyze-possible-completions-default context flags))
+(cl-defmethod semantic-analyze-possible-completions :around (context &rest 
    ;; In theory, we don't need the below since the context will
    ;; do it for us.
@@ -93,8 +95,9 @@ semantic-analyze-possible-completions
                        (semantic-analyze-current-context context)))
             (ans (if (not context)
-                     (error "Nothing to complete")
-                   (:override))))
+                     (when (called-interactively-p 'any)
+                       (error "Nothing to complete"))
+                   (cl-call-next-method))))
        ;; If interactive, display them.
        (when (called-interactively-p 'any)
          (with-output-to-temp-buffer "*Possible Completions*"
diff --git a/lisp/cedet/semantic/bovine/make.el 
index 3676c6972f..dbfa060b5d 100644
--- a/lisp/cedet/semantic/bovine/make.el
+++ b/lisp/cedet/semantic/bovine/make.el
@@ -32,9 +32,6 @@
  (require 'semantic/analyze)
  (require 'semantic/dep)
-(declare-function semantic-analyze-possible-completions-default
-                 "semantic/analyze/complete")
  ;;; Code:
  (define-lex-analyzer semantic-lex-make-backslash-no-newline
    "Detect and create a beginning of line token (BOL)."
@@ -174,13 +171,13 @@ semantic-format-tag-uml-prototype
  This is the same as a regular prototype."
    (semantic-format-tag-prototype tag parent color))
-(define-mode-local-override semantic-analyze-possible-completions
-  makefile-mode (context)
+(cl-defmethod semantic-analyze-possible-completions
+  (context &context (major-mode makefile-mode) &rest _)
    "Return a list of possible completions in a Makefile.
  Uses default implementation, and also gets a list of filenames."
    (require 'semantic/analyze/complete)
    (with-current-buffer (oref context buffer)
-    (let* ((normal (semantic-analyze-possible-completions-default context))
+    (let* ((normal (cl-call-next-method))
           (classes (oref context prefixclass))
           (filetags nil))
        (when (memq 'filename classes)
diff --git a/lisp/cedet/semantic/grammar.el b/lisp/cedet/semantic/grammar.el
index 813580ba6c..6767cb10fc 100644
--- a/lisp/cedet/semantic/grammar.el
+++ b/lisp/cedet/semantic/grammar.el
@@ -1914,13 +1914,15 @@ semantic-analyze-current-context
context-return))) -(define-mode-local-override semantic-analyze-possible-completions
-  semantic-grammar-mode (context)
+(cl-defmethod semantic-analyze-possible-completions
+  (context &context (major-mode semantic-grammar-mode) &rest _)
    "Return a list of possible completions based on CONTEXT."
    (require 'semantic/analyze/complete)
    (if (semantic-grammar-in-lisp-p)
+      ;; FIXME: Does the `let' make the `with-mode-local' redundant here?
        (with-mode-local emacs-lisp-mode
-       (semantic-analyze-possible-completions context))
+       (let ((major-mode 'emacs-lisp-mode))
+         (semantic-analyze-possible-completions context)))
      (with-current-buffer (oref context buffer)
        (let* ((prefix (car (oref context prefix)))
             (completetext (cond ((semantic-tag-p prefix)
diff --git a/lisp/cedet/semantic/texi.el b/lisp/cedet/semantic/texi.el
index 3a0050b920..510c4ea043 100644
--- a/lisp/cedet/semantic/texi.el
+++ b/lisp/cedet/semantic/texi.el
@@ -412,8 +412,8 @@ semantic-texi-command-completion-list
    "List of commands that we might bother completing.")
-(define-mode-local-override semantic-analyze-possible-completions
-  texinfo-mode (context)
+(cl-defmethod semantic-analyze-possible-completions
+  (context &context (major-mode texinfo-mode) &rest _)
    "List smart completions at point.
  Since texinfo is not a programming language the default version is not
  useful.  Instead, look at the current symbol.  If it is a command

reply via email to

[Prev in Thread] Current Thread [Next in Thread]