From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: Thoughts on Refactoring In-Buffer Completion In message.el Date: Tue, 16 Aug 2022 22:45:08 -0400 Message-ID: References: <82e662dc46347e410c0b1d871e998b00@condition-alpha.com> <5b04a4a1f5a497f030397f6813551ad2@condition-alpha.com> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="19488"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: emacs-devel@gnu.org To: Alexander Adolf Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Wed Aug 17 04:47:43 2022 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oO95q-0004lQ-Ao for ged-emacs-devel@m.gmane-mx.org; Wed, 17 Aug 2022 04:47:42 +0200 Original-Received: from localhost ([::1]:43918 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oO95o-0003Pf-Nb for ged-emacs-devel@m.gmane-mx.org; Tue, 16 Aug 2022 22:47:40 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:55434) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oO93q-0002d0-JV for emacs-devel@gnu.org; Tue, 16 Aug 2022 22:45:41 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:60100) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oO93n-0001Bv-D1 for emacs-devel@gnu.org; Tue, 16 Aug 2022 22:45:37 -0400 Original-Received: from pmg2.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 127A88079C; Tue, 16 Aug 2022 22:45:14 -0400 (EDT) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 5A581804AB; Tue, 16 Aug 2022 22:45:11 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1660704311; bh=V0knpTSvM36oGUVc8NZKpqa5AjiwLF3FunIj86KjKJQ=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=OEllFANbvbKNAfcMZL92rtZ91netsakqGWWsw2Cw1A+IhWjZ8V6/DuSuoCPnJOvX6 zgauygl1GldI++7zyH4Nvup/jP09IgXTyRkmTz1epJB9GbZk0xJjMA/qUr1Ggu/xnQ gRopR9k22VAZRMnrRE/OI4/eD91YshEl1hkQd7lkYJx9F/kZviEArZkG85x/C+xR7I OHoBsn9QQWM2wKdLmDYxzdLWAFILQGPuLm+4Pg88GSRrJu6gn2Xsvok5fuplw0NMb8 XdruEgu3qGRDrd8rQDz/TAeo3DodIR6xldj+fAfp4YTAghtVgWdOzP1/PxYk+Tir/t KZL3alm+EIWLw== Original-Received: from pastel (unknown [45.72.195.111]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 2FDD4120234; Tue, 16 Aug 2022 22:45:11 -0400 (EDT) In-Reply-To: (Alexander Adolf's message of "Wed, 27 Jul 2022 23:16:48 +0200") Received-SPF: pass client-ip=132.204.25.50; envelope-from=monnier@iro.umontreal.ca; helo=mailscanner.iro.umontreal.ca X-Spam_score_int: -42 X-Spam_score: -4.3 X-Spam_bar: ---- X-Spam_report: (-4.3 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 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-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:293531 Archived-At: > +When RECIPE is a function, it is called for completion. RECIPE > +should be a function that obeys the same rules as those of > +`completion-at-point-functions'. Right. > +When RECIPE is a plist, the stored properties are used to control [ Remove "stored" and "are used to". ] > +how in-buffer completion is performed. The following properties > +are currently defined: > + > +:category > + > + The symbol defining the category in > + `completion-category-defaults' to use for completion. Also > + see `completion-category-overrides', and `completion-styles'. > + > +:fieldsep-re > + > + The regular expression to use when scanning backwards in the > + buffer. All text between point, and any preceding text > + matching this regular expression, will be used as the prefix > + for finding completion candidates. I don't think we should limit ourselves to completion based on a prefix. IOW, when I have: To: aladol where represents the position of the cursor, we should consider "aldol" as the string we're trying to complete (knowing that we're trying to complete it from the middle). `completion-at-point-functions` already supports this just fine. All we have to do is to look both backward and forward to find the beginning and the end, rather than only looking backward to find the beginning and always using `point` as the end. You currently use "\\([:,]\\|^\\)[ \t]*" as the regexp, but the ":" is redundant because the code should already take the end of header's regexp match as a limit on the lefthand side. And we should strip the spaces by hand anyway, so the regexp should just be "," (and we should default to that if the property is not provided, as will presumably always be the case). > +:completions > + > + The function that provides completions, and that obeys the > + same rules as those of `completion-at-point-functions'. I think you forgot to update this part of the docstring: it should say that this is a completion table rather than a function that obeys the rules of `completion-at-point-functions`. > +(defun message-completion-alist-set-completions (cat compl) > + "Set the completion function for category CAT to COMPL. > +Modifies the value of `message-completion-alist'. This is a > +convenience function for use in init files." Hmm... I'm not sure it's really convenient. IIUC this is intended for users to say "use EUDC for email completions instead"? If so, maybe we should support this more directly so users only need to set a simple variable to get that result? [ Then again, we already have `message-expand-name-databases` for that, so maybe you have some other use case in mind? ] > + (let* ((completions (plist-get recipe :completions)) > + (beg (save-excursion > + (re-search-backward (plist-get recipe :fieldsep-re)) Usually properties in plists are optional. Admittedly, it's not always the case, but I'd recommend signaling more explicit errors when a property is missing. Here if `:fieldsep-re` is missing we'll just get an error about nil not being a string without any hint where this nil comes from (and the naive users may end up in a wild goose chase if they try to search the web for "how to fix: wrong-type-argument stringp nil"). IOW I'd do something like (or (plist-get recipe :completions) (error "Missing :completions in %S" recipe)) Same for `:fieldsep-re` (not for :category since a nil category seems like a fine default), unless you make it default to ",". > + (match-end 0))) > + (end (point)) > + (cat (plist-get recipe :category)) > + completion-table) > + (setq completion-table (if (functionp completions) > + (funcall completions beg end) > + completions)) Hmm... why do you call `completions` here? Just use the function itself as the completion table. The completion machinery will call it if/when/as needed. > -(defun message-expand-group () > +(defun message-expand-group (&optional pfx-beg pfx-end) IIUC the completion table for newgroups should be simply `gnus-active-hashtb` instead of #'message-expand-group (which is not a valid completion table). `message-expand-group` should then be marked as obsolete. > -(defun message-expand-name () > +(defun message-expand-name (&optional pfx-beg pfx-end) > (cond (message-expand-name-standard-ui > - (let ((beg (save-excursion > - (skip-chars-backward "^\n:,") (skip-chars-forward " \t") > - (point))) > - (end (save-excursion > - (skip-chars-forward "^\n,") (skip-chars-backward " \t") > - (point)))) > + (let ((beg (or pfx-beg (save-excursion > + (skip-chars-backward "^\n:,") > + (skip-chars-forward " \t") > + (point)))) > + (end (or pfx-end (save-excursion > + (skip-chars-forward "^\n,") > + (skip-chars-backward " \t") > + (point))))) > (when (< beg end) > (list beg end (message--name-table (buffer-substring beg end)))))) > ((and (memq 'eudc message-expand-name-databases) Again, this is not a completion table. A completion table for names could be: (completion-table-dynamic #'message--name-table) or maybe (completion-table-with-cache #'message--name-table) > @@ -8401,7 +8479,12 @@ message--name-table > bbdb-responses) > (lambda (string pred action) > (pcase action > - ('metadata '(metadata (category . email))) > + ('metadata (let* ((recipe (alist-get message-email-recipient-header-regexp > + message-completion-alist)) > + (cat (plist-get recipe :category))) > + (pcase recipe > + ((pred functionp) '(metadata (category . email))) > + (_ (when cat `(metadata (category . ,cat))))))) This is pretty hideous. Better not do that, and let the other (currently commented out) part of the code add the metadata from the plist when we already have that plist info at hand. > I have also removed message-expand-name, and message--name-table, and > instead am calling out to EUDC. EUDC is enhanced by two new backends for > ecomplete, and mailabbrev. Thus, in terms of functionality, end users > should see no difference. To use the new EUDC back-ends you'll need to > do one or both of: > > (require 'eudcb-ecomplete) > (add-to-list 'eudc-server-hotlist '("localhost" . ecomplete)) > > (require 'eudcb-mailabbrev) > (add-to-list 'eudc-server-hotlist '("localhost" . mailabbrev)) IOW users *will* see a (bad) difference because their setup will not work as before until they add those lines to their init file :-( > +(defcustom message-mail-address-snarf-hook nil > + "Hook run to snarf email addresses. > +This hook is run just after the message was sent as mail. > + > +The functions on this hook are called once for each header line > +where email addresses were found. They take a single argument, a > +list of cons cells as returned by `mail-header-parse-addresses'. The `-hook` suffix is used only for "normal hooks", i.e. hooks run by `run-hooks`. So this should use a name that ends in `-functions` instead, which is the suffix normally used for those so-called "abnormal hooks", i.e. those that take a non-empty list of arguments or whose return value is not ignored. This hook should probably include a function for ecomplete in its default value, so as to preserve the existing behavior. Stefan