Replace all uses of the old defadvice with the new advice-add

* lisp/org.el (org-run-like-in-org-mode): Strength reduce `eval`
to `cl-progv`.
(org--check-org-structure-template-alist): Strength reduce `eval`
to `symbol-value`.
(org-map-entries, org-eval-in-calendar, org-diary-sexp-entry):
Make sure we use the new lexically scoped dialect.
(org--math-always-on): New function, extracted from advice.
(org-cdlatex-mode): Use it with `advice-add`.
(org-self-insert-command): Simplify `and`+`listp` into `consp`.
(org-submit-bug-report):
Make sure we use the new lexically scoped dialect.

* lisp/org-protocol.el (org-protocol-convert-query-to-plist):
Use `cl-mapcan`.
(org--protocol-detect-protocol-server): New function, extracted
from advice.
(server-visit-files): Use it with `advice-add`.

* lisp/org-mouse.el (org--mouse-dnd-insert-text): New function, extracted
from advice.
(dnd-insert-text): Use it with `advice-add`.
(org--mouse-dnd-open-file): New function, extracted from advice.
(dnd-open-file): Use it with `advice-add`.
(org--mouse-open-at-point): New function, extracted from advice.
(org-mode-hook): Advise `org-open-at-point` with `advice-add`.

* lisp/org-ctags.el (org--ctags-load-tag-list): New function, extracted
from advice.
(visit-tags-table): Use it with `advice-add`.
(org--ctags-set-org-mark-before-finding-tag): New function, extracted
from advice.
(xref-find-definitions): Use it with `advice-add`.

* lisp/org-compat.el (org-bookmark-jump-unhide): Accept (unused) args.
(save-place-find-file-hook): Use `advice-add`.
(org--ecb-show-context): New function, extracted from advice.
(ecb-method-clicked): Use it with `advice-add`.
(org-mark-jump-unhide): Accept (unused) args.
(pop-to-mark-command, exchange-point-and-mark, pop-global-mark):
Use `advice-add`.

Along the way, remove some redundant `:group` args
(redundant because they specify the same group as would be used by
default anyway) and make a few other simplifications.
Also don't bother putting `advice-add` within an eval-after-load
since the advice machinery already takes care of handling it.

lisp/org-compat.el

@@ -901,7 +901,6 @@ attention to case differences."
 (defcustom org-imenu-depth 2
   "The maximum level for Imenu access to Org headlines.
 This also applied for speedbar access."
-  :group 'org-imenu-and-speedbar
   :type 'integer)
 
 ;;;; Imenu
@@ -1114,7 +1113,7 @@ ELEMENT is the element at point."
 
 ;;;; Bookmark
 
-(defun org-bookmark-jump-unhide ()
+(defun org-bookmark-jump-unhide (&rest _)
   "Unhide the current position, to show the bookmark location."
   (and (derived-mode-p 'org-mode)
        (or (org-invisible-p)
@@ -1123,7 +1122,7 @@ ELEMENT is the element at point."
        (org-show-context 'bookmark-jump)))
 
 ;; Make `bookmark-jump' shows the jump location if it was hidden.
-(add-hook 'bookmark-after-jump-hook 'org-bookmark-jump-unhide)
+(add-hook 'bookmark-after-jump-hook #'org-bookmark-jump-unhide)
 
 ;;;; Calendar
 
@@ -1176,42 +1175,29 @@ key."
 ;;;; Saveplace
 
 ;; Make sure saveplace shows the location if it was hidden
-(eval-after-load 'saveplace
-  '(defadvice save-place-find-file-hook (after org-make-visible activate)
-     "Make the position visible."
-     (org-bookmark-jump-unhide)))
+(advice-add 'save-place-find-file-hook :after #'org-bookmark-jump-unhide)
 
 ;;;; Ecb
 
 ;; Make sure ecb shows the location if it was hidden
-(eval-after-load 'ecb
-  '(defadvice ecb-method-clicked (after esf/org-show-context activate)
-     "Make hierarchy visible when jumping into location from ECB tree buffer."
-     (when (derived-mode-p 'org-mode)
-       (org-show-context))))
+(advice-add 'ecb-method-clicked :after #'org--ecb-show-context)
+(defun org--ecb-show-context (&rest _)
+  "Make hierarchy visible when jumping into location from ECB tree buffer."
+  (when (derived-mode-p 'org-mode)
+    (org-show-context)))
 
 ;;;; Simple
 
-(defun org-mark-jump-unhide ()
+(defun org-mark-jump-unhide (&rest _)
   "Make the point visible with `org-show-context' after jumping to the mark."
   (when (and (derived-mode-p 'org-mode)
 	     (org-invisible-p))
     (org-show-context 'mark-goto)))
 
-(eval-after-load 'simple
-  '(defadvice pop-to-mark-command (after org-make-visible activate)
-     "Make the point visible with `org-show-context'."
-     (org-mark-jump-unhide)))
+(advice-add 'pop-to-mark-command :after #'org-mark-jump-unhide)
 
-(eval-after-load 'simple
-  '(defadvice exchange-point-and-mark (after org-make-visible activate)
-     "Make the point visible with `org-show-context'."
-     (org-mark-jump-unhide)))
-
-(eval-after-load 'simple
-  '(defadvice pop-global-mark (after org-make-visible activate)
-     "Make the point visible with `org-show-context'."
-     (org-mark-jump-unhide)))
+(advice-add 'exchange-point-and-mark :after #'org-mark-jump-unhide)
+(advice-add 'pop-global-mark :after #'org-mark-jump-unhide)
 
 ;;;; Session
 

lisp/org-ctags.el

@@ -157,7 +157,6 @@ See the ctags documentation for more information.")
 (defcustom org-ctags-path-to-ctags
   (if (executable-find "ctags-exuberant") "ctags-exuberant" "ctags")
   "Name of the ctags executable file."
-  :group 'org-ctags
   :version "24.1"
   :type 'file)
 
@@ -166,7 +165,6 @@ See the ctags documentation for more information.")
     org-ctags-ask-rebuild-tags-file-then-find-tag
     org-ctags-ask-append-topic)
   "List of functions to be prepended to ORG-OPEN-LINK-FUNCTIONS by ORG-CTAGS."
-  :group 'org-ctags
   :version "24.1"
   :type 'hook
   :options '(org-ctags-find-tag
@@ -188,7 +186,6 @@ Created as a local variable in each buffer.")
   "Text to insert when creating a new org file via opening a hyperlink.
 The following patterns are replaced in the string:
     `%t' - replaced with the capitalized title of the hyperlink"
-  :group 'org-ctags
   :version "24.1"
   :type 'string)
 
@@ -207,7 +204,8 @@ The following patterns are replaced in the string:
                   (visit-tags-table tags-filename))))))
 
 
-(defadvice visit-tags-table (after org-ctags-load-tag-list activate compile)
+(advice-add 'visit-tags-table :after #'org--ctags-load-tag-list)
+(defun org--ctags-load-tag-list (&rest _)
   (when (and org-ctags-enabled-p tags-file-name)
     (setq-local org-ctags-tag-list
 		(org-ctags-all-tags-in-current-tags-table))))
@@ -295,8 +293,9 @@ The new topic will be titled NAME (or TITLE if supplied)."
 ;;;; Misc interoperability with etags system =================================
 
 
-(defadvice xref-find-definitions
-    (before org-ctags-set-org-mark-before-finding-tag activate compile)
+(advice-add 'xref-find-definitions :before
+            #'org--ctags-set-org-mark-before-finding-tag)
+(defun org--ctags-set-org-mark-before-finding-tag (&rest _)
   "Before trying to find a tag, save our current position on org mark ring."
   (save-excursion
     (when (and (derived-mode-p 'org-mode) org-ctags-enabled-p)

lisp/org-mouse.el

@@ -580,15 +580,17 @@ This means, between the beginning of line and the point."
   (insert text)
   (beginning-of-line))
 
-(defadvice dnd-insert-text (around org-mouse-dnd-insert-text activate)
+(advice-add 'dnd-insert-text :around #'org--mouse-dnd-insert-text)
+(defun org--mouse-dnd-insert-text (orig-fun window action text &rest args)
   (if (derived-mode-p 'org-mode)
       (org-mouse-insert-item text)
-    ad-do-it))
+    (apply orig-fun window action text args)))
 
-(defadvice dnd-open-file (around org-mouse-dnd-open-file activate)
+(advice-add 'dnd-open-file :around #'org--mouse-dnd-open-file)
+(defun org--mouse-dnd-open-file (orig-fun uri &rest args)
   (if (derived-mode-p 'org-mode)
       (org-mouse-insert-item uri)
-    ad-do-it))
+    (apply orig-fun uri args)))
 
 (defun org-mouse-match-closure (function)
   (let ((match (match-data t)))
@@ -894,15 +896,17 @@ This means, between the beginning of line and the point."
                   (1 `(face nil keymap ,org-mouse-map mouse-face highlight) prepend)))
                t))
 
-            (defadvice org-open-at-point (around org-mouse-open-at-point activate)
-              (let ((context (org-context)))
-                (cond
-                 ((assq :headline-stars context) (org-cycle))
-                 ((assq :checkbox context) (org-toggle-checkbox))
-                 ((assq :item-bullet context)
-                  (let ((org-cycle-include-plain-lists t)) (org-cycle)))
-                 ((org-footnote-at-reference-p) nil)
-                 (t ad-do-it))))))
+            (advice-add 'org-open-at-point :around #'org--mouse-open-at-point)))
+
+(defun org--mouse-open-at-point (orig-fun &rest args)
+  (let ((context (org-context)))
+    (cond
+     ((assq :headline-stars context) (org-cycle))
+     ((assq :checkbox context) (org-toggle-checkbox))
+     ((assq :item-bullet context)
+      (let ((org-cycle-include-plain-lists t)) (org-cycle)))
+     ((org-footnote-at-reference-p) nil)
+     (t (apply orig-fun args)))))
 
 (defun org-mouse-move-tree-start (_event)
   (interactive "e")

lisp/org-persist.el

@@ -179,7 +179,6 @@
                                        cache-dir))
                                    "org-persist/"))
   "Directory where the data is stored."
-  :group 'org-persist
   :type 'directory)
 
 (defcustom org-persist-remote-files 100
@@ -931,8 +930,8 @@ Also, remove containers associated with non-existing files."
       (message "Missing write access rights to org-persist-directory: %S"
                org-persist-directory)
     (add-hook 'kill-emacs-hook #'org-persist-write-all)
-    ;; `org-persist-gc' should run before `org-persist-write-all'.  So we are adding the
-    ;; hook after `org-persist-write-all'.
+    ;; `org-persist-gc' should run before `org-persist-write-all'.
+    ;; So we are adding the hook after `org-persist-write-all'.
     (add-hook 'kill-emacs-hook #'org-persist-gc)))
 
 (add-hook 'after-init-hook #'org-persist-load-all)

lisp/org-protocol.el

@@ -174,7 +174,6 @@ The filenames passed on the command line are passed to the emacs-server in
 reverse order.  Set to t (default) to re-reverse the list, i.e. use the
 sequence on the command line.  If nil, the sequence of the filenames is
 unchanged."
-  :group 'org-protocol
   :type 'boolean)
 
 (defcustom org-protocol-project-alist nil
@@ -233,7 +232,6 @@ Example:
 Consider using the interactive functions `org-protocol-create'
 and `org-protocol-create-for-org' to help you filling this
 variable with valid contents."
-  :group 'org-protocol
   :type 'alist)
 
 (defcustom org-protocol-protocol-alist nil
@@ -284,20 +282,17 @@ Here is an example:
         (\"your-protocol\"
          :protocol \"your-protocol\"
          :function your-protocol-handler-function)))"
-  :group 'org-protocol
   :type '(alist))
 
 (defcustom org-protocol-default-template-key nil
   "The default template key to use.
 This is usually a single character string but can also be a
 string with two characters."
-  :group 'org-protocol
   :type '(choice (const nil) (string)))
 
 (defcustom org-protocol-data-separator "/+\\|\\?"
   "The default data separator to use.
 This should be a single regexp string."
-  :group 'org-protocol
   :version "24.4"
   :package-version '(Org . "8.0")
   :type 'regexp)
@@ -309,7 +304,8 @@ This should be a single regexp string."
 Emacsclient compresses double and triple slashes."
   (when (string-match "^\\([a-z]+\\):/" uri)
     (let* ((splitparts (split-string uri "/+")))
-      (setq uri (concat (car splitparts) "//" (mapconcat 'identity (cdr splitparts) "/")))))
+      (setq uri (concat (car splitparts) "//"
+                        (mapconcat #'identity (cdr splitparts) "/")))))
   uri)
 
 (defun org-protocol-split-data (data &optional unhexify separator)
@@ -549,10 +545,10 @@ Now template ?b will be used."
   "Convert QUERY key=value pairs in the URL to a property list."
   (when query
     (let ((plus-decoded (replace-regexp-in-string "\\+" " " query t t)))
-      (apply 'append (mapcar (lambda (x)
-			       (let ((c (split-string x "=")))
-				 (list (intern (concat ":" (car c))) (cadr c))))
-			     (split-string plus-decoded "&"))))))
+      (cl-mapcan (lambda (x)
+		   (let ((c (split-string x "=")))
+		     (list (intern (concat ":" (car c))) (cadr c))))
+		 (split-string plus-decoded "&")))))
 
 (defun org-protocol-open-source (fname)
   "Process an org-protocol://open-source?url= style URL with FNAME.
@@ -641,7 +637,7 @@ Old-style links such as \"protocol://sub-protocol://param1/param2\" are
 also recognized.
 
 If a matching protocol is found, the protocol is stripped from
-fname and the result is passed to the protocol function as the
+FNAME and the result is passed to the protocol function as the
 first parameter.  The second parameter will be non-nil if FNAME
 uses key=val&key2=val2-type arguments, or nil if FNAME uses
 val/val2-type arguments.  If the function returns nil, the
@@ -687,12 +683,12 @@ to deal with new-style links.")
                     (throw 'fname t))))))))
       fname)))
 
-(defadvice server-visit-files (before org-protocol-detect-protocol-server activate)
+(advice-add 'server-visit-files :around #'org--protocol-detect-protocol-server)
+(defun org--protocol-detect-protocol-server (orig-fun files client &rest args)
   "Advice server-visit-flist to call `org-protocol-modify-filename-for-protocol'."
   (let ((flist (if org-protocol-reverse-list-of-files
-                   (reverse  (ad-get-arg 0))
-                 (ad-get-arg 0)))
-        (client (ad-get-arg 1)))
+                   (reverse files)
+                 files)))
     (catch 'greedy
       (dolist (var flist)
 	;; `\' to `/' on windows.  FIXME: could this be done any better?
@@ -701,11 +697,16 @@ to deal with new-style links.")
 		       fname (member var flist)  client))
           (if (eq fname t) ;; greedy? We need the t return value.
               (progn
-                (ad-set-arg 0 nil)
+                ;; FIXME: Doesn't this just ignore all the files before
+                ;; this one (the remaining ones have been passed to
+                ;; `org-protocol-check-filename-for-protocol' but not
+                ;; the ones before).
+                (setq files nil)
                 (throw 'greedy t))
             (if (stringp fname) ;; probably filename
                 (setcar var fname)
-              (ad-set-arg 0 (delq var (ad-get-arg 0))))))))))
+              (setq files (delq var files)))))))
+    (apply orig-fun files client args)))
 
 ;;; Org specific functions:
 

lisp/org-tempo.el

@@ -67,7 +67,6 @@ just like `org-structure-template-alist'.  The tempo snippet
 
 Do not use \"I\" as a KEY, as it is reserved for expanding
 \"#+include\"."
-  :group 'org-tempo
   :type '(repeat (cons (string :tag "Key")
 		       (string :tag "Keyword")))
   :package-version '(Org . "9.2"))
@@ -102,8 +101,8 @@ Tempo templates will be added."
 
 Go through `org-structure-template-alist' and
 `org-tempo-keywords-alist' and update tempo templates."
-  (mapc 'org--check-org-structure-template-alist '(org-structure-template-alist
-						   org-tempo-keywords-alist))
+  (mapc #'org--check-org-structure-template-alist '(org-structure-template-alist
+						    org-tempo-keywords-alist))
   (let ((keys (org-tempo--keys)))
     ;; Check for duplicated snippet keys and warn if any are found.
     (when (> (length keys) (length (delete-dups keys)))
@@ -176,8 +175,8 @@ didn't succeed."
 ;; Org Tempo is set up with each new Org buffer and potentially in the
 ;; current Org buffer.
 
-(add-hook 'org-mode-hook 'org-tempo-setup)
-(add-hook 'org-tab-before-tab-emulation-hook 'org-tempo-complete-tag)
+(add-hook 'org-mode-hook #'org-tempo-setup)
+(add-hook 'org-tab-before-tab-emulation-hook #'org-tempo-complete-tag)
 
 ;; Enable Org Tempo in all open Org buffers.
 (dolist (b (org-buffer-list 'files))