Re: [elpa] externals/ebdb f7b50402cf: Final round of compiler quieting

Re: [elpa] externals/ebdb f7b50402cf: Final round of compiler quieting

[Stefan Monnier]

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

>     Not all the way there, but I don't know what to do about the format
>     complaints.

The suspect that problem is that your definition of `ebdb-add-to-list`
duplicates its argument, so

    (ebdb-add-to-list X (format FOO BAR))

becomes

    (when (format FOO BAR) (push (format FOO BAR) X))

Since `format` always returns a string, the `when` can be optimized away
so the above becomes:

    (progn (format FOO BAR) (push (format FOO BAR) X))

Beside the useless call to `format` there are other minor problems with
this definition of `ebdb-add-to-list`, such as the fact that its
arguments are not evaluated in the expected order (left to right), and
the fact that the second argument gets evaluated twice, so:

    (ebdb-add-to-list (progn (message "hello") 'x)
                      (progn (message "world") 3))

ends up adding 3 to x and emitting

    world
    world
    hello

instead of

    hello
    world

But the more serious problem is that `ebdb-add-to-list` is defined as
a function yet it can't work as a function because its first argument is
expected to be a "place" not a value.
So you have to define it as a macro rather than a (define-inline) function.

Also, personally I'd recommend you rename the to use a word like "push"
(and to switch its args) rather than `add-to-list`, so the reader can
guess that it takes an place, like `(cl-)push(new)`, rather than
a symbol, like `add-to-list`.


        Stefan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ebdb.patch --]
[-- Type: text/x-diff, Size: 9270 bytes --]

diff --git a/ebdb-complete.el b/ebdb-complete.el
index 444af05c30..220de71602 100644
--- a/ebdb-complete.el
+++ b/ebdb-complete.el
@@ -81,7 +81,7 @@
 
 (define-obsolete-function-alias
   'ebdb-complete-quit-window
-  'ebdb-complete-bury-ebdb-buffer "1.0.0")
+  #'ebdb-complete-bury-ebdb-buffer "1.0.0")
 
 (defun ebdb-complete-bury-ebdb-buffer ()
   "Hide EBDB buffer and switch to message buffer.
@@ -107,7 +107,7 @@ Before switch, this command will do some clean jobs."
 
 (define-obsolete-function-alias
   'ebdb-complete-push-mail-and-quit-window
-  'ebdb-complete-push-mail-and-bury-ebdb-buffer "1.0.0")
+  #'ebdb-complete-push-mail-and-bury-ebdb-buffer "1.0.0")
 
 (defun ebdb-complete-push-mail-and-bury-ebdb-buffer ()
   "Push email-address to Message window and hide EBDB buffer."
@@ -201,18 +201,18 @@ when in message body, this command will indent regular text."
 
 (defun ebdb-complete-keybinding-setup ()
   "Setup ebdb-complete Keybindings."
-  (define-key ebdb-mode-map "q" 'ebdb-complete-bury-ebdb-buffer)
-  (define-key ebdb-mode-map "\C-c\C-c" 'ebdb-complete-push-mail)
-  (define-key ebdb-mode-map (kbd "RET") 'ebdb-complete-push-mail-and-bury-ebdb-buffer))
+  (define-key ebdb-mode-map "q" #'ebdb-complete-bury-ebdb-buffer)
+  (define-key ebdb-mode-map "\C-c\C-c" #'ebdb-complete-push-mail)
+  (define-key ebdb-mode-map (kbd "RET") #'ebdb-complete-push-mail-and-bury-ebdb-buffer))
 
 ;;;###autoload
 (defun ebdb-complete-enable ()
   "Enable ebdb-complete, it will rebind TAB key in `message-mode-map'."
   (interactive)
   (require 'message)
-  (add-hook 'ebdb-mode-hook 'ebdb-complete-keybinding-setup)
-  (define-key message-mode-map "\t" 'ebdb-complete-message-tab)
-  (define-key mail-mode-map "\t" 'ebdb-complete-message-tab)
+  (add-hook 'ebdb-mode-hook #'ebdb-complete-keybinding-setup)
+  (define-key message-mode-map "\t" #'ebdb-complete-message-tab)
+  (define-key mail-mode-map "\t" #'ebdb-complete-message-tab)
   (message "ebdb-complete: Override EBDB keybindings: `q', `C-c C-c' and `RET'"))
 
 (provide 'ebdb-complete)
diff --git a/ebdb-i18n.el b/ebdb-i18n.el
index 49175d264e..5f24f3d7bf 100644
--- a/ebdb-i18n.el
+++ b/ebdb-i18n.el
@@ -51,8 +51,7 @@
 (defcustom ebdb-i18n-ignorable-scripts '(latin)
   "A list of script types that should be considered \"standard\".
 That is, no special handling will be done for them."
-  :type '(repeat :tag "Script type" symbol)
-  :group 'ebdb-i18n)
+  :type '(repeat :tag "Script type" symbol))
 
 (defcustom ebdb-i18ngroup-countries-pref-scripts nil
   "An alist equivalent to `ebdb-i18n-countries', but in alternate scripts.
@@ -66,8 +65,7 @@ Any country name listed here will be offered along with the
 English version for completion, and will be preferred over the
 English version for display."
 
-  :type '(repeat (cons string string))
-  :group 'ebdb-i18n)
+  :type '(repeat (cons string string)))
 
 ;; defvars come first to pacify compiler.
 
diff --git a/ebdb-message.el b/ebdb-message.el
index a32ae4e63c..7073dea3c0 100644
--- a/ebdb-message.el
+++ b/ebdb-message.el
@@ -64,7 +64,6 @@ configurations.
 
 Note that this should be a different symbol from that used in
 Gnus's article-reading config."
-  :group 'ebdb-mua-message
   :type '(choice (const nil)
 		 (symbol :tag "Window config name")))
 
@@ -167,7 +166,7 @@ Also fires when postponing a draft."
 		   #'ebdb-message-complete-mail-cleanup
 		   nil t)))
     ('nil nil)
-    (_ (define-key mail-mode-map "\M-\t" 'ebdb-complete-mail)))
+    (_ (define-key mail-mode-map "\M-\t" #'ebdb-complete-mail)))
 
   (when ebdb-message-window-configuration
     (add-to-list 'gnus-window-to-buffer
diff --git a/ebdb-mhe.el b/ebdb-mhe.el
index 57268fb718..9bc9c5450b 100644
--- a/ebdb-mhe.el
+++ b/ebdb-mhe.el
@@ -28,7 +28,6 @@
 (require 'mh-e)
 (if (fboundp 'mh-version)
     (require 'mh-comp))              ; For mh-e 4.x
-(require 'advice)
 
 (defgroup ebdb-mua-mhe nil
   "EBDB customizations for mhe."
@@ -144,16 +143,16 @@ Returns the empty string if HEADER is not in the message."
     (ebdb-load))
   (define-key mh-folder-mode-map ";" ebdb-mua-keymap)
   (when ebdb-complete-mail
-    (define-key mh-letter-mode-map "\M-;" 'ebdb-complete-mail)
-    (define-key mh-letter-mode-map "\e\t" 'ebdb-complete-mail)))
+    (define-key mh-letter-mode-map "\M-;" #'ebdb-complete-mail)
+    (define-key mh-letter-mode-map "\e\t" #'ebdb-complete-mail)))
 
 ;;;###autoload
 (defun ebdb-mhe-auto-update ()
   (ebdb-mua-auto-update ebdb-mhe-auto-update-p))
 
-(add-hook 'mh-show-hook 'ebdb-mhe-auto-update)
+(add-hook 'mh-show-hook #'ebdb-mhe-auto-update)
 
-(add-hook 'mh-folder-mode-hook 'ebdb-insinuate-mh)
+(add-hook 'mh-folder-mode-hook #'ebdb-insinuate-mh)
 
 (provide 'ebdb-mhe)
 ;;; ebdb-mhe.el ends here
diff --git a/ebdb-rmail.el b/ebdb-rmail.el
index b86d12cb15..4a4ed59675 100644
--- a/ebdb-rmail.el
+++ b/ebdb-rmail.el
@@ -90,9 +90,9 @@ the value of `ebdb-default-window-size'."
 (defun ebdb-rmail-auto-update ()
   (ebdb-mua-auto-update ebdb-rmail-auto-update-p))
 
-(add-hook 'rmail-mode-hook 'ebdb-insinuate-rmail)
+(add-hook 'rmail-mode-hook #'ebdb-insinuate-rmail)
 
-(add-hook 'rmail-show-message-hook 'ebdb-rmail-auto-update)
+(add-hook 'rmail-show-message-hook #'ebdb-rmail-auto-update)
 
 (provide 'ebdb-rmail)
 ;;; ebdb-rmail.el ends here
diff --git a/ebdb-vm.el b/ebdb-vm.el
index cdac018046..d7cbfd0cf2 100644
--- a/ebdb-vm.el
+++ b/ebdb-vm.el
@@ -118,26 +118,22 @@ the value of `ebdb-default-window-size'."
 (defcustom ebdb/vm-auto-folder-headers '("From:" "To:" "CC:")
   "The headers used by `ebdb/vm-auto-folder'.
 The order in this list is the order how matching will be performed."
-  :group 'ebdb-mua-vm
   :type '(repeat (string :tag "header name")))
 
 ;;;###autoload
 (defcustom ebdb/vm-auto-folder-field "vm-folder"
   "The xfield which `ebdb/vm-auto-folder' searches for."
-  :group 'ebdb-mua-vm
   :type 'symbol)
 
 ;;;###autoload
 (defcustom ebdb/vm-virtual-folder-field "vm-virtual"
   "The xfield which `ebdb/vm-virtual-folder' searches for."
-  :group 'ebdb-mua-vm
   :type 'symbol)
 
 ;;;###autoload
 (defcustom ebdb/vm-virtual-real-folders nil
   "Real folders used for defining virtual folders.
 If nil use `vm-primary-inbox'."
-  :group 'ebdb-mua-vm
   :type '(choice (const :tag "Use vm-primary-inbox" nil)
                  (repeat (string :tag "Real folder"))))
 
@@ -255,7 +251,6 @@ Its elements may be strings used both as the field value to check for
 and as the label to apply to the message.
 If an element is a cons pair (VALUE . LABEL), VALUE is the field value
 to search for and LABEL is the label to apply."
-  :group 'ebdb-mua-vm
   :type '(repeat string))
 
 (defcustom ebdb/vm-auto-add-label-field 'ebdb-mail-alias-field
@@ -263,7 +258,6 @@ to search for and LABEL is the label to apply."
 This is either a single EBDB field or a list of fields that
 `ebdb/vm-auto-add-label' uses to check for labels to apply to a message.
 Defaults to `ebdb-mail-alias-field' which defaults to `mail-alias'."
-  :group 'ebdb-mua-vm
   :type '(choice symbol (repeat symbol)))
 
 (defun ebdb/vm-auto-add-label (record)
@@ -310,7 +304,7 @@ from different senders."
                             ebdb/vm-auto-add-label-list))))
         (if labels
             (vm-add-message-labels
-             (mapconcat 'identity labels " ") 1)))))
+             (mapconcat #'identity labels " ") 1)))))
 
 \f
 
@@ -372,11 +366,11 @@ from different senders."
   (unless ebdb-db-list
     (ebdb-load))
   (define-key vm-mode-map ";" ebdb-mua-keymap)
-  (define-key vm-mode-map "/" 'ebdb)
+  (define-key vm-mode-map "/" #'ebdb)
   ;; `mail-mode-map' is the parent of `vm-mail-mode-map'.
   ;; So the following is also done by `ebdb-insinuate-mail'.
   (if (and ebdb-complete-mail (boundp 'vm-mail-mode-map))
-      (define-key vm-mail-mode-map "\M-\t" 'ebdb-complete-mail))
+      (define-key vm-mail-mode-map "\M-\t" #'ebdb-complete-mail))
 
   ;; Set up user field for use in `vm-summary-format'
   ;; (1) Big solution: use whole name
diff --git a/ebdb.el b/ebdb.el
index e4861c2756..d73f82752a 100644
--- a/ebdb.el
+++ b/ebdb.el
@@ -916,17 +916,19 @@ You really should not disable debugging.  But it will speed things up."
 ;; directly.  But presumably we could replace all the object-*
 ;; functions with the ebdb-* inlines.
 
-(define-inline ebdb-add-to-list (list-var element)
+(defmacro ebdb-add-to-list (list-var element) ;Rename to *push* and swap args?
   "Add ELEMENT to the value of LIST-VAR if it isn't there yet and non-nil.
 The test for presence of ELEMENT is done with `equal'."
-  (inline-quote (when ,element (cl-pushnew ,element ,list-var :test #'equal))))
+  (macroexp-let2 nil element element
+    `(when ,element (cl-pushnew ,element ,list-var :test #'equal))))
 
-(define-inline ebdb-remove-from-list (list-var element)
+(defmacro ebdb-remove-from-list (list-var element) ;Rename and swap args?
   "Remove ELEMENT from LIST-VAR, if present.
 Test for presence is done with `equal'."
-  (inline-quote (when (and ,element ,list-var)
-		  (setf ,list-var
-			(delete ,element ,list-var)))))
+  (macroexp-let2 nil element element
+    `(when ,element
+       ,(gv-letplace (getter setter) list-var
+	  (funcall setter `(delete ,element ,getter))))))
 
 ;;; Struct and object definitions.
 

Re: [elpa] externals/ebdb f7b50402cf: Final round of compiler quieting

[Eric Abrahamsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>     Not all the way there, but I don't know what to do about the format
>>     complaints.
>
> The suspect that problem is that your definition of `ebdb-add-to-list`
> duplicates its argument, so
>
>     (ebdb-add-to-list X (format FOO BAR))
>
> becomes
>
>     (when (format FOO BAR) (push (format FOO BAR) X))
>
> Since `format` always returns a string, the `when` can be optimized away
> so the above becomes:
>
>     (progn (format FOO BAR) (push (format FOO BAR) X))
>
> Beside the useless call to `format` there are other minor problems with
> this definition of `ebdb-add-to-list`, such as the fact that its
> arguments are not evaluated in the expected order (left to right), and
> the fact that the second argument gets evaluated twice, so:
>
>     (ebdb-add-to-list (progn (message "hello") 'x)
>                       (progn (message "world") 3))
>
> ends up adding 3 to x and emitting
>
>     world
>     world
>     hello
>
> instead of
>
>     hello
>     world
>
> But the more serious problem is that `ebdb-add-to-list` is defined as
> a function yet it can't work as a function because its first argument is
> expected to be a "place" not a value.
> So you have to define it as a macro rather than a (define-inline) function.
>
> Also, personally I'd recommend you rename the to use a word like "push"
> (and to switch its args) rather than `add-to-list`, so the reader can
> guess that it takes an place, like `(cl-)push(new)`, rather than
> a symbol, like `add-to-list`.

Thank you very much for the detailed explanation, and for the patch!
This is an area of Elisp I still don't have much feel for, and I don't
think would have come up with the right gv-/macroexp- invocations
myself.

I'll get this released in the next few days.

Thanks again,
Eric

Re: [elpa] externals/ebdb f7b50402cf: Final round of compiler quieting

[Eric Abrahamsen]

On 10/23/23 22:40 PM, Stefan Monnier wrote:
>>     Not all the way there, but I don't know what to do about the format
>>     complaints.
>
> The suspect that problem is that your definition of `ebdb-add-to-list`
> duplicates its argument, so
>
>     (ebdb-add-to-list X (format FOO BAR))
>
> becomes
>
>     (when (format FOO BAR) (push (format FOO BAR) X))
>
> Since `format` always returns a string, the `when` can be optimized away
> so the above becomes:
>
>     (progn (format FOO BAR) (push (format FOO BAR) X))
>
> Beside the useless call to `format` there are other minor problems with
> this definition of `ebdb-add-to-list`, such as the fact that its
> arguments are not evaluated in the expected order (left to right), and
> the fact that the second argument gets evaluated twice, so:
>
>     (ebdb-add-to-list (progn (message "hello") 'x)
>                       (progn (message "world") 3))
>
> ends up adding 3 to x and emitting
>
>     world
>     world
>     hello
>
> instead of
>
>     hello
>     world
>
> But the more serious problem is that `ebdb-add-to-list` is defined as
> a function yet it can't work as a function because its first argument is
> expected to be a "place" not a value.
> So you have to define it as a macro rather than a (define-inline) function.
>
> Also, personally I'd recommend you rename the to use a word like "push"
> (and to switch its args) rather than `add-to-list`, so the reader can
> guess that it takes an place, like `(cl-)push(new)`, rather than
> a symbol, like `add-to-list`.
>
>
>         Stefan
>
> diff --git a/ebdb-complete.el b/ebdb-complete.el
> index 444af05c30..220de71602 100644
> --- a/ebdb-complete.el
> +++ b/ebdb-complete.el
> @@ -81,7 +81,7 @@
>  
>  (define-obsolete-function-alias
>    'ebdb-complete-quit-window
> -  'ebdb-complete-bury-ebdb-buffer "1.0.0")
> +  #'ebdb-complete-bury-ebdb-buffer "1.0.0")
>  
>  (defun ebdb-complete-bury-ebdb-buffer ()
>    "Hide EBDB buffer and switch to message buffer.
> @@ -107,7 +107,7 @@ Before switch, this command will do some clean jobs."
>  
>  (define-obsolete-function-alias
>    'ebdb-complete-push-mail-and-quit-window
> -  'ebdb-complete-push-mail-and-bury-ebdb-buffer "1.0.0")
> +  #'ebdb-complete-push-mail-and-bury-ebdb-buffer "1.0.0")
>  
>  (defun ebdb-complete-push-mail-and-bury-ebdb-buffer ()
>    "Push email-address to Message window and hide EBDB buffer."
> @@ -201,18 +201,18 @@ when in message body, this command will indent regular text."
>  
>  (defun ebdb-complete-keybinding-setup ()
>    "Setup ebdb-complete Keybindings."
> -  (define-key ebdb-mode-map "q" 'ebdb-complete-bury-ebdb-buffer)
> -  (define-key ebdb-mode-map "\C-c\C-c" 'ebdb-complete-push-mail)
> -  (define-key ebdb-mode-map (kbd "RET") 'ebdb-complete-push-mail-and-bury-ebdb-buffer))
> +  (define-key ebdb-mode-map "q" #'ebdb-complete-bury-ebdb-buffer)
> +  (define-key ebdb-mode-map "\C-c\C-c" #'ebdb-complete-push-mail)
> +  (define-key ebdb-mode-map (kbd "RET") #'ebdb-complete-push-mail-and-bury-ebdb-buffer))
>  
>  ;;;###autoload
>  (defun ebdb-complete-enable ()
>    "Enable ebdb-complete, it will rebind TAB key in `message-mode-map'."
>    (interactive)
>    (require 'message)
> -  (add-hook 'ebdb-mode-hook 'ebdb-complete-keybinding-setup)
> -  (define-key message-mode-map "\t" 'ebdb-complete-message-tab)
> -  (define-key mail-mode-map "\t" 'ebdb-complete-message-tab)
> +  (add-hook 'ebdb-mode-hook #'ebdb-complete-keybinding-setup)
> +  (define-key message-mode-map "\t" #'ebdb-complete-message-tab)
> +  (define-key mail-mode-map "\t" #'ebdb-complete-message-tab)
>    (message "ebdb-complete: Override EBDB keybindings: `q', `C-c C-c' and `RET'"))
>  
>  (provide 'ebdb-complete)
> diff --git a/ebdb-i18n.el b/ebdb-i18n.el
> index 49175d264e..5f24f3d7bf 100644
> --- a/ebdb-i18n.el
> +++ b/ebdb-i18n.el
> @@ -51,8 +51,7 @@
>  (defcustom ebdb-i18n-ignorable-scripts '(latin)
>    "A list of script types that should be considered \"standard\".
>  That is, no special handling will be done for them."
> -  :type '(repeat :tag "Script type" symbol)
> -  :group 'ebdb-i18n)
> +  :type '(repeat :tag "Script type" symbol))
>  
>  (defcustom ebdb-i18ngroup-countries-pref-scripts nil
>    "An alist equivalent to `ebdb-i18n-countries', but in alternate scripts.
> @@ -66,8 +65,7 @@ Any country name listed here will be offered along with the
>  English version for completion, and will be preferred over the
>  English version for display."
>  
> -  :type '(repeat (cons string string))
> -  :group 'ebdb-i18n)
> +  :type '(repeat (cons string string)))
>  
>  ;; defvars come first to pacify compiler.
>  
> diff --git a/ebdb-message.el b/ebdb-message.el
> index a32ae4e63c..7073dea3c0 100644
> --- a/ebdb-message.el
> +++ b/ebdb-message.el
> @@ -64,7 +64,6 @@ configurations.
>  
>  Note that this should be a different symbol from that used in
>  Gnus's article-reading config."
> -  :group 'ebdb-mua-message
>    :type '(choice (const nil)
>  		 (symbol :tag "Window config name")))
>  
> @@ -167,7 +166,7 @@ Also fires when postponing a draft."
>  		   #'ebdb-message-complete-mail-cleanup
>  		   nil t)))
>      ('nil nil)
> -    (_ (define-key mail-mode-map "\M-\t" 'ebdb-complete-mail)))
> +    (_ (define-key mail-mode-map "\M-\t" #'ebdb-complete-mail)))
>  
>    (when ebdb-message-window-configuration
>      (add-to-list 'gnus-window-to-buffer
> diff --git a/ebdb-mhe.el b/ebdb-mhe.el
> index 57268fb718..9bc9c5450b 100644
> --- a/ebdb-mhe.el
> +++ b/ebdb-mhe.el
> @@ -28,7 +28,6 @@
>  (require 'mh-e)
>  (if (fboundp 'mh-version)
>      (require 'mh-comp))              ; For mh-e 4.x
> -(require 'advice)
>  
>  (defgroup ebdb-mua-mhe nil
>    "EBDB customizations for mhe."
> @@ -144,16 +143,16 @@ Returns the empty string if HEADER is not in the message."
>      (ebdb-load))
>    (define-key mh-folder-mode-map ";" ebdb-mua-keymap)
>    (when ebdb-complete-mail
> -    (define-key mh-letter-mode-map "\M-;" 'ebdb-complete-mail)
> -    (define-key mh-letter-mode-map "\e\t" 'ebdb-complete-mail)))
> +    (define-key mh-letter-mode-map "\M-;" #'ebdb-complete-mail)
> +    (define-key mh-letter-mode-map "\e\t" #'ebdb-complete-mail)))
>  
>  ;;;###autoload
>  (defun ebdb-mhe-auto-update ()
>    (ebdb-mua-auto-update ebdb-mhe-auto-update-p))
>  
> -(add-hook 'mh-show-hook 'ebdb-mhe-auto-update)
> +(add-hook 'mh-show-hook #'ebdb-mhe-auto-update)
>  
> -(add-hook 'mh-folder-mode-hook 'ebdb-insinuate-mh)
> +(add-hook 'mh-folder-mode-hook #'ebdb-insinuate-mh)
>  
>  (provide 'ebdb-mhe)
>  ;;; ebdb-mhe.el ends here
> diff --git a/ebdb-rmail.el b/ebdb-rmail.el
> index b86d12cb15..4a4ed59675 100644
> --- a/ebdb-rmail.el
> +++ b/ebdb-rmail.el
> @@ -90,9 +90,9 @@ the value of `ebdb-default-window-size'."
>  (defun ebdb-rmail-auto-update ()
>    (ebdb-mua-auto-update ebdb-rmail-auto-update-p))
>  
> -(add-hook 'rmail-mode-hook 'ebdb-insinuate-rmail)
> +(add-hook 'rmail-mode-hook #'ebdb-insinuate-rmail)
>  
> -(add-hook 'rmail-show-message-hook 'ebdb-rmail-auto-update)
> +(add-hook 'rmail-show-message-hook #'ebdb-rmail-auto-update)
>  
>  (provide 'ebdb-rmail)
>  ;;; ebdb-rmail.el ends here
> diff --git a/ebdb-vm.el b/ebdb-vm.el
> index cdac018046..d7cbfd0cf2 100644
> --- a/ebdb-vm.el
> +++ b/ebdb-vm.el
> @@ -118,26 +118,22 @@ the value of `ebdb-default-window-size'."
>  (defcustom ebdb/vm-auto-folder-headers '("From:" "To:" "CC:")
>    "The headers used by `ebdb/vm-auto-folder'.
>  The order in this list is the order how matching will be performed."
> -  :group 'ebdb-mua-vm
>    :type '(repeat (string :tag "header name")))
>  
>  ;;;###autoload
>  (defcustom ebdb/vm-auto-folder-field "vm-folder"
>    "The xfield which `ebdb/vm-auto-folder' searches for."
> -  :group 'ebdb-mua-vm
>    :type 'symbol)
>  
>  ;;;###autoload
>  (defcustom ebdb/vm-virtual-folder-field "vm-virtual"
>    "The xfield which `ebdb/vm-virtual-folder' searches for."
> -  :group 'ebdb-mua-vm
>    :type 'symbol)
>  
>  ;;;###autoload
>  (defcustom ebdb/vm-virtual-real-folders nil
>    "Real folders used for defining virtual folders.
>  If nil use `vm-primary-inbox'."
> -  :group 'ebdb-mua-vm
>    :type '(choice (const :tag "Use vm-primary-inbox" nil)
>                   (repeat (string :tag "Real folder"))))
>  
> @@ -255,7 +251,6 @@ Its elements may be strings used both as the field value to check for
>  and as the label to apply to the message.
>  If an element is a cons pair (VALUE . LABEL), VALUE is the field value
>  to search for and LABEL is the label to apply."
> -  :group 'ebdb-mua-vm
>    :type '(repeat string))
>  
>  (defcustom ebdb/vm-auto-add-label-field 'ebdb-mail-alias-field
> @@ -263,7 +258,6 @@ to search for and LABEL is the label to apply."
>  This is either a single EBDB field or a list of fields that
>  `ebdb/vm-auto-add-label' uses to check for labels to apply to a message.
>  Defaults to `ebdb-mail-alias-field' which defaults to `mail-alias'."
> -  :group 'ebdb-mua-vm
>    :type '(choice symbol (repeat symbol)))
>  
>  (defun ebdb/vm-auto-add-label (record)
> @@ -310,7 +304,7 @@ from different senders."
>                              ebdb/vm-auto-add-label-list))))
>          (if labels
>              (vm-add-message-labels
> -             (mapconcat 'identity labels " ") 1)))))
> +             (mapconcat #'identity labels " ") 1)))))
>  
>  \f
>  
> @@ -372,11 +366,11 @@ from different senders."
>    (unless ebdb-db-list
>      (ebdb-load))
>    (define-key vm-mode-map ";" ebdb-mua-keymap)
> -  (define-key vm-mode-map "/" 'ebdb)
> +  (define-key vm-mode-map "/" #'ebdb)
>    ;; `mail-mode-map' is the parent of `vm-mail-mode-map'.
>    ;; So the following is also done by `ebdb-insinuate-mail'.
>    (if (and ebdb-complete-mail (boundp 'vm-mail-mode-map))
> -      (define-key vm-mail-mode-map "\M-\t" 'ebdb-complete-mail))
> +      (define-key vm-mail-mode-map "\M-\t" #'ebdb-complete-mail))
>  
>    ;; Set up user field for use in `vm-summary-format'
>    ;; (1) Big solution: use whole name
> diff --git a/ebdb.el b/ebdb.el
> index e4861c2756..d73f82752a 100644
> --- a/ebdb.el
> +++ b/ebdb.el
> @@ -916,17 +916,19 @@ You really should not disable debugging.  But it will speed things up."
>  ;; directly.  But presumably we could replace all the object-*
>  ;; functions with the ebdb-* inlines.
>  
> -(define-inline ebdb-add-to-list (list-var element)
> +(defmacro ebdb-add-to-list (list-var element) ;Rename to *push* and swap args?
>    "Add ELEMENT to the value of LIST-VAR if it isn't there yet and non-nil.
>  The test for presence of ELEMENT is done with `equal'."
> -  (inline-quote (when ,element (cl-pushnew ,element ,list-var :test #'equal))))
> +  (macroexp-let2 nil element element
> +    `(when ,element (cl-pushnew ,element ,list-var :test #'equal))))
>  
> -(define-inline ebdb-remove-from-list (list-var element)
> +(defmacro ebdb-remove-from-list (list-var element) ;Rename and swap args?
>    "Remove ELEMENT from LIST-VAR, if present.
>  Test for presence is done with `equal'."
> -  (inline-quote (when (and ,element ,list-var)
> -		  (setf ,list-var
> -			(delete ,element ,list-var)))))
> +  (macroexp-let2 nil element element
> +    `(when ,element
> +       ,(gv-letplace (getter setter) list-var
> +	  (funcall setter `(delete ,element ,getter))))))

I'm finally getting to push this code! I decided that ebdb-add-to-list
simply wasn't necessary -- there were hardly any cases where the element
test was meaningful, and otherwise this could just be `ebdb-pushnew'
that delegates to `cl-pushnew' with a :test. I'll keep
ebdb-remove-from-list, with your re-definition, but couldn't guess how
to rename it appropriately. `pop' isn't right, I suppose it could be
`ebdb-delete-from-list' since it uses delete under the hood -- otherwise
I don't see a clear convention in place, did you have something specific
in mind?

Thanks as always!
Eric

Re: [elpa] externals/ebdb f7b50402cf: Final round of compiler quieting

[Stefan Monnier]

> I'm finally getting to push this code! I decided that ebdb-add-to-list
> simply wasn't necessary -- there were hardly any cases where the element
> test was meaningful, and otherwise this could just be `ebdb-pushnew'
> that delegates to `cl-pushnew' with a :test. I'll keep
> ebdb-remove-from-list, with your re-definition, but couldn't guess how
> to rename it appropriately. `pop' isn't right, I suppose it could be
> `ebdb-delete-from-list' since it uses delete under the hood -- otherwise
> I don't see a clear convention in place, did you have something specific
> in mind?

No, sorry, nothing specific in mind, the choice is all yours,


        Stefan

Re: [elpa] externals/ebdb f7b50402cf: Final round of compiler quieting

[Eric Abrahamsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I'm finally getting to push this code! I decided that ebdb-add-to-list
>> simply wasn't necessary -- there were hardly any cases where the element
>> test was meaningful, and otherwise this could just be `ebdb-pushnew'
>> that delegates to `cl-pushnew' with a :test. I'll keep
>> ebdb-remove-from-list, with your re-definition, but couldn't guess how
>> to rename it appropriately. `pop' isn't right, I suppose it could be
>> `ebdb-delete-from-list' since it uses delete under the hood -- otherwise
>> I don't see a clear convention in place, did you have something specific
>> in mind?
>
> No, sorry, nothing specific in mind, the choice is all yours,

Understood!