emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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
  0 siblings, 0 replies; 10+ 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] 10+ messages in thread

end of thread, other threads:[~2024-04-28 12:53 UTC | newest]

Thread overview: 10+ 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

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).