* [PATCH] `org-ctags-create-tags` creates empty TAGS file [9.6.15 (release_9.6.15 @ /home/martin/Projects/emacs/lisp/org/)]
@ 2024-02-09 23:57 Martin Marshall
2024-02-10 4:49 ` Is there something people use instead of org-ctags? (was: [PATCH] `org-ctags-create-tags` creates empty TAGS file) Martin Marshall
2024-02-10 14:27 ` [PATCH] `org-ctags-create-tags` creates empty TAGS file [9.6.15 (release_9.6.15 @ /home/martin/Projects/emacs/lisp/org/)] Ihor Radchenko
0 siblings, 2 replies; 25+ messages in thread
From: Martin Marshall @ 2024-02-09 23:57 UTC (permalink / raw)
To: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 2317 bytes --]
Hi, the docstring of `org-ctags-create-tags` says it should "(Re)create
tags file in the directory of the active buffer," creating tags from the
internal links found in the org files. However, it always creates an
empty TAGS file.
The cause appears to be a pair of escaped quotes used with
`shell-command` when it calls the "ctags" executable.
* Re-creating the issue
1. First, as explained in the commentary of "org-ctags.el", make sure
you have exuberant-ctags installed on your system. On a debian-based
system, like so...
--8<---------------cut here---------------start------------->8---
sudo apt install exuberant-ctags
--8<---------------cut here---------------end--------------->8---
2. Start Emacs from the command-line with "emacs -Q".
3. In the "*scratch*" buffer, paste the expression shown below.
--8<---------------cut here---------------start------------->8---
(let* ((testdir (expand-file-name "test1234xyz" "~"))
(orgfile (expand-file-name "test-file.org" testdir))
(tagsfile (expand-file-name "TAGS" testdir)))
(unless (file-exists-p testdir)
(make-directory testdir))
(find-file orgfile)
(insert "<<test link>>")
(save-buffer)
(require 'org-ctags)
(org-ctags-create-tags testdir)
(find-file tagsfile))
--8<---------------cut here---------------end--------------->8---
4. If you evaluate the above code. It creates the "~/test1234xyz"
directory, an org file containing a link, and a new TAGS file.
It also opens the new TAGS file. But as you can see, it's empty.
* Cause
The cause appears to be some escaped quotes around a shell command
argument. The FILES argument passed to the "ctags" executable uses
globbing. But since it's surrounded by double quotes, no globbing
occurs, and "ctags" doesn't actually scan any files.
If we change this:
"--regex-orgmode=\"%s\" -f \"%s\" -e -R \"%s\"")
To this:
"--regex-orgmode=\"%s\" -f \"%s\" -e -R %s")
It works as expected.
I've attached a patch against the current Emacs master branch. I hope
that's sufficient, given the minimal nature of the change.
Emacs : GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.38, cairo version 1.16.0)
of 2024-02-09
Package: Org mode version 9.6.15 (release_9.6.15 @ /home/martin/Projects/emacs/lisp/org/)
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Fix for org-ctags-create-tags command --]
[-- Type: text/x-diff, Size: 1067 bytes --]
From a6719edafd928a5ce27036be5d5bec00eaafa8ec Mon Sep 17 00:00:00 2001
From: Martin Marshall <law@martinmarshall.com>
Date: Fri, 9 Feb 2024 17:40:03 -0500
Subject: [PATCH] org-ctags.el: Fix use of "ctags" executable
* lisp/org/org-ctags.el (org-ctags-create-tags): Allow file
globbing in `shell-command' invocation of "ctags".
TINYCHANGE
---
lisp/org/org-ctags.el | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lisp/org/org-ctags.el b/lisp/org/org-ctags.el
index 2417353ee5d..9e523e7dc67 100644
--- a/lisp/org/org-ctags.el
+++ b/lisp/org/org-ctags.el
@@ -486,7 +486,7 @@ org-ctags-create-tags
(setq exitcode
(shell-command
(format (concat "%s --langdef=orgmode --langmap=orgmode:.org "
- "--regex-orgmode=\"%s\" -f \"%s\" -e -R \"%s\"")
+ "--regex-orgmode=\"%s\" -f \"%s\" -e -R %s")
org-ctags-path-to-ctags
org-ctags-tag-regexp
(expand-file-name (concat dir-name "/TAGS"))
--
2.39.2
[-- Attachment #3: Type: text/plain, Size: 34 bytes --]
--
Best regards,
Martin Marshall
^ permalink raw reply related [flat|nested] 25+ messages in thread* Is there something people use instead of org-ctags? (was: [PATCH] `org-ctags-create-tags` creates empty TAGS file) 2024-02-09 23:57 [PATCH] `org-ctags-create-tags` creates empty TAGS file [9.6.15 (release_9.6.15 @ /home/martin/Projects/emacs/lisp/org/)] Martin Marshall @ 2024-02-10 4:49 ` Martin Marshall 2024-02-10 14:29 ` Ihor Radchenko 2024-02-10 14:27 ` [PATCH] `org-ctags-create-tags` creates empty TAGS file [9.6.15 (release_9.6.15 @ /home/martin/Projects/emacs/lisp/org/)] Ihor Radchenko 1 sibling, 1 reply; 25+ messages in thread From: Martin Marshall @ 2024-02-10 4:49 UTC (permalink / raw) To: emacs-orgmode I was curious how long this bug had been around and why no one complained about it before. So I looked through the Git log and found that it was introduced on 1/18/2010. That's just over two weeks after the package was added to Emacs[1]! Other than a 4 year old Reddit post[2], there've been no bug reports or mailing list discussions about it in the fourteen years since the bug was introduced. This gives me the impression that very few people are using the org-ctags package. That's surprising, because it seems like it could be very useful. It allows for linking to "direct targets" from external files[3], which is similar to the "come-from" links that Howm implements. It's a very low-effort, low-friction way to add links between different notes. Is there some other way to create this sort of simple external link in org-mode? Is there some other package that provides a similar feature? [1] Commit 53868111d000302b50706769526f15164600d739 [2] https://www.reddit.com/r/emacs/comments/fg71cw/orgctags_failed_to_create_tags/ [3] That look like <<this>>. See https://orgmode.org/manual/Internal-Links.html -- Best regards, Martin Marshall ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Is there something people use instead of org-ctags? (was: [PATCH] `org-ctags-create-tags` creates empty TAGS file) 2024-02-10 4:49 ` Is there something people use instead of org-ctags? (was: [PATCH] `org-ctags-create-tags` creates empty TAGS file) Martin Marshall @ 2024-02-10 14:29 ` Ihor Radchenko 0 siblings, 0 replies; 25+ messages in thread From: Ihor Radchenko @ 2024-02-10 14:29 UTC (permalink / raw) To: Martin Marshall; +Cc: emacs-orgmode Martin Marshall <law@martinmarshall.com> writes: > Other than a 4 year old Reddit post[2], there've been no bug reports or > mailing list discussions about it in the fourteen years since the bug > was introduced. This gives me the impression that very few people are > using the org-ctags package. Yeah. > That's surprising, because it seems like it could be very useful. It > allows for linking to "direct targets" from external files[3], which is > similar to the "come-from" links that Howm implements. It's a very > low-effort, low-friction way to add links between different notes. > > Is there some other way to create this sort of simple external link in > org-mode? Is there some other package that provides a similar feature? The most commonly used link type is id:. It is also file-independent and can link to arbitrary headings with :ID: property. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] `org-ctags-create-tags` creates empty TAGS file [9.6.15 (release_9.6.15 @ /home/martin/Projects/emacs/lisp/org/)] 2024-02-09 23:57 [PATCH] `org-ctags-create-tags` creates empty TAGS file [9.6.15 (release_9.6.15 @ /home/martin/Projects/emacs/lisp/org/)] Martin Marshall 2024-02-10 4:49 ` Is there something people use instead of org-ctags? (was: [PATCH] `org-ctags-create-tags` creates empty TAGS file) Martin Marshall @ 2024-02-10 14:27 ` Ihor Radchenko 2024-02-10 21:10 ` Morgan Willcock 1 sibling, 1 reply; 25+ messages in thread From: Ihor Radchenko @ 2024-02-10 14:27 UTC (permalink / raw) To: Martin Marshall; +Cc: emacs-orgmode Martin Marshall <law@martinmarshall.com> writes: > Hi, the docstring of `org-ctags-create-tags` says it should "(Re)create > tags file in the directory of the active buffer," creating tags from the > internal links found in the org files. However, it always creates an > empty TAGS file. > > The cause appears to be a pair of escaped quotes used with > `shell-command` when it calls the "ctags" executable. > ... > I've attached a patch against the current Emacs master branch. I hope > that's sufficient, given the minimal nature of the change. Thanks! Applied, onto main. https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=981402a93 -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] `org-ctags-create-tags` creates empty TAGS file [9.6.15 (release_9.6.15 @ /home/martin/Projects/emacs/lisp/org/)] 2024-02-10 14:27 ` [PATCH] `org-ctags-create-tags` creates empty TAGS file [9.6.15 (release_9.6.15 @ /home/martin/Projects/emacs/lisp/org/)] Ihor Radchenko @ 2024-02-10 21:10 ` Morgan Willcock 2024-02-10 21:20 ` Ihor Radchenko 2024-03-19 10:21 ` Max Nikulin 0 siblings, 2 replies; 25+ messages in thread From: Morgan Willcock @ 2024-02-10 21:10 UTC (permalink / raw) To: Ihor Radchenko; +Cc: Martin Marshall, emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 978 bytes --] Ihor Radchenko <yantar92@posteo.net> writes: > Martin Marshall <law@martinmarshall.com> writes: > >> Hi, the docstring of `org-ctags-create-tags` says it should "(Re)create >> tags file in the directory of the active buffer," creating tags from the >> internal links found in the org files. However, it always creates an >> empty TAGS file. >> >> The cause appears to be a pair of escaped quotes used with >> `shell-command` when it calls the "ctags" executable. >> ... >> I've attached a patch against the current Emacs master branch. I hope >> that's sufficient, given the minimal nature of the change. > > Thanks! > Applied, onto main. > https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=981402a93 Doesn't this change mean that it will now break when the expanded path has whitespace characters in it? The shell expansion should work if the asterisk is outside of the quotes. I've attached an (untested) patch to explain what I mean. -- Morgan Willcock [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-org-ctags.el-Quote-directory-name-for-ctags-shell-co.patch --] [-- Type: text/x-patch, Size: 1355 bytes --] From b5f52034b693175df2ec057cb5e9e4de55e70078 Mon Sep 17 00:00:00 2001 From: Morgan Willcock <morgan@ice9.digital> Date: Sat, 10 Feb 2024 21:02:30 +0000 Subject: [PATCH] org-ctags.el: Quote directory name for "ctags" shell command * lisp/org-ctags.el (org-ctags-create-tags): Expand the quoted form of the directory name in the "ctags" shell command. This allows the directory name to contain whitespace characters. --- lisp/org-ctags.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lisp/org-ctags.el b/lisp/org-ctags.el index 693ccc87b..49c1d1228 100644 --- a/lisp/org-ctags.el +++ b/lisp/org-ctags.el @@ -484,11 +484,11 @@ defun org-ctags-create-tags (setq exitcode (shell-command (format (concat "%s --langdef=orgmode --langmap=orgmode:.org " - "--regex-orgmode=\"%s\" -f \"%s\" -e -R %s") + "--regex-orgmode=\"%s\" -f \"%s\" -e -R \"%s\"*") org-ctags-path-to-ctags org-ctags-tag-regexp (expand-file-name (concat dir-name "/TAGS")) - (expand-file-name (concat dir-name "/*"))))) + (expand-file-name (concat dir-name "/"))))) (cond ((eql 0 exitcode) (setq-local org-ctags-tag-list -- 2.41.0.windows.3 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH] `org-ctags-create-tags` creates empty TAGS file [9.6.15 (release_9.6.15 @ /home/martin/Projects/emacs/lisp/org/)] 2024-02-10 21:10 ` Morgan Willcock @ 2024-02-10 21:20 ` Ihor Radchenko 2024-03-19 10:21 ` Max Nikulin 1 sibling, 0 replies; 25+ messages in thread From: Ihor Radchenko @ 2024-02-10 21:20 UTC (permalink / raw) To: Morgan Willcock; +Cc: Martin Marshall, emacs-orgmode Morgan Willcock <morgan@ice9.digital> writes: >>> I've attached a patch against the current Emacs master branch. I hope >>> that's sufficient, given the minimal nature of the change. >> >> Thanks! >> Applied, onto main. >> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=981402a93 > > Doesn't this change mean that it will now break when the expanded path > has whitespace characters in it? Right. I fixed this case on main. https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=d3a139427 > The shell expansion should work if the asterisk is outside of the > quotes. I've attached an (untested) patch to explain what I mean. I went with `shell-quote-argument' variant. Canceled. (the patch) -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] `org-ctags-create-tags` creates empty TAGS file [9.6.15 (release_9.6.15 @ /home/martin/Projects/emacs/lisp/org/)] 2024-02-10 21:10 ` Morgan Willcock 2024-02-10 21:20 ` Ihor Radchenko @ 2024-03-19 10:21 ` Max Nikulin 2024-03-20 12:08 ` Ihor Radchenko 1 sibling, 1 reply; 25+ messages in thread From: Max Nikulin @ 2024-03-19 10:21 UTC (permalink / raw) To: Morgan Willcock; +Cc: Martin Marshall, emacs-orgmode On 11/02/2024 04:10, Morgan Willcock wrote: > > The shell expansion should work if the asterisk is outside of the > quotes. I've attached an (untested) patch to explain what I mean. Never try to quote arbitrary strings by double or single quotes in shell. There are enough fancy characters that may be interpreted in a special way. The safest approach is to use `process-file' instead of `shell-command', but in the case of a remote file shell is unavoidable and would require additional round trip for `file-expand-wildcards'. The committed change is anyway incomplete. > +++ b/lisp/org-ctags.el > @@ -484,11 +484,11 @@ defun org-ctags-create-tags > (setq exitcode > (shell-command > (format (concat "%s --langdef=orgmode --langmap=orgmode:.org " > - "--regex-orgmode=\"%s\" -f \"%s\" -e -R %s") > + "--regex-orgmode=\"%s\" -f \"%s\" -e -R \"%s\"*") -----------------------------------------------------------^^^^^^ These quote characters should be removed as well > org-ctags-path-to-ctags > org-ctags-tag-regexp > (expand-file-name (concat dir-name "/TAGS")) It requires `shell-quote-argument' as well > - (expand-file-name (concat dir-name "/*"))))) > + (expand-file-name (concat dir-name "/"))))) > (cond > ((eql 0 exitcode) > (setq-local org-ctags-tag-list ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] `org-ctags-create-tags` creates empty TAGS file [9.6.15 (release_9.6.15 @ /home/martin/Projects/emacs/lisp/org/)] 2024-03-19 10:21 ` Max Nikulin @ 2024-03-20 12:08 ` Ihor Radchenko 2024-04-28 7:37 ` [PATCH] org-ctags.el: Protect shell specials in directory name Max Nikulin 0 siblings, 1 reply; 25+ messages in thread From: Ihor Radchenko @ 2024-03-20 12:08 UTC (permalink / raw) To: Max Nikulin; +Cc: Morgan Willcock, Martin Marshall, emacs-orgmode Max Nikulin <manikulin@gmail.com> writes: > The committed change is anyway incomplete. > ... May you submit a patch? -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH] org-ctags.el: Protect shell specials in directory name 2024-03-20 12:08 ` Ihor Radchenko @ 2024-04-28 7:37 ` Max Nikulin 2024-04-28 12:53 ` Ihor Radchenko 0 siblings, 1 reply; 25+ messages in thread From: Max Nikulin @ 2024-04-28 7:37 UTC (permalink / raw) To: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 161 bytes --] On 20/03/2024 19:08, Ihor Radchenko wrote: > Max Nikulin writes: >> The committed change is anyway incomplete. > > May you submit a patch? See the attachments. [-- Attachment #2: 0001-org-ctags.el-Protect-shell-specials-in-directory-nam.patch --] [-- Type: text/x-patch, Size: 2376 bytes --] From 067dc590bb1c26c881f14d218da2cd502413ec5d Mon Sep 17 00:00:00 2001 From: Max Nikulin <manikulin@gmail.com> Date: Wed, 27 Mar 2024 23:04:07 +0700 Subject: [PATCH 1/2] org-ctags.el: Protect shell specials in directory name * lisp/org-ctags.el (org-ctags-create-tags): Escape shell specials. Directory name (the argument or `default-directory') may contain various characters interpreted by shell. Effects may vary from just incorrect actual path to execution of a command embedded into path. Neither double nor single quotes is a safe way to use directory name in shell commands since the name may contain these characters. A follow-up to Martin Marshall. [PATCH] `org-ctags-create-tags` creates empty TAGS file. Fri, 09 Feb 2024 18:57:48 -0500. <https://list.orgmode.org/87h6ihgphf.fsf@martinmarshall.com> --- lisp/org-ctags.el | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lisp/org-ctags.el b/lisp/org-ctags.el index 6431a2765..52b21dbd1 100644 --- a/lisp/org-ctags.el +++ b/lisp/org-ctags.el @@ -477,18 +477,21 @@ (defun org-ctags-create-tags (&optional directory-name) its subdirectories contain large numbers of taggable files." (interactive) (cl-assert (buffer-file-name)) - (let ((dir-name (or directory-name - (file-name-directory (buffer-file-name)))) + (let ((dir-name (shell-quote-argument + (expand-file-name + (if directory-name + (file-name-as-directory directory-name) + (file-name-directory (buffer-file-name)))))) (exitcode nil)) (save-excursion (setq exitcode (shell-command (format (concat "%s --langdef=orgmode --langmap=orgmode:.org " - "--regex-orgmode=\"%s\" -f \"%s\" -e -R %s") + "--regex-orgmode=%s -f %sTAGS -e -R %s*") org-ctags-path-to-ctags - org-ctags-tag-regexp - (expand-file-name (concat dir-name "/TAGS")) - (expand-file-name (concat (shell-quote-argument dir-name) "/*"))))) + (shell-quote-argument org-ctags-tag-regexp) + dir-name + dir-name))) (cond ((eql 0 exitcode) (setq-local org-ctags-tag-list -- 2.39.2 [-- Attachment #3: 0002-test-org-ctags.el-Test-escaping-of-shell-arguments.patch --] [-- Type: text/x-patch, Size: 8573 bytes --] From 52611183ff73d0701d41102e0fa97134178cae10 Mon Sep 17 00:00:00 2001 From: Max Nikulin <manikulin@gmail.com> Date: Sun, 28 Apr 2024 14:13:04 +0700 Subject: [PATCH 2/2] test-org-ctags.el: Test escaping of shell arguments * testing/lisp/test-org-ctags.el (test-org-ctags/create-tags-escape): A new test that tag regexp and directory names are properly quoted while "*" wildcard is active. (test-org-ctags/list-elements test-org-ctags/list-elements-equal-p) (test-org-ctags/list-elements-equal-explain): Helpers to provide informative failure messages. (test-org-ctags/with-fake-ctags): A helper to create temporary directories and a file and to temporary arrange a mock shell command instead of ctags executable. (test-org-ctags/mock-command test-org-ctags/get-args): Helpers to define a mock shell command and to obtain its actual arguments. --- testing/lisp/test-org-ctags.el | 192 +++++++++++++++++++++++++++++++++ 1 file changed, 192 insertions(+) create mode 100644 testing/lisp/test-org-ctags.el diff --git a/testing/lisp/test-org-ctags.el b/testing/lisp/test-org-ctags.el new file mode 100644 index 000000000..7f5fca948 --- /dev/null +++ b/testing/lisp/test-org-ctags.el @@ -0,0 +1,192 @@ +;;; test-org-ctags.el --- tests for org-ctags.el -*- lexical-binding: t -*- + +;; Copyright (C) 2024 Max Nikulin +;; Authors: Max Nikulin + +;; This file is not part of GNU Emacs. + +;; This program is free software; you can redistribute it and/or modify +;; it under the terms of the GNU General Public License as published by +;; the Free Software Foundation, either version 3 of the License, or +;; (at your option) any later version. + +;; This program is distributed in the hope that it will be useful, +;; but WITHOUT ANY WARRANTY; without even the implied warranty of +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;; GNU General Public License for more details. + +;; You should have received a copy of the GNU General Public License +;; along with this program. If not, see <https://www.gnu.org/licenses/>. + +;;; Code: + +;; Alternative implementation for `test-org-ctags/mock-command' +;; is required for cmd.exe. +(unless (string-equal "-c" shell-command-switch) + (signal 'missing-test-dependency "POSIX shell")) + +(require 'org-ctags) + +;;;; Helpers: + +(defun test-org-ctags/mock-command (temp-file command-name) + "Define shell function COMMAND-NAME wrining arguments to TEMP-FILE." + ;; Failure exit code is used to prevent further `org-ctags' actions. + (format "%s() { printf '%%s\\n' %s \"$@\" >%s 2>&1 ; false ; } ; %s" + command-name command-name + (shell-quote-argument temp-file) + command-name)) + +(defun test-org-ctags/get-args (temp-file base magic) + "Read list of strings from TEMP-FILE. + +If TEMP-FILE does not start from MAGIC then return +its content as a string. Otherwise strip first line +and trailing newline, replace BASE with \"TMPDIR\" string, +return list of lines." + (let* ((case-fold-search nil) + (content + (and + (file-exists-p temp-file) + (with-temp-buffer + (insert-file-contents temp-file) + (goto-char (point-min)) + (when (looking-at magic) + (while (search-forward base nil 'noerror) + (replace-match "TMPDIR" 'fixedcase 'literal))) + (goto-char (point-max)) + (when (and (bolp) (> (point) 1)) + (delete-char -1)) + (buffer-string))))) + (if (and content (string-prefix-p magic content)) + (cdr (split-string content "\n")) + content))) + +(defmacro test-org-ctags/with-fake-ctags + (temp-dir subdir &rest body) + "Run BODY with `org-ctags-path-to-ctags' set to a test function. + +Create a buffer backed by a file in the TEMP-DIR/SUBDIR directory." + (declare (indent 2)) + (let ((buffer (gensym "buffer")) + (base (gensym "base")) + (dir (gensym "dir")) + (temp-file (gensym "temp-file"))) + `(let* ((,base ,temp-dir) + (,dir (concat ,base "/" ,subdir)) + (,temp-file (concat ,dir "/ctags.txt")) + (org-ctags-path-to-ctags + (test-org-ctags/mock-command ,temp-file "ctags-mock")) + ,buffer) + (make-directory ,dir) + (unwind-protect + ;; `org-ctags' commands call `buffer-file-name'. + (with-current-buffer + (setq ,buffer (find-file-noselect ,temp-file)) + (insert "Sould be overwritten by org-ctags mock script") + (save-buffer) + ,@body + (test-org-ctags/get-args ,temp-file ,base "ctags-mock\n")) + (kill-buffer ,buffer) + (delete-file ,temp-file) + (delete-directory ,dir))))) +\f +;;;; Comparator to have informative failures: + +(defun test-org-ctags/list-elements (lst &optional indicies) + "Select INDICIES elements from LST list. + +INDICIES should be sorted in growing order." + (if (not (and indicies (listp lst))) + lst + (let (selected + (prev 0)) + (dolist (i indicies (nreverse selected)) + (setq lst (nthcdr (- i prev) lst)) + (setq prev i) + (push (car lst) selected))))) + +(defun test-org-ctags/list-elements-equal-p + (expect actual indicies &rest _comments) + "Call `equal' for lists EXPECT and INDICIES elements from ACTUAL. + +_COMMENTS should appear in failure message." + (equal expect + (test-org-ctags/list-elements actual indicies))) + +(defun test-org-ctags/list-elements-equal-explain + (expect actual indicies &rest _comments) + "`ert-eplainer' for `test-org-ctags/list-elements-equal-p'." + (if (listp actual) + (list + 'selected-elements + (test-org-ctags/list-elements actual indicies)) + "Shell command failed")) + +(put 'test-org-ctags/list-elements-equal-p + 'ert-explainer + 'test-org-ctags/list-elements-equal-explain) +\f +;;;; Tests: + +(ert-deftest test-org-ctags/create-tags-escape () + "Test that `org-ctags-create-tags' escapes shell arguments." + (let ((temp-dir (make-temp-file "test-org-ctags-" 'dir))) + (unwind-protect + (progn + (should + (test-org-ctags/list-elements-equal-p + (list (format "--regex-orgmode=%s" org-ctags-tag-regexp)) + (test-org-ctags/with-fake-ctags temp-dir "regexp" + (org-ctags-create-tags)) + '(2) + "Regexp should be escaped.")) + + (should + (test-org-ctags/list-elements-equal-p + '("TMPDIR/regular/ctags.txt") + (test-org-ctags/with-fake-ctags temp-dir "regular" + (org-ctags-create-tags (concat temp-dir "/regular"))) + '(7) + "Wildcard should be expanded." + "Directory passed as an argument.")) + + (should + (test-org-ctags/list-elements-equal-p + '("TMPDIR/space char/TAGS" "TMPDIR/space char/ctags.txt") + (test-org-ctags/with-fake-ctags temp-dir "space char" + (org-ctags-create-tags (concat temp-dir "/space char"))) + '(4 7) + "Space characters should not split arguments." + "Directory passed as an argument.")) + + (should + (test-org-ctags/list-elements-equal-p + '("TMPDIR/apostrophe' sep '/TAGS" "TMPDIR/apostrophe' sep '/ctags.txt") + (test-org-ctags/with-fake-ctags temp-dir "apostrophe' sep '" + (org-ctags-create-tags)) + '(4 7) + "Apostrophes should be regular characters." + "Path is derived from `default-directory'.")) + + (should + (test-org-ctags/list-elements-equal-p + '("TMPDIR/def-dir.$HOME/TAGS" "TMPDIR/def-dir.$HOME/ctags.txt") + (test-org-ctags/with-fake-ctags temp-dir "def-dir.$HOME" + (org-ctags-create-tags)) + '(4 7) + "$VARIABLES should not be expanded in directory names." + "Path is derived from `default-directory'.")) + + (should + (test-org-ctags/list-elements-equal-p + '("TMPDIR/arg.$HOME/TAGS" "TMPDIR/arg.$HOME/ctags.txt") + (test-org-ctags/with-fake-ctags temp-dir "arg.$HOME" + (org-ctags-create-tags (concat temp-dir "/arg.$HOME"))) + '(4 7) + "$VARIABLES should not be expanded in directory names." + "Directory passed as an argument"))) + (delete-directory temp-dir)))) + +(provide 'test-org-ctags) +;;; test-org.el ends here -- 2.39.2 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH] org-ctags.el: Protect shell specials in directory name 2024-04-28 7:37 ` [PATCH] org-ctags.el: Protect shell specials in directory name Max Nikulin @ 2024-04-28 12:53 ` Ihor Radchenko 2024-04-28 16:51 ` Max Nikulin 0 siblings, 1 reply; 25+ messages in thread From: Ihor Radchenko @ 2024-04-28 12:53 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode Max Nikulin <manikulin@gmail.com> writes: >>> The committed change is anyway incomplete. >> >> May you submit a patch? > See the attachments. Thanks! I tried to run make test with your patch applied, but I am getting interactive prompt there: Visit tags table (default TAGS): -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] org-ctags.el: Protect shell specials in directory name 2024-04-28 12:53 ` Ihor Radchenko @ 2024-04-28 16:51 ` Max Nikulin 2024-04-28 16:55 ` Ihor Radchenko 0 siblings, 1 reply; 25+ messages in thread From: Max Nikulin @ 2024-04-28 16:51 UTC (permalink / raw) To: emacs-orgmode On 28/04/2024 19:53, Ihor Radchenko wrote: >>> May you submit a patch? > Max Nikulin writes: >> See the attachments. > > I tried to run make test with your patch applied, but I am getting > interactive prompt there: > > Visit tags table (default TAGS): You may press RET... It is due to invasive org-ctags behavior. Nick Dokos. org-ctags land grab. Mon, 20 Mar 2023 23:36:09 -0400. https://list.orgmode.org/87o7omg4ie.fsf@alphaville.usersys.redhat.com ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] org-ctags.el: Protect shell specials in directory name 2024-04-28 16:51 ` Max Nikulin @ 2024-04-28 16:55 ` Ihor Radchenko 2024-04-28 16:58 ` Max Nikulin 0 siblings, 1 reply; 25+ messages in thread From: Ihor Radchenko @ 2024-04-28 16:55 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode Max Nikulin <manikulin@gmail.com> writes: >> I tried to run make test with your patch applied, but I am getting >> interactive prompt there: >> >> Visit tags table (default TAGS): > > You may press RET... > > It is due to invasive org-ctags behavior. Tests must be fully automated. We use make test in CI and things like project-compile are non-interactive. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] org-ctags.el: Protect shell specials in directory name 2024-04-28 16:55 ` Ihor Radchenko @ 2024-04-28 16:58 ` Max Nikulin 2024-04-28 17:02 ` Ihor Radchenko 0 siblings, 1 reply; 25+ messages in thread From: Max Nikulin @ 2024-04-28 16:58 UTC (permalink / raw) To: emacs-orgmode On 28/04/2024 23:55, Ihor Radchenko wrote: > Max Nikulin <manikulin@gmail.com> writes: > >>> I tried to run make test with your patch applied, but I am getting >>> interactive prompt there: >>> >>> Visit tags table (default TAGS): >> >> You may press RET... >> >> It is due to invasive org-ctags behavior. > > Tests must be fully automated. We use make test in CI and things like > project-compile are non-interactive. I do not mind. Requiring unexpected user interactions is a feature of org-ctags. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] org-ctags.el: Protect shell specials in directory name 2024-04-28 16:58 ` Max Nikulin @ 2024-04-28 17:02 ` Ihor Radchenko 2024-04-29 10:26 ` Max Nikulin 0 siblings, 1 reply; 25+ messages in thread From: Ihor Radchenko @ 2024-04-28 17:02 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode Max Nikulin <manikulin@gmail.com> writes: >> Tests must be fully automated. We use make test in CI and things like >> project-compile are non-interactive. > > I do not mind. Requiring unexpected user interactions is a feature of > org-ctags. So, may you update the patch to make tests automated? -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] org-ctags.el: Protect shell specials in directory name 2024-04-28 17:02 ` Ihor Radchenko @ 2024-04-29 10:26 ` Max Nikulin 2024-04-29 13:12 ` Ihor Radchenko 0 siblings, 1 reply; 25+ messages in thread From: Max Nikulin @ 2024-04-29 10:26 UTC (permalink / raw) To: emacs-orgmode On 29/04/2024 00:02, Ihor Radchenko wrote: > Max Nikulin writes: > >>> Tests must be fully automated. We use make test in CI and things like >>> project-compile are non-interactive. >> >> I do not mind. Requiring unexpected user interactions is a feature of >> org-ctags. > > So, may you update the patch to make tests automated? Notice that new tests for org-ctags do not require user interactions. Try make test BTEST_RE=test-org-ctags/ ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] org-ctags.el: Protect shell specials in directory name 2024-04-29 10:26 ` Max Nikulin @ 2024-04-29 13:12 ` Ihor Radchenko 2024-04-29 16:54 ` [PATCH] org-ctags.el: Do not activate on load Max Nikulin 0 siblings, 1 reply; 25+ messages in thread From: Ihor Radchenko @ 2024-04-29 13:12 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode Max Nikulin <manikulin@gmail.com> writes: >> So, may you update the patch to make tests automated? > > Notice that new tests for org-ctags do not require user interactions. Try > > make test BTEST_RE=test-org-ctags/ Sure, but that does not change the fact that make test is broken after applying your patch. 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. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH] org-ctags.el: Do not activate on load 2024-04-29 13:12 ` Ihor Radchenko @ 2024-04-29 16:54 ` Max Nikulin 2024-04-30 10:02 ` Ihor Radchenko 0 siblings, 1 reply; 25+ messages in thread From: Max Nikulin @ 2024-04-29 16:54 UTC (permalink / raw) To: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 466 bytes --] 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? [-- Attachment #2: 0001-org-ctags.el-Define-unload-function.patch --] [-- Type: text/x-patch, Size: 4299 bytes --] From 0d260a0f260afb0f407d00b6a53ed2121a240203 Mon Sep 17 00:00:00 2001 From: Max Nikulin <manikulin@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 [-- Attachment #3: 0002-org-ctags.el-Do-not-activate-on-load.patch --] [-- Type: text/x-patch, Size: 4381 bytes --] From 5915811995f83fbfb89606bfa4f316d2b4521268 Mon Sep 17 00:00:00 2001 From: Max Nikulin <manikulin@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.fsf@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 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH] org-ctags.el: Do not activate on load 2024-04-29 16:54 ` [PATCH] org-ctags.el: Do not activate on load Max Nikulin @ 2024-04-30 10:02 ` Ihor Radchenko 2024-04-30 11:20 ` [PATCH] org.el: Call EXT-enable for `org-modules' (was Re: [PATCH] org-ctags.el: Do not activate on load) Max Nikulin 2024-04-30 12:59 ` [PATCH] org-ctags.el: Do not activate on load Ihor Radchenko 0 siblings, 2 replies; 25+ messages in thread From: Ihor Radchenko @ 2024-04-30 10:02 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode Max Nikulin <manikulin@gmail.com> writes: > Isn't it better to modify buggy org-ctags than to add various kludges to > tests? Yes, of course. Thanks for doing this! Applied, onto main. https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=735334445 https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=badb09d67 https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=0f0019e32 https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=3c01767f7 -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH] org.el: Call EXT-enable for `org-modules' (was Re: [PATCH] org-ctags.el: Do not activate on load) 2024-04-30 10:02 ` Ihor Radchenko @ 2024-04-30 11:20 ` Max Nikulin 2024-05-01 10:21 ` Ihor Radchenko 2024-04-30 12:59 ` [PATCH] org-ctags.el: Do not activate on load Ihor Radchenko 1 sibling, 1 reply; 25+ messages in thread From: Max Nikulin @ 2024-04-30 11:20 UTC (permalink / raw) To: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 532 bytes --] On 30/04/2024 17:02, Ihor Radchenko wrote: > Max Nikulin writes: > >> Isn't it better to modify buggy org-ctags than to add various kludges to >> tests? > > Applied, onto main. I was expecting that you would be against it since it is a breaking change. See a follow-up to Ihor Radchenko. Re: Autoloading side effects (was: Re: [BUG] org-mouse is activated without explicit require) Mon, 12 Dec 2022 10:25:16 +0000 <https://list.orgmode.org/871qp4j4vn.fsf@localhost> However now I am in doubts why org-ctags is not a minor mode. [-- Attachment #2: 0001-org.el-Call-EXT-enable-for-org-modules.patch --] [-- Type: text/x-patch, Size: 4796 bytes --] From 9cf1ebd0d8a0b1aa059f049aeb0c297b114aa4e2 Mon Sep 17 00:00:00 2001 From: Max Nikulin <manikulin@gmail.com> Date: Tue, 30 Apr 2024 18:05:11 +0700 Subject: [PATCH] org.el: Call EXT-enable for `org-modules' * lisp/org.el (org-load-modules-maybe): Activate extensions that implement EXT-enable functions. * etc/ORG-NEWS: * lisp/org-ctags.el: Update docs. It should simplify configuration for `org-ctags' users after badb09d67. The idea was discussed in Ihor Radchenko. Re: Autoloading side effects (was: Re: [BUG] org-mouse is activated without explicit require) Mon, 12 Dec 2022 10:25:16 +0000 <https://list.orgmode.org/871qp4j4vn.fsf@localhost> An alternative from the same thread is converting org modules to minor modes. --- etc/ORG-NEWS | 23 +++++++++++++++++++---- lisp/org-ctags.el | 5 ++++- lisp/org.el | 11 +++++++++-- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index 8dbc3292d..708d11f4f 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -651,10 +651,14 @@ 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 +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: +the library does not modify ~org-open-link-functions~ any more +when it is loaded using ~(require 'org-ctags)~ or a similar construct. +To setup hooks and advices either +[[*~feature-enable~ functions are called for extensions from ~org-modules~][customize ~org-modules~]] +and add ~org-ctags~ or run ~org-ctags-enable~ explicitly: #+begin_src emacs-lisp (with-eval-after-load "org-ctags" @@ -1473,10 +1477,21 @@ buffer it will be added. If not specified, new headings are created at level 1 at the end of the accessible part of the buffer, as before. ** Miscellaneous +*** ~feature-enable~ functions are called for extensions from ~org-modules~ + +Accordingly to Emacs [[info:elisp#Coding Conventions][coding conventions]] +loading a library must not modify behavior +since it may be done for the sake of help or command completion. +To make user configuration more convenient, when an extension ~EXT~ listed +in ~org-modules~ implements ~EXT-enable~ function, it is executed +during loading of Org mode or in response to customization of ~org-modules~. +So instead of ~(require 'EXT)~ in your init file add ~EXT~ to ~org-modules~. +See also +[[*~org-ctags~ is not activated by default any more][~org-ctags~ is not activated by default any more]]. + *** =org-crypt.el= now applies initial visibility settings to decrypted entries Previously, all the text was unfolded unconditionally, including property drawers. - *** Blank lines after removed objects are now retained during export When certain objects in Org document are to be excluded from export, diff --git a/lisp/org-ctags.el b/lisp/org-ctags.el index d3af103dd..9313c43cd 100644 --- a/lisp/org-ctags.el +++ b/lisp/org-ctags.el @@ -57,10 +57,13 @@ ;;; Commentary: ;; (add-hook 'org-mode-hook ;; (lambda () ;; (define-key org-mode-map "\C-co" 'org-ctags-find-tag-interactive))) +;; +;; To activate the library, you either need to add it to `org-modules' +;; or to call `org-ctags-enable' explicitly: +;; ;; (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'. ;; diff --git a/lisp/org.el b/lisp/org.el index 2d1a2055f..7030000ef 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -724,10 +724,17 @@ (defvar org-modules-loaded nil ;;;###autoload (defun org-load-modules-maybe (&optional force) - "Load all extensions listed in `org-modules'." + "Load all extensions listed in `org-modules'. +If an extension defines EXT-enable function then invoke it +to activate the module." (when (or force (not org-modules-loaded)) (dolist (ext org-modules) - (condition-case-unless-debug nil (require ext) + (condition-case-unless-debug nil + (let* ((feature (require ext)) + (enable-function (intern (concat (symbol-name feature) + "-enable")))) + (when (fboundp enable-function) + (funcall enable-function))) (error (message "Problems while trying to load feature `%s'" ext)))) (setq org-modules-loaded t))) -- 2.39.2 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH] org.el: Call EXT-enable for `org-modules' (was Re: [PATCH] org-ctags.el: Do not activate on load) 2024-04-30 11:20 ` [PATCH] org.el: Call EXT-enable for `org-modules' (was Re: [PATCH] org-ctags.el: Do not activate on load) Max Nikulin @ 2024-05-01 10:21 ` Ihor Radchenko 2024-05-01 11:38 ` Max Nikulin 0 siblings, 1 reply; 25+ messages in thread From: Ihor Radchenko @ 2024-05-01 10:21 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode Max Nikulin <manikulin@gmail.com> writes: >> Applied, onto main. > > I was expecting that you would be against it since it is a breaking change. > > See a follow-up to > Ihor Radchenko. Re: Autoloading side effects > (was: Re: [BUG] org-mouse is activated without explicit require) > Mon, 12 Dec 2022 10:25:16 +0000 > <https://list.orgmode.org/871qp4j4vn.fsf@localhost> Not against. The side effects in org-ctags are _disruptive_. And Bastien agreed that we can go ahead with breaking change in this case. Later in the thread we deviated to discussing what exactly constitutes a harmful side effect and what is acceptable, but that's a more general discussion. org-ctags is clearly too aggressive. > However now I am in doubts why org-ctags is not a minor mode. What would be the benefit? > ** Miscellaneous > +*** ~feature-enable~ functions are called for extensions from ~org-modules~ > + > +Accordingly to Emacs [[info:elisp#Coding Conventions][coding conventions]] > +loading a library must not modify behavior > +since it may be done for the sake of help or command completion. > +To make user configuration more convenient, when an extension ~EXT~ listed > +in ~org-modules~ implements ~EXT-enable~ function, it is executed > +during loading of Org mode or in response to customization of ~org-modules~. > +So instead of ~(require 'EXT)~ in your init file add ~EXT~ to ~org-modules~. > +See also > +[[*~org-ctags~ is not activated by default any more][~org-ctags~ is not activated by default any more]]. I'd like to discuss this idea with enable-feature on emacs-devel first. Maybe we can adapt it more widely without special handling for org-modules. See https://yhetil.org/emacs-devel/87zft9esva.fsf@localhost/ [FR] Provide a way to activate packages automatically for side effect -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] org.el: Call EXT-enable for `org-modules' (was Re: [PATCH] org-ctags.el: Do not activate on load) 2024-05-01 10:21 ` Ihor Radchenko @ 2024-05-01 11:38 ` Max Nikulin 2024-05-01 13:57 ` Ihor Radchenko 0 siblings, 1 reply; 25+ messages in thread From: Max Nikulin @ 2024-05-01 11:38 UTC (permalink / raw) To: emacs-orgmode On 01/05/2024 17:21, Ihor Radchenko wrote: > Max Nikulin writes: > >> However now I am in doubts why org-ctags is not a minor mode. > > What would be the benefit? Documented conventions how to enable or disable minor mode. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] org.el: Call EXT-enable for `org-modules' (was Re: [PATCH] org-ctags.el: Do not activate on load) 2024-05-01 11:38 ` Max Nikulin @ 2024-05-01 13:57 ` Ihor Radchenko 0 siblings, 0 replies; 25+ messages in thread From: Ihor Radchenko @ 2024-05-01 13:57 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode Max Nikulin <manikulin@gmail.com> writes: >>> However now I am in doubts why org-ctags is not a minor mode. >> >> What would be the benefit? > > Documented conventions how to enable or disable minor mode. I guess you can convert it into a global minor mode. I see no major downsides. So, patches welcome. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] org-ctags.el: Do not activate on load 2024-04-30 10:02 ` Ihor Radchenko 2024-04-30 11:20 ` [PATCH] org.el: Call EXT-enable for `org-modules' (was Re: [PATCH] org-ctags.el: Do not activate on load) Max Nikulin @ 2024-04-30 12:59 ` Ihor Radchenko 2024-04-30 13:37 ` Max Nikulin 1 sibling, 1 reply; 25+ messages in thread From: Ihor Radchenko @ 2024-04-30 12:59 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode Ihor Radchenko <yantar92@posteo.net> writes: > Applied, onto main. > https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=735334445 > https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=badb09d67 > https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=0f0019e32 > https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=3c01767f7 And we got test failures on CI: https://builds.sr.ht/~bzg/job/1208474 /bin/sh: 1: Syntax error: Bad function name (ert-test-failed ((should (test-org-ctags/list-elements-equal-p (list ...) (test-org-ctags/with-fake-ctags temp-dir "regexp" ...) '(2) "Regexp should be escaped.")) :form (test-org-ctags/list-elements-equal-p ("--regex-orgmode=/<<([^<>]+)>>/\\1/d,definition/") "Sould be overwritten by org-ctags mock script" (2) "Regexp should be escaped.") :value nil :explanation "Shell command failed")) FAILED 630/1311 test-org-ctags/create-tags-escape (0.003107 sec) at ../lisp/test-org-ctags.el:132 Any ideas? -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] org-ctags.el: Do not activate on load 2024-04-30 12:59 ` [PATCH] org-ctags.el: Do not activate on load Ihor Radchenko @ 2024-04-30 13:37 ` Max Nikulin 2024-04-30 15:31 ` Ihor Radchenko 0 siblings, 1 reply; 25+ messages in thread From: Max Nikulin @ 2024-04-30 13:37 UTC (permalink / raw) To: emacs-orgmode On 30/04/2024 19:59, Ihor Radchenko wrote: > /bin/sh: 1: Syntax error: Bad function name > (ert-test-failed > ((should > (test-org-ctags/list-elements-equal-p > (list ...) > (test-org-ctags/with-fake-ctags temp-dir "regexp" ...) > '(2) > "Regexp should be escaped.")) > :form > (test-org-ctags/list-elements-equal-p > ("--regex-orgmode=/<<([^<>]+)>>/\\1/d,definition/") > "Sould be overwritten by org-ctags mock script" > (2) > "Regexp should be escaped.") > :value nil :explanation "Shell command failed")) > FAILED 630/1311 test-org-ctags/create-tags-escape (0.003107 sec) at ../lisp/test-org-ctags.el:132 At least error message is not bad. Dash does not like ctags-mock() { ... } diff --git a/testing/lisp/test-org-ctags.el b/testing/lisp/test-org-ctags.el index 7f5fca948..b8e3e4d22 100644 --- a/testing/lisp/test-org-ctags.el +++ b/testing/lisp/test-org-ctags.el @@ -76,17 +76,17 @@ (defmacro test-org-ctags/with-fake-ctags (,dir (concat ,base "/" ,subdir)) (,temp-file (concat ,dir "/ctags.txt")) (org-ctags-path-to-ctags - (test-org-ctags/mock-command ,temp-file "ctags-mock")) + (test-org-ctags/mock-command ,temp-file "ctags_mock")) ,buffer) (make-directory ,dir) (unwind-protect ;; `org-ctags' commands call `buffer-file-name'. (with-current-buffer (setq ,buffer (find-file-noselect ,temp-file)) - (insert "Sould be overwritten by org-ctags mock script") + (insert "Should be overwritten by org-ctags mock script") (save-buffer) ,@body - (test-org-ctags/get-args ,temp-file ,base "ctags-mock\n")) + (test-org-ctags/get-args ,temp-file ,base "ctags_mock\n")) (kill-buffer ,buffer) (delete-file ,temp-file) (delete-directory ,dir))))) ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH] org-ctags.el: Do not activate on load 2024-04-30 13:37 ` Max Nikulin @ 2024-04-30 15:31 ` Ihor Radchenko 0 siblings, 0 replies; 25+ messages in thread From: Ihor Radchenko @ 2024-04-30 15:31 UTC (permalink / raw) To: Max Nikulin; +Cc: emacs-orgmode Max Nikulin <manikulin@gmail.com> writes: > diff --git a/testing/lisp/test-org-ctags.el b/testing/lisp/test-org-ctags.el > index 7f5fca948..b8e3e4d22 100644 > --- a/testing/lisp/test-org-ctags.el > +++ b/testing/lisp/test-org-ctags.el > @@ -76,17 +76,17 @@ (defmacro test-org-ctags/with-fake-ctags > (,dir (concat ,base "/" ,subdir)) > (,temp-file (concat ,dir "/ctags.txt")) > (org-ctags-path-to-ctags > - (test-org-ctags/mock-command ,temp-file "ctags-mock")) > + (test-org-ctags/mock-command ,temp-file "ctags_mock")) > ... Applied. Let's see how it goes. https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=c6bbde4c7 -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2024-05-01 13:57 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-02-09 23:57 [PATCH] `org-ctags-create-tags` creates empty TAGS file [9.6.15 (release_9.6.15 @ /home/martin/Projects/emacs/lisp/org/)] Martin Marshall 2024-02-10 4:49 ` Is there something people use instead of org-ctags? (was: [PATCH] `org-ctags-create-tags` creates empty TAGS file) Martin Marshall 2024-02-10 14:29 ` Ihor Radchenko 2024-02-10 14:27 ` [PATCH] `org-ctags-create-tags` creates empty TAGS file [9.6.15 (release_9.6.15 @ /home/martin/Projects/emacs/lisp/org/)] Ihor Radchenko 2024-02-10 21:10 ` Morgan Willcock 2024-02-10 21:20 ` Ihor Radchenko 2024-03-19 10:21 ` Max Nikulin 2024-03-20 12:08 ` Ihor Radchenko 2024-04-28 7:37 ` [PATCH] org-ctags.el: Protect shell specials in directory name Max Nikulin 2024-04-28 12:53 ` Ihor Radchenko 2024-04-28 16:51 ` Max Nikulin 2024-04-28 16:55 ` Ihor Radchenko 2024-04-28 16:58 ` Max Nikulin 2024-04-28 17:02 ` Ihor Radchenko 2024-04-29 10:26 ` Max Nikulin 2024-04-29 13:12 ` Ihor Radchenko 2024-04-29 16:54 ` [PATCH] org-ctags.el: Do not activate on load Max Nikulin 2024-04-30 10:02 ` Ihor Radchenko 2024-04-30 11:20 ` [PATCH] org.el: Call EXT-enable for `org-modules' (was Re: [PATCH] org-ctags.el: Do not activate on load) Max Nikulin 2024-05-01 10:21 ` Ihor Radchenko 2024-05-01 11:38 ` Max Nikulin 2024-05-01 13:57 ` Ihor Radchenko 2024-04-30 12:59 ` [PATCH] org-ctags.el: Do not activate on load Ihor Radchenko 2024-04-30 13:37 ` Max Nikulin 2024-04-30 15:31 ` Ihor Radchenko
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).