GNU bug report logs - #67687
Feature request: automatic tags management

Previous Next

Full log

Message #29 received at 67687 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67687 <at> debbugs.gnu.org, eskinjp <at> gmail.com, stefankangas <at> gmail.com
Subject: Re: bug#67687: Feature request: automatic tags management
Date: Thu, 21 Dec 2023 18:46:11 +0200

On 21/12/2023 09:40, Eli Zaretskii wrote:
>> Date: Thu, 21 Dec 2023 02:24:01 +0200
>> Cc: 67687 <at> debbugs.gnu.org
>> From: Dmitry Gutov <dmitry <at> gutov.dev>
>>
>> That is a good sign (with another positive bit of feedback on Reddit
>> yesterday), so I think it's time to ask the head maintainers what they
>> think about the inclusion of this feature in the core now.
>>
>> Eli/Stefan?
> 
> I didn't hear about any disadvantages; are there any issues we need to
> consider?

Nothing major, I believe. But someone in charge should make a yes-or-no 
decision.

Also, the patch adds a new value to .dir-locals.el (to make 
etags-regen-mode easy to use for Emacs development right away).

If someone edits their development branch with an older (e.g. stable) 
version of Emacs, this will be one more unsafe-variable warning to 
handle at least once.

> Does the etags test suite still pass after these changes?

I don't see why it would be affected. The mode is off by default anyway 
-- though I think it might be a good idea to flip it on after some more 
testing. But that is up to you and Stefan K. as well.

> Some comments based on a superficial look at the branch:
> 
>    . there are no updates for NEWS and the Emacs manual

I'll certainly add something to NEWS. Not sure where and what should be 
in the manual.

>    . the doc string of etags-regen-mode should explain more about what
>      it does

Fair.

>    . the new -L switch to etags is not mentioned in --help and in the
>      man page of etags

You're probably looking at the Git branch in the repository. See instead 
the patch attached to this bug report. I've removed a bunch of less 
essential changes.

Some to be considered for later (the project.el changes, which make file 
listing somewhat faster), and some not. The -L flag is in the latter 
category.

>    . defcustoms don't have a :version tag

Will add.

>    . etags-regen-lang-regexp-alist could have a shorter name:
>      etags-regen-regexp-alist, and its doc string should describe the
>      form of the alist

It seemed that "lang" is good to have in the name, so the variable's 
meaning is more obvious, but I don't really mind changing it.

>    . in the safe-local-variable form of etags-regen-lang-regexp-alist,
>      why do we force the language name to match a certain regexp, and
>      likewise with extensions in etags-regen-file-extensions?

That's mostly defensive programming. Indeed, shell-quote-argument should 
deal with most of the potential security problems.

>    . the shell command in etags-regen--all-mtimes is non-portable: it
>      needs xargs and stat commands; please use
>      directory-files-recursively with file-attributes instead, at least
>      as fallback

This is already done in the latest version (etags-regen.diff in the bug 
attachments).

Which brings a problem: the mode is now likely unusable over Tramp in 
any project of significant size. Something to improve later.

>    . I see several FIXMEs and TODOs in the code

I don't plan on addressing any of those before the first checkin.

>    . I wonder whether we should make sure etags supports the new -L
>      switch, and signal an error if not -- since you invoke etags
>      via PATH, it is quite possible to invoke an older version, the one
>      installed on the system, and not the one in the repository, as
>      long as Emacs 30 is not installed

Yeah, this is not a problem anymore, see above.

>    . why does etags-regen--tags-generate require 'dired?

A remnant of some older code. Removed it.

>    . why do we have to use advice-add/remove in etags-regen-mode? can't
>      we add hooks to the relevant functions instead?

We can, and probably will do. What I like about the current solution, 
though, is that it can be published to GNU ELPA (as "core" package) 
verbatim, and work with older emacsen as well.

>    . you are removing a project-files method from project.el -- isn't
>      that backward-incompatible?

Not in the latest patch.

Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.