bug#51102: [PATCH] Make speedbar XEmacs compatibility mode obsolete (etags)

bug#51102: [PATCH] Make speedbar XEmacs compatibility mode obsolete (etags)

[Stefan Kangas]

[-- Attachment #1: Type: text/plain, Size: 515 bytes --]

Severity: wishlist

The documentation for `speedbar-use-imenu-flag' says:

    (defcustom speedbar-use-imenu-flag t
      "Non-nil means use imenu for file parsing, nil to use etags.
    XEmacs prior to 20.4 doesn't support imenu, therefore the default is to
    use etags instead.  Etags support is not as robust as imenu support."
    :tag "Use Imenu for tags"
    :group 'speedbar
    :type 'boolean)

Is there any reason to keep the etags support around, or should we
install something like the attached patch?

[-- Attachment #2: 0001-Make-speedbar-XEmacs-compatibility-mode-obsolete-eta.patch --]
[-- Type: text/x-diff, Size: 7362 bytes --]

From beb2633f5c5642d2c9a9a5d9727ae9df3df7ff05 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Fri, 8 Oct 2021 23:09:50 +0200
Subject: [PATCH] Make speedbar XEmacs compatibility mode obsolete (etags)

* lisp/speedbar.el: (speedbar): Remove :group etags from defgroup.
(speedbar-dynamic-tags-function-list): Remove etags support that was
only kept around for XEmacs compatibility.
(speedbar-use-imenu-flag, speedbar-insert-etags-list)
(speedbar-extract-one-symbol, speedbar-parse-c-or-c++tag)
(speedbar-parse-tex-string, speedbar-fetch-etags-parse-list)
(speedbar-fetch-etags-command, speedbar-fetch-etags-arguments)
(speedbar-toggle-etags): Make obsolete.
(speedbar-tag-expand): Doc fix.
---
 etc/NEWS         |  7 +++++++
 lisp/speedbar.el | 43 +++++++++++++++----------------------------
 2 files changed, 22 insertions(+), 28 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index b91a5cbb72..32c8dbd8c2 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -105,6 +105,13 @@ also handle ANSI codes for faint, italic and blinking text, displaying
 it with new 'ansi-term-faint/italic/slow-blinking/fast-blinking'
 faces.
 
+** speedbar
+
+---
+*** etags support is now obsolete.
+This was only kept around for XEmacs compatibility, and the imenu
+support is more robust.
+
 \f
 * New Modes and Packages in Emacs 29.1
 
diff --git a/lisp/speedbar.el b/lisp/speedbar.el
index 3cc3e27606..96e668fc06 100644
--- a/lisp/speedbar.el
+++ b/lisp/speedbar.el
@@ -112,7 +112,6 @@
 ;; customization stuff
 (defgroup speedbar nil
   "File and tag browser frame."
-  :group 'etags
   :group 'tools
   :group 'convenience
   :link '(custom-manual "(speedbar) Top")
@@ -306,17 +305,17 @@ speedbar-frame-parameters
 		       (symbol :tag "Parameter")
 		       (sexp :tag "Value"))))
 
-(defcustom speedbar-use-imenu-flag (fboundp 'imenu)
+(defcustom speedbar-use-imenu-flag t
   "Non-nil means use imenu for file parsing, nil to use etags.
 XEmacs prior to 20.4 doesn't support imenu, therefore the default is to
 use etags instead.  Etags support is not as robust as imenu support."
   :tag "Use Imenu for tags"
   :group 'speedbar
   :type 'boolean)
+(make-obsolete-variable 'speedbar-use-imenu-flag nil "29.1")
 
 (defvar speedbar-dynamic-tags-function-list
-  '((speedbar-fetch-dynamic-imenu . speedbar-insert-imenu-list)
-    (speedbar-fetch-dynamic-etags . speedbar-insert-etags-list))
+  '((speedbar-fetch-dynamic-imenu . speedbar-insert-imenu-list))
   "Set to a list of functions which will return and insert a list of tags.
 Each element is of the form ( FETCH . INSERT ) where FETCH
 is a function which takes one parameter (the file to tag) and returns a
@@ -2350,6 +2349,7 @@ speedbar-insert-imenu-list
 
 (defun speedbar-insert-etags-list (indent lst)
   "At level INDENT, insert the etags generated LST."
+  (declare (obsolete nil "29.1"))
   (speedbar-insert-generic-list indent lst
 				'speedbar-tag-expand
 				'speedbar-tag-find))
@@ -3356,7 +3356,7 @@ speedbar-tag-find
 
 (defun speedbar-tag-expand (text token indent)
   "Expand a tag sublist.  Imenu will return sub-lists of specialized tag types.
-Etags does not support this feature.  TEXT will be the button string.
+TEXT will be the button string.
 TOKEN will be the list, and INDENT is the current indentation level."
   (cond ((string-search "+" text)	;we have to expand this file
 	 (speedbar-change-expand-button-char ?-)
@@ -3503,10 +3503,6 @@ speedbar-fetch-dynamic-tags
 
 ;;; Tag Management -- Imenu
 ;;
-(if (not speedbar-use-imenu-flag)
-
-    nil
-
 (eval-when-compile (condition-case nil (require 'imenu) (error nil)))
 (declare-function imenu--make-index-alist "imenu" (&optional no-error))
 
@@ -3524,7 +3520,7 @@ speedbar-fetch-dynamic-imenu
 		  (lambda (a b) (string< (car a) (car b))))
 	  index-alist))
     (error t)))
-)
+
 \f
 ;;; Tag Management -- etags  (old XEmacs compatibility part)
 ;;
@@ -3548,12 +3544,14 @@ speedbar-fetch-etags-parse-list
 to be at the beginning of a line in the etags buffer.
 
 This variable is ignored if `speedbar-use-imenu-flag' is non-nil.")
+(make-obsolete-variable 'speedbar-fetch-etags-parse-list nil "29.1")
 
 (defcustom speedbar-fetch-etags-command "etags"
   "Command used to create an etags file.
 This variable is ignored if `speedbar-use-imenu-flag' is t."
   :group 'speedbar
   :type 'string)
+(make-obsolete-variable 'speedbar-fetch-etags-command nil "29.1")
 
 (defcustom speedbar-fetch-etags-arguments '("-D" "-I" "-o" "-")
   "List of arguments to use with `speedbar-fetch-etags-command'.
@@ -3562,7 +3560,8 @@ speedbar-fetch-etags-arguments
 This variable is ignored if `speedbar-use-imenu-flag' is t."
   :group 'speedbar
   :type '(choice (const nil)
-		 (repeat :tag "List of arguments" string)))
+                 (repeat :tag "List of arguments" string)))
+(make-obsolete-variable 'speedbar-fetch-etags-arguments nil "29.1")
 
 (defun speedbar-toggle-etags (flag)
   "Toggle FLAG in `speedbar-fetch-etags-arguments'.
@@ -3573,6 +3572,7 @@ speedbar-toggle-etags
 
   This function is a convenience function for XEmacs menu created by
 Farzin Guilak <farzin@protocol.com>."
+  (declare (obsolete nil "29.1"))
   (interactive)
   (cond
    ((equal flag "sort")
@@ -3591,6 +3591,7 @@ speedbar-toggle-etags
 (defun speedbar-fetch-dynamic-etags (file)
   "For FILE, run etags and create a list of symbols extracted.
 Each symbol will be associated with its line position in FILE."
+  (declare (obsolete nil "29.1"))
   (let ((newlist nil))
     (unwind-protect
 	(save-excursion
@@ -3628,27 +3629,11 @@ speedbar-fetch-dynamic-etags
 	(sort newlist (lambda (a b) (string< (car a) (car b))))
       (reverse newlist))))
 
-;; This bit donated by Farzin Guilak <farzin@protocol.com> but I'm not
-;; sure it's needed with the different sorting method.
-;;
-;(defun speedbar-clean-etags()
-;  "Removes spaces before the ^? character, and removes `#define',
-;return types, etc. preceding tags.  This ensures that the sort operation
-;works on the tags, not the return types."
-;  (save-excursion
-;    (goto-char (point-min))
-;    (while
-;	(re-search-forward "(?[ \t](?\C-?" nil t)
-;      (replace-match "\C-?" nil nil))
-;    (goto-char (point-min))
-;    (while
-;	(re-search-forward "\\(.*[ \t]+\\)\\([^ \t\n]+.*\C-?\\)" nil t)
-;      (delete-region (match-beginning 1) (match-end 1)))))
-
 (defun speedbar-extract-one-symbol (expr)
   "At point, return nil, or one alist in the form (SYMBOL . POSITION).
 The line should contain output from etags.  Parse the output using the
 regular expression EXPR."
+  (declare (obsolete nil "29.1"))
   (let* ((sym (if (stringp expr)
 		  (if (save-excursion
 			(re-search-forward expr (line-end-position) t))
@@ -3668,6 +3653,7 @@ speedbar-extract-one-symbol
 
 (defun speedbar-parse-c-or-c++tag ()
   "Parse a C or C++ tag, which tends to be a little complex."
+  (declare (obsolete nil "29.1"))
   (save-excursion
     (let ((bound (line-end-position)))
       (cond ((re-search-forward "\C-?\\([^\C-a]+\\)\C-a" bound t)
@@ -3684,6 +3670,7 @@ speedbar-parse-c-or-c++tag
 
 (defun speedbar-parse-tex-string ()
   "Parse a Tex string.  Only find data which is relevant."
+  (declare (obsolete nil "29.1"))
   (save-excursion
     (let ((bound (line-end-position)))
       (cond ((re-search-forward "\\(\\(sub\\)*section\\|chapter\\|cite\\)\\s-*{[^\C-?}]*}?" bound t)
-- 
2.30.2

bug#51102: [PATCH] Make speedbar XEmacs compatibility mode obsolete (etags)

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Fri, 8 Oct 2021 17:36:04 -0400
> Cc: "Eric M. Ludlam" <zappo@gnu.org>
> 
>     (defcustom speedbar-use-imenu-flag t
>       "Non-nil means use imenu for file parsing, nil to use etags.
>     XEmacs prior to 20.4 doesn't support imenu, therefore the default is to
>     use etags instead.  Etags support is not as robust as imenu support."
>     :tag "Use Imenu for tags"
>     :group 'speedbar
>     :type 'boolean)
> 
> Is there any reason to keep the etags support around, or should we
> install something like the attached patch?

I see no reason to remove etags support.  I also don't understand the
"Etags support is not as robust as imenu support" part of the doc
string.

bug#51102: [PATCH] Make speedbar XEmacs compatibility mode obsolete (etags)

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> I also don't understand the "Etags support is not as robust as imenu
> support" part of the doc string.

The author seems to have held the opinion that etags is not relevant on
Emacs:

;;; Tag Management -- etags  (old XEmacs compatibility part)

That comment dates to 1998, when speedbar.el was first imported.

bug#51102: [PATCH] Make speedbar XEmacs compatibility mode obsolete (etags)

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sat, 9 Oct 2021 00:08:06 -0700
> Cc: 51102@debbugs.gnu.org, zappo@gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I also don't understand the "Etags support is not as robust as imenu
> > support" part of the doc string.
> 
> The author seems to have held the opinion that etags is not relevant on
> Emacs:
> 
> ;;; Tag Management -- etags  (old XEmacs compatibility part)
> 
> That comment dates to 1998, when speedbar.el was first imported.

Yes, I've seen that.  I'd like the author to elaborate on that: what
exactly is not robust in the etags support by Speedbar?

bug#51102: [PATCH] Make speedbar XEmacs compatibility mode obsolete (etags)

[Eric Ludlam]

[-- Attachment #1: Type: text/plain, Size: 1123 bytes --]

Hi all.

That was a very long time ago so I don't remember any specifics from then,
but I can think of two reasons imenu is better.

One, you don't need to save the file and run an external command.

Two, imenu has some nice binning it does to make the tags more navigable.

I don't recall specific issues with etags within speedbar though, so
"robust" is probably not the right word here.

Eric


On Sat, Oct 9, 2021, 3:52 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Stefan Kangas <stefan@marxist.se>
> > Date: Sat, 9 Oct 2021 00:08:06 -0700
> > Cc: 51102@debbugs.gnu.org, zappo@gnu.org
> >
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> > > I also don't understand the "Etags support is not as robust as imenu
> > > support" part of the doc string.
> >
> > The author seems to have held the opinion that etags is not relevant on
> > Emacs:
> >
> > ;;; Tag Management -- etags  (old XEmacs compatibility part)
> >
> > That comment dates to 1998, when speedbar.el was first imported.
>
> Yes, I've seen that.  I'd like the author to elaborate on that: what
> exactly is not robust in the etags support by Speedbar?
>

[-- Attachment #2: Type: text/html, Size: 2075 bytes --]

bug#51102: [PATCH] Make speedbar XEmacs compatibility mode obsolete (etags)

[Eli Zaretskii]

> From: Eric Ludlam <ericludlam@gmail.com>
> Date: Sun, 10 Oct 2021 12:31:43 -0400
> Cc: Stefan Kangas <stefan@marxist.se>, 51102@debbugs.gnu.org, 
> 	"Eric M. Ludlam" <zappo@gnu.org>
> 
> That was a very long time ago so I don't remember any specifics from then, but I can think of two reasons
> imenu is better.
> 
> One, you don't need to save the file and run an external command.
> 
> Two, imenu has some nice binning it does to make the tags more navigable.
> 
> I don't recall specific issues with etags within speedbar though, so "robust" is probably not the right word
> here.  

Thanks.

is it possible that some files will be in a programming language that
imenu doesn't support, but etags does?

bug#51102: [PATCH] Make speedbar XEmacs compatibility mode obsolete (etags)

[Eric Ludlam]

[-- Attachment #1: Type: text/plain, Size: 1224 bytes --]

Yes, that could be, though my recollection is that the opposite was more
common back in the day.  I don't know what the current balance might be.

I spent a lot of time pulling in a wide array of taggers to solve this type
of problem.  After speedbar I switched to putting them in semantic tags
instead which had more utility than the more focused speedbar feature while
still working fine in speedbar.

Eric

On Sun, Oct 10, 2021, 1:51 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Eric Ludlam <ericludlam@gmail.com>
> > Date: Sun, 10 Oct 2021 12:31:43 -0400
> > Cc: Stefan Kangas <stefan@marxist.se>, 51102@debbugs.gnu.org,
> >       "Eric M. Ludlam" <zappo@gnu.org>
> >
> > That was a very long time ago so I don't remember any specifics from
> then, but I can think of two reasons
> > imenu is better.
> >
> > One, you don't need to save the file and run an external command.
> >
> > Two, imenu has some nice binning it does to make the tags more navigable.
> >
> > I don't recall specific issues with etags within speedbar though, so
> "robust" is probably not the right word
> > here.
>
> Thanks.
>
> is it possible that some files will be in a programming language that
> imenu doesn't support, but etags does?
>

[-- Attachment #2: Type: text/html, Size: 2054 bytes --]

bug#51102: [PATCH] Make speedbar XEmacs compatibility mode obsolete (etags)

[Eli Zaretskii]

> From: Eric Ludlam <ericludlam@gmail.com>
> Date: Sun, 10 Oct 2021 13:57:58 -0400
> Cc: Stefan Kangas <stefan@marxist.se>, 51102@debbugs.gnu.org, 
> 	"Eric M. Ludlam" <zappo@gnu.org>
> 
> Yes, that could be, though my recollection is that the opposite was more common back in the day.  I don't
> know what the current balance might be.
> 
> I spent a lot of time pulling in a wide array of taggers to solve this type of problem.  After speedbar I switched
> to putting them in semantic tags instead which had more utility than the more focused speedbar feature
> while still working fine in speedbar.

OK, thanks.  So I think we should leave the etags support as an
optional feature, but remove the reference to XEmacs from the doc
string of the defcustom.

bug#51102: [PATCH] Make speedbar XEmacs compatibility mode obsolete (etags)

[Eric Ludlam]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

Sounds good.  Thanks.

On Sun, Oct 10, 2021, 2:37 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Eric Ludlam <ericludlam@gmail.com>
> > Date: Sun, 10 Oct 2021 13:57:58 -0400
> > Cc: Stefan Kangas <stefan@marxist.se>, 51102@debbugs.gnu.org,
> >       "Eric M. Ludlam" <zappo@gnu.org>
> >
> > Yes, that could be, though my recollection is that the opposite was more
> common back in the day.  I don't
> > know what the current balance might be.
> >
> > I spent a lot of time pulling in a wide array of taggers to solve this
> type of problem.  After speedbar I switched
> > to putting them in semantic tags instead which had more utility than the
> more focused speedbar feature
> > while still working fine in speedbar.
>
> OK, thanks.  So I think we should leave the etags support as an
> optional feature, but remove the reference to XEmacs from the doc
> string of the defcustom.
>

[-- Attachment #2: Type: text/html, Size: 1546 bytes --]

bug#51102: [PATCH] Make speedbar XEmacs compatibility mode obsolete (etags)

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> OK, thanks.  So I think we should leave the etags support as an
> optional feature, but remove the reference to XEmacs from the doc
> string of the defcustom.

I think we should also document under which circumstances one would want
to use etags instead of imenu.

For example, someone could check which languages have support in etags
but not in imenu and list them.

bug#51102: [PATCH] Make speedbar XEmacs compatibility mode obsolete (etags)

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Mon, 11 Oct 2021 07:18:24 -0700
> Cc: Eric Ludlam <ericludlam@gmail.com>, zappo@gnu.org, 51102@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > OK, thanks.  So I think we should leave the etags support as an
> > optional feature, but remove the reference to XEmacs from the doc
> > string of the defcustom.
> 
> I think we should also document under which circumstances one would want
> to use etags instead of imenu.
> 
> For example, someone could check which languages have support in etags
> but not in imenu and list them.

Sure, if we know that, or can find out, that'd be useful information.
The languages supported by etags can be seen in "etags --help".

bug#51102: Document speedbar etags compatibility mode better

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> I think we should also document under which circumstances one would want
> to use etags instead of imenu.
>
> For example, someone could check which languages have support in etags
> but not in imenu and list them.

I don't think we can usefully document this -- because keeping that
documentation up-to-date with both etags and imenu versions would
be too much work.  (And be inaccurate, since people have different etags
versions installed.)

So I think we can just leave this up to the users to decide for
themselves, and I'm therefore closing this bug report.