From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Eric Ludlam Newsgroups: gmane.emacs.devel Subject: Re: completion-at-point + semantic : erroneous error Date: Fri, 11 Oct 2019 21:06:04 -0400 Message-ID: References: <957ad127-0d84-69e3-49b6-9799975bd724@siege-engine.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="186622"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 Cc: Emacs Development To: Stefan Monnier , Eric Ludlam Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Oct 12 03:06:21 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1iJ5rQ-000mNs-0j for ged-emacs-devel@m.gmane.org; Sat, 12 Oct 2019 03:06:20 +0200 Original-Received: from localhost ([::1]:58110 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iJ5rO-0000qX-SJ for ged-emacs-devel@m.gmane.org; Fri, 11 Oct 2019 21:06:18 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:49111) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iJ5rG-0000qM-HK for emacs-devel@gnu.org; Fri, 11 Oct 2019 21:06:11 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iJ5rE-0001dE-S2 for emacs-devel@gnu.org; Fri, 11 Oct 2019 21:06:10 -0400 Original-Received: from mail-qt1-x82c.google.com ([2607:f8b0:4864:20::82c]:34200) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1iJ5rE-0001cN-Nc for emacs-devel@gnu.org; Fri, 11 Oct 2019 21:06:08 -0400 Original-Received: by mail-qt1-x82c.google.com with SMTP id 3so16576732qta.1 for ; Fri, 11 Oct 2019 18:06:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding:content-language; bh=HB9+xovpiQNhK8axtxwqTOjmFJWK4Wpu7ghpFNWxZNc=; b=CoT9uD5bYKlJ2Ajm73afRS5YDPn9emEacHiRTVE57OopggRuFFFntwBmHRk3MxN3JX xHGGeDdQiP/l6w2swu0NCLdeiXtlvoqUSz6vRt+dftbm9gEm1+WjrUIQBgfkEQsMgMa1 2u7vBSgvWvXVxvjziLWgtDWdGXqnhVcXL1nWllV8KX5epeJbpFJt1AT8Z4p7SW6j1d4q dhmaSLMcVAkqpg5syOnmHd4bYmgux1BlM4Egi6z0n9ksgxcu+AScKpYs14ZWZUiID7sa Cp9vPtdxN+kTQL+wb/SSZNZRDyRITfqPTm7VV4VSSryKCW0IQxF2ApENVp6qL9Lb6Drb KXaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=HB9+xovpiQNhK8axtxwqTOjmFJWK4Wpu7ghpFNWxZNc=; b=GIEQI9c9ainwUBHHv7k1ygVpm+Ec2qmKRsA+gvSfOHvinnNGjaDMTBaDBGG82PQ1BU T/WTMcLxeZDwbQeGsF6y6EeN2I5pz0z8D0mm76T31LUTLeOkSBHn7qmVH3C6ZSDCCloW dx7TMJAKvOo3Va1ep2bTMrxJAYMdPFbk+e6wEIYnIJuqiZm83KODxSyyew4K38CVvIXM L2gskmLcKBQSTQ5wAzEJ/8s3maRngZXVKpYP7iTwbluD/+WFvX+vYFFry9OA80sMisxA ZcHBQz9lYyXyvmCOimAAj7GdG1Ot6MpTfc4v0uV2XrpL/ZRh8x3lX7gIhtGyfFrqsIi0 gmFA== X-Gm-Message-State: APjAAAXoaWZ8xxuV7sT/syvNIuGmikWWKBPAobB3y04jdH61efiioaTs SqYdo41koArxU3855OYVPIsYMHuiXwQ= X-Google-Smtp-Source: APXvYqyFVkkAYkzJGCkWmn3DOCqIhWS80FDO2TpmvC0vEd4bCPCXGJXh2qnli7Gsx0U1LU7X75pSlQ== X-Received: by 2002:ac8:73cf:: with SMTP id v15mr19576369qtp.310.1570842366496; Fri, 11 Oct 2019 18:06:06 -0700 (PDT) Original-Received: from [192.168.1.202] (pool-108-20-30-136.bstnma.fios.verizon.net. [108.20.30.136]) by smtp.googlemail.com with ESMTPSA id 14sm4461388qtb.54.2019.10.11.18.06.04 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 11 Oct 2019 18:06:05 -0700 (PDT) In-Reply-To: Content-Language: en-US X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4864:20::82c X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:240900 Archived-At: 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. Eric > > Stefan > > > diff --git a/lisp/cedet/semantic/analyze/complete.el b/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 > ;; > ;;;###autoload > -(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 flags) > ;; In theory, we don't need the below since the context will > ;; do it for us. > ;;(semantic-refresh-tags-safe) > @@ -93,8 +95,9 @@ semantic-analyze-possible-completions > context > (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 b/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 > >