[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#67687: Feature request: automatic tags management
From: |
Dmitry Gutov |
Subject: |
bug#67687: Feature request: automatic tags management |
Date: |
Sat, 30 Dec 2023 05:05:01 +0200 |
User-agent: |
Mozilla Thunderbird |
On 28/12/2023 11:30, Eli Zaretskii wrote:
Date: Sun, 24 Dec 2023 03:43:25 +0200
From: Dmitry Gutov <dmitry@gutov.dev>
Cc: 67687@debbugs.gnu.org, eskinjp@gmail.com, stefankangas@gmail.com
On 22/12/2023 01:37, Dmitry Gutov wrote:
On 21/12/2023 18:46, Dmitry Gutov wrote:
See instead the patch attached to this bug report.
Here's an update, incorporating the feedback from here and there.
Fixed a typo in dir-locals and implemented better support for
etags-regen-ignores (though with one omission).
To me it looks good to check in now.
Thanks, I have a few minor comments.
Attaching the next version, which addresses most but not all comments of
yours and Stefan's.
+(defcustom etags-regen-program (executable-find "etags")
+ "Name of the etags program.
+
+If you only have `ctags' installed, you can also set this to
+\"ctags -e\". Some features might not be supported this way."
+ ;; Always having our 'etags' here would be easier, but we can't
+ ;; always rely on it being installed. So it might be ctags's etags.
+ :type 'file)
Please add :version tags to all the defcustoms you add.
Done.
+(defcustom etags-regen-tags-file "TAGS"
+ "Name of the tags file to create inside the project.
This and the other defcustom's here should say in the first line of
the doc string that they are for etags-regen-mode. This will help
discoverability and also produce a more helpful display with the
various apropos commands.
I've tried, but it seems hard to fit into most of them while keeping to
the requisite max number of columns. Only managed to fit that into
etags-regen-program and etags-regen-file-extensions.
TBH, most of the time it would seem superfluous, given the namespaced
names. But it's probably good to mention in 'etags-regen-program', on
balance.
+This value should either be a simple file name (no directory
^^^^^^^^^^
"The value" or just "Value".
Ok.
+specified), or a function that accepts a project root directory
+and returns a distinct file name for the tags file for it.
That function should also return only a file name without leading
directories, right? If so, the text should be more explicit about
that. For example:
Value should be either a string specifying a file name without
leading directories, or or a function that accepts a project's root
directory and returns such a file name, to be used as the tags file
for that project.
The
+latter option is most useful when you prefer to store the tag
^^^^^^
Using "option" in a doc string of a user option might be ambiguous. I
suggest to use "alternative" or "possibility" instead.
Done.
+files somewhere outside -- e.g. in `temporary-file-directory'."
So the function could return a file name _with_ leading directories?
Indeed. So I added "absolute" to the docstring.
We could allow it to return a nondirectory name too, if somebody finds a
use case.
+;;;###autoload
+(put 'etags-regen-file-extensions 'safe-local-variable
+ (lambda (value) (and (listp value) (seq-every-p #'stringp value))))
Why not use list-of-strings-p here?
Again, that "core ELPA" consideration. We could deploy this feature to a
number of released Emacs versions, if we don't introduce such dependencies.
+;;;###autoload
+(put 'etags-regen-ignores 'safe-local-variable
+ (lambda (value) (and (listp value) (seq-every-p #'stringp value))))
And here.
+ (if (> (+ (length added-files)
+ (length changed-files)
+ (length removed-files))
+ 100)
+ (progn
+ (message "etags-regen: Too many changes, falling back to full
rescan")
Should the magic 100 value be a defvar, not a hard-coded constant?
Moved it to a defvar.
+(defun etags-regen--maybe-generate ()
+ (let ((proj))
Would
(let (proj)
do here? IOW, why the extra pair of parens?
Altered.
+ (lambda (f) (or (not (string-match-p match-re f))
+ (string-match-p "/\\.#" f)
Is that '/' there to detect regexps for absolute file names? If so,
that won't work for Windows.
It's to detect backup files.
+(defun etags-regen--ignore-regexp (ignore)
+ (require 'dired)
+ ;; It's somewhat brittle to rely on Dired.
+ (let ((re (dired-glob-regexp ignore)))
+ ;; We could implement root anchoring here, but \\= doesn't work in
+ ;; string-match :-(.
+ (concat (unless (eq ?/ (aref re 3)) "/")
+ ;; Cutting off the anchors.
+ (substring re 2 (- (length re) 2))
+ (unless (eq ?/ (aref re (- (length re) 3)))
+ ;; Either match a full name segment, or eos.
+ "\\(?:/\\|\\'\\)"))))
Same here: what is the purpose of comparisons with a slash? I think
we need some more comments there explaining the logic of the code.
We compare with a slash to see whether the glob was matching against a
directory (in which case it's already anchored to the name of a file
name segment), otherwise we add such anchoring to either the end of a
file name segment or eos (thus allowing a glob match both directory
names and file names).
Added a shorter comment saying the same.
+(defun etags-regen--append-tags (&rest file-names)
+ (goto-char (point-max))
+ (let ((options (etags-regen--build-program-options (etags-regen--ctags-p)))
+ (inhibit-read-only t))
+ ;; XXX: call-process is significantly faster, though.
+ ;; Like 10ms vs 20ms here.
+ (shell-command
+ (format "%s %s %s -o -"
+ etags-regen-program (mapconcat #'identity options " ")
+ (mapconcat #'identity file-names " "))
+ t etags-regen--errors-buffer-name))
Should we indeed use call-process?
Something for later improvement.
Looking at the code, I believe I decided to use 'shell-command' for the
first version because of how easy it makes to output stderr to a
separate buffer. call-process only offers writing them to a file.
etags-regen-v4.diff
Description: Text Data
- bug#67687: Feature request: automatic tags management, (continued)
- bug#67687: Feature request: automatic tags management, Jon Eskin, 2023/12/07
- bug#67687: Feature request: automatic tags management, Dmitry Gutov, 2023/12/09
- bug#67687: Feature request: automatic tags management, Jon Eskin, 2023/12/10
- bug#67687: Feature request: automatic tags management, Jon Eskin, 2023/12/20
- bug#67687: Feature request: automatic tags management, Dmitry Gutov, 2023/12/20
- bug#67687: Feature request: automatic tags management, Eli Zaretskii, 2023/12/21
- bug#67687: Feature request: automatic tags management, Dmitry Gutov, 2023/12/21
- bug#67687: Feature request: automatic tags management, Dmitry Gutov, 2023/12/21
- bug#67687: Feature request: automatic tags management, Dmitry Gutov, 2023/12/23
- bug#67687: Feature request: automatic tags management, Eli Zaretskii, 2023/12/28
- bug#67687: Feature request: automatic tags management,
Dmitry Gutov <=
- bug#67687: Feature request: automatic tags management, Eli Zaretskii, 2023/12/30
- bug#67687: Feature request: automatic tags management, Dmitry Gutov, 2023/12/30
- bug#67687: Feature request: automatic tags management, Stefan Kangas, 2023/12/30
- bug#67687: Feature request: automatic tags management, Dmitry Gutov, 2023/12/31
- bug#67687: Feature request: automatic tags management, Eli Zaretskii, 2023/12/31
- bug#67687: Feature request: automatic tags management, Dmitry Gutov, 2023/12/31
- bug#67687: Feature request: automatic tags management, Stefan Kangas, 2023/12/29
- bug#67687: Feature request: automatic tags management, Dmitry Gutov, 2023/12/29
- bug#67687: Feature request: automatic tags management, Stefan Kangas, 2023/12/30
- bug#67687: Feature request: automatic tags management, Dmitry Gutov, 2023/12/30