On 29/04/2024 20:12, Ihor Radchenko wrote:
Max Nikulin writes:
Notice that new tests for org-ctags do not require user interactions.
[...]
Of course, the cause is the known side effect of loading org-ctags.
Maybe we can disable the tests until that bug is fixed.
Or, ideally, load org-ctags only when the relevant tests are running and
unload it after they finish.
Isn't it better to modify buggy org-ctags than to add various kludges to
tests?
From 0d260a0f260afb0f407d00b6a53ed2121a240203 Mon Sep 17 00:00:00 2001
From: Max Nikulin <maniku...@gmail.com>
Date: Mon, 29 Apr 2024 21:34:13 +0700
Subject: [PATCH 1/2] org-ctags.el: Define unload function
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* lisp/org-ctags.el (org-ctags-unload-function): New function to cleanup
during `unload-feature' call.
(org-ctags--open-link-functions-list org-ctags-open-link-functions):
Define and use list of options available for `org-open-link-functions'.
(org-ctags--visit-tags-table): Give a name to remove the function from
`org-mode-hook' on library unload.
Prevent the following error after library unloading
Symbol’s function definition is void: org-ctags-find-tag
---
lisp/org-ctags.el | 60 +++++++++++++++++++++++++++++++----------------
1 file changed, 40 insertions(+), 20 deletions(-)
diff --git a/lisp/org-ctags.el b/lisp/org-ctags.el
index 6431a2765..c6f7fc708 100644
--- a/lisp/org-ctags.el
+++ b/lisp/org-ctags.el
@@ -161,6 +161,20 @@ (defcustom org-ctags-path-to-ctags
:version "24.1"
:type 'file)
+(defconst org-ctags--open-link-functions-list
+ (list
+ #'org-ctags-find-tag
+ #'org-ctags-ask-rebuild-tags-file-then-find-tag
+ #'org-ctags-rebuild-tags-file-then-find-tag
+ #'org-ctags-ask-append-topic
+ #'org-ctags-append-topic
+ #'org-ctags-ask-visit-buffer-or-file
+ #'org-ctags-visit-buffer-or-file
+ #'org-ctags-fail-silently)
+ "Options for `org-open-link-functions'.
+Ensure that the user option and `unload-feature'
+use the same set of functions.")
+
(defcustom org-ctags-open-link-functions
'(org-ctags-find-tag
org-ctags-ask-rebuild-tags-file-then-find-tag
@@ -168,14 +182,7 @@ (defcustom org-ctags-open-link-functions
"List of functions to be prepended to ORG-OPEN-LINK-FUNCTIONS by ORG-CTAGS."
:version "24.1"
:type 'hook
- :options '(org-ctags-find-tag
- org-ctags-ask-rebuild-tags-file-then-find-tag
- org-ctags-rebuild-tags-file-then-find-tag
- org-ctags-ask-append-topic
- org-ctags-append-topic
- org-ctags-ask-visit-buffer-or-file
- org-ctags-visit-buffer-or-file
- org-ctags-fail-silently))
+ :options org-ctags--open-link-functions-list)
(defvar org-ctags-tag-list nil
@@ -191,18 +198,20 @@ (defcustom org-ctags-new-topic-template
:type 'string)
-(add-hook 'org-mode-hook
- (lambda ()
- (when (and org-ctags-enabled-p
- (buffer-file-name))
- ;; Make sure this file's directory is added to default
- ;; directories in which to search for tags.
- (let ((tags-filename
- (expand-file-name
- (concat (file-name-directory (buffer-file-name))
- "/TAGS"))))
- (when (file-exists-p tags-filename)
- (visit-tags-table tags-filename))))))
+(defun org-ctags--visit-tags-table ()
+ "Load tags for current file.
+A function for `org-mode-hook."
+ (when (and org-ctags-enabled-p
+ (buffer-file-name))
+ ;; Make sure this file's directory is added to default
+ ;; directories in which to search for tags.
+ (let ((tags-filename
+ (expand-file-name
+ (concat (file-name-directory (buffer-file-name))
+ "/TAGS"))))
+ (when (file-exists-p tags-filename)
+ (visit-tags-table tags-filename)))))
+(add-hook 'org-mode-hook #'org-ctags--visit-tags-table)
(advice-add 'visit-tags-table :after #'org--ctags-load-tag-list)
@@ -219,6 +228,17 @@ (defun org-ctags-enable ()
(add-hook 'org-open-link-functions fn t)))
+(defun org-ctags-unload-function ()
+ "Disable `org-ctags' library.
+Called by `unload-feature'."
+ (put 'org-mode 'find-tag-default-function nil)
+ (advice-remove 'visit-tags-table #'org--ctags-load-tag-list)
+ (advice-remove 'xref-find-definitions
+ #'org--ctags-set-org-mark-before-finding-tag)
+ (dolist (fn org-ctags--open-link-functions-list)
+ (remove-hook 'org-open-link-functions fn nil)))
+
+
;;; General utility functions. ===============================================
;; These work outside org-ctags mode.
--
2.39.2
From 5915811995f83fbfb89606bfa4f316d2b4521268 Mon Sep 17 00:00:00 2001
From: Max Nikulin <maniku...@gmail.com>
Date: Mon, 29 Apr 2024 23:28:29 +0700
Subject: [PATCH 2/2] org-ctags.el: Do not activate on load
* etc/ORG-NEWS: Announce the change breaking for `org-ctags' users and
provide init file code to enable the feature.
* lisp/org-ctags.el (org-ctags-enable): Do no invoke this function
during library loading. Collect all initialization code in its body.
Setting up hooks during library loading leads to various issues.
- Emacs coding conventions insist on incompatible changes if loading
a library modifies behavior, see
Info node `(elisp) Coding Conventions'.
- The library may be autoloaded for the sake of help completion
breaking `org-open-at-point':
Nick Dokos. org-ctags land grab. Mon, 20 Mar 2023 23:36:09 -0400.
<https://list.orgmode.org/87o7omg4ie....@alphaville.usersys.redhat.com>
- Unrelated unit tests fail due to user prompt:
Ihor Radchenko. Re: [PATCH] org-ctags.el: Protect shell specials
in directory name. Sun, 28 Apr 2024 12:53:38 +0000.
<https://list.orgmode.org/87a5ldk5rh.fsf@localhost>
---
etc/ORG-NEWS | 12 ++++++++++++
lisp/org-ctags.el | 17 +++++++++++------
2 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 06d3cf093..fea38e54e 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -641,6 +641,18 @@ This behaviour has been expanded to store an additional =CUSTOM_ID=
link when storing any type of external link type in an Org file, not
just =id:= links.
+*** ~org-ctags~ is not activated by default any more
+
+To follow Emacs [[info:elisp#Coding Conventions][coding conventions]] and to avoid confusion of users
+who accidentally get ~org-ctags~ autoloaded due to help completion,
+the library does not modify ~org-open-link-functions~ during loading
+any more. Run ~org-ctags-enable~ to setup hooks and advices:
+
+#+begin_src emacs-lisp
+(with-eval-after-load "org-ctags"
+ (org-ctags-enable))
+#+end_src
+
** New and changed options
*** ~org-babel-lua-multiple-values-separator~
diff --git a/lisp/org-ctags.el b/lisp/org-ctags.el
index c6f7fc708..784147572 100644
--- a/lisp/org-ctags.el
+++ b/lisp/org-ctags.el
@@ -57,6 +57,12 @@ ;;; Commentary:
;; (add-hook 'org-mode-hook
;; (lambda ()
;; (define-key org-mode-map "\C-co" 'org-ctags-find-tag-interactive)))
+;; (with-eval-after-load "org-ctags"
+;; (org-ctags-enable))
+;;
+;; To activate the library, you need to call `org-ctags-enable' explicitly.
+;; It used to be invoked during library loading, but it was against Emacs
+;; policy and caused inconvenience of Org users who do not use `org-ctags'.
;;
;; By default, with org-ctags loaded, org will first try and visit the tag
;; with the same name as the link; then, if unsuccessful, ask the user if
@@ -211,10 +217,8 @@ (defun org-ctags--visit-tags-table ()
"/TAGS"))))
(when (file-exists-p tags-filename)
(visit-tags-table tags-filename)))))
-(add-hook 'org-mode-hook #'org-ctags--visit-tags-table)
-(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
@@ -222,6 +226,11 @@ (defun org--ctags-load-tag-list (&rest _)
(defun org-ctags-enable ()
+ (add-hook 'org-mode-hook #'org-ctags--visit-tags-table)
+ (advice-add 'visit-tags-table :after #'org--ctags-load-tag-list)
+ (advice-add 'xref-find-definitions :before
+ #'org--ctags-set-org-mark-before-finding-tag)
+
(put 'org-mode 'find-tag-default-function 'org-ctags-find-tag-at-point)
(setq org-ctags-enabled-p t)
(dolist (fn org-ctags-open-link-functions)
@@ -314,8 +323,6 @@ (defun org-ctags-open-file (name &optional title)
;;;; Misc interoperability with etags system =================================
-(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
@@ -543,8 +550,6 @@ (defun org-ctags-find-tag-interactive ()
'org-open-link-functions tag))))))
-(org-ctags-enable)
-
(provide 'org-ctags)
;;; org-ctags.el ends here
--
2.39.2
