Package: emacs;
Reported by: Stefan Kangas <stefan <at> marxist.se>
Date: Sun, 26 Jan 2020 16:15:02 UTC
Severity: wishlist
Tags: patch
Fixed in version 28.1
Done: Stefan Kangas <stefan <at> marxist.se>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 39293 in the body.
You can then email your comments to 39293 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Sun, 26 Jan 2020 16:15:02 GMT) Full text and rfc822 format available.Stefan Kangas <stefan <at> marxist.se>
:bug-gnu-emacs <at> gnu.org
.
(Sun, 26 Jan 2020 16:15:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Stefan Kangas <stefan <at> marxist.se> To: bug-gnu-emacs <bug-gnu-emacs <at> gnu.org> Subject: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Sun, 26 Jan 2020 04:13:27 +0100
[Message part 1 (text/plain, inline)]
The attached patch changes bookmark-bmenu-mode to be based on tabulated-list-mode instead of special-mode. This allows us to simplify the code in several cases. In addition, we get many features for free, such as sorting columns by clicking on the column headers, changing size of columns. In the future, this will obviously include any new feature added to 'tabulated-list-mode'. The only functional step backwards is that we no longer support the optional "inline" header line -- a bookmark.el-specific hack to have a header without using 'header-line-format'. I don't believe this feature is very useful since the lack of such support for anything similar in e.g. 'package-menu-mode' has not caused any problems. It seems to have been added together with 'header-line-format' as a fire escape if the latter caused any problems. I recently added a number of tests to bookmark.el on master, which were developed as part of this suggested change. These tests pass using both the new and the old code, which gives some degree of confidence in this change. Any comments, objections or suggestions? Thanks. Best regards, Stefan Kangas
[0001-Base-bookmark-bmenu-mode-on-tabulated-list-mode.patch (text/x-diff, attachment)]
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Sun, 26 Jan 2020 18:08:02 GMT) Full text and rfc822 format available.Message #8 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Drew Adams <drew.adams <at> oracle.com> To: Stefan Kangas <stefan <at> marxist.se>, 39293 <at> debbugs.gnu.org Subject: RE: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Sun, 26 Jan 2020 10:05:48 -0800 (PST)
> The attached patch changes bookmark-bmenu-mode to be based on > tabulated-list-mode instead of special-mode. > > This allows us to simplify the code in several cases. In addition, we > get many features for free, such as sorting columns by clicking on the > column headers, changing size of columns. In the future, this will > obviously include any new feature added to 'tabulated-list-mode'. > > The only functional step backwards is that we no longer support the > optional "inline" header line -- a bookmark.el-specific hack to have a > header without using 'header-line-format'. I don't believe this > feature is very useful since the lack of such support for anything > similar in e.g. 'package-menu-mode' has not caused any problems. It > seems to have been added together with 'header-line-format' as a fire > escape if the latter caused any problems. > > I recently added a number of tests to bookmark.el on master, which > were developed as part of this suggested change. These tests pass > using both the new and the old code, which gives some degree of > confidence in this change. > > Any comments, objections or suggestions? Thanks. I _strongly_ object to this. And I would say the same thing if such a suggestion were made for Dired. If vanilla Emacs does this then I will have to separate Bookmark+ completely from `bookmark.el', incorporating its code prior to your change. I don't want to do that, but I will have to (and it won't be hard to do - that's not the problem). Until now, I've made a concerted effort to be compatible with vanilla `bookmark.el', for the benefit of users. It's very wrong to think either that things like the bookmark-list display and Dired's listings are as simple as what `tabulated-list-mode' provides, or that their features can be easily added on top of `tabulated-list-mode'. This kind of proposal is, IMO, a consequence of one or both of the following: 1. Favoring development, or rather maintenance, over user convenience, power, and features. The imagined gain is a chimera. The code has been used for a very long time, and there is little maintenance burden. 2. Not appreciating the specificity of the features offered by libraries such as Dired and bookmarking. Not being well acquainted with such features, and so supposing that they don't exist or they're no big deal and not worth bothering about. Being acquainted with `tabulated-list-mode', and thinking its features are wonderful, abundant, and sufficiently general and flexible. Consider sorting, as just one example among many (yes, many). Sorting in Dired or the bookmark list (at least with Bookmark+) is _much more_ useful and flexible than just sorting columns. Why? Because the things displayed are of different kinds across rows, not just across columns. The columns, for bookmark-list display, are less interesting (this is less true of Dired, but the same consideration holds). Here are the predefined ways you can sort bookmarks with Bookmark+. And users can easily define their own ways of sorting, just as they can define their own kinds of bookmarks. key binding --- ------- s C-r bmkp-reverse-multi-sort-order s * bmkp-bmenu-sort-modified-before-unmodified s 0 bmkp-bmenu-sort-by-creation-time s > bmkp-bmenu-sort-marked-before-unmarked s D bmkp-bmenu-sort-flagged-before-unflagged s I bmkp-bmenu-sort-by-Info-position s a bmkp-bmenu-sort-annotated-before-unannotated s b bmkp-bmenu-sort-by-last-buffer-or-file-access s d bmkp-bmenu-sort-by-last-bookmark-access s f d bmkp-bmenu-sort-by-last-local-file-access s f k bmkp-bmenu-sort-by-local-file-type s f n bmkp-bmenu-sort-by-file-name s f s bmkp-bmenu-sort-by-local-file-size s f u bmkp-bmenu-sort-by-last-local-file-update s g bmkp-bmenu-sort-by-Gnus-thread s i bmkp-bmenu-sort-by-Info-node-name s k bmkp-bmenu-sort-by-bookmark-type s n bmkp-bmenu-sort-by-bookmark-name s r bmkp-reverse-sort-order s s bmkp-bmenu-change-sort-order-repeat s t bmkp-bmenu-sort-tagged-before-untagged s u bmkp-bmenu-sort-by-url s v bmkp-bmenu-sort-by-bookmark-visit-frequency What's more, you can combine sort orders - see https://www.emacswiki.org/emacs/BookmarkPlus#SortingBookmarks --- May I invite you to please spend your (much-appreciated) volunteer effort on something else? This isn't broken, and your proposal would shatter it. Do I use `tabulated-list-mode', or am I just grousing, as an old fart stuck in his ways and unfamiliar with `tabulated-list-mode'? Yes, I use it (e.g. in my library `apu.el'). But it is very limited, and the limitations do not just stem from a lack of more generic features. I use it only when it's appropriate. There's only so much you will ever be able to get out of `tabulated-list-mode'. Apply it so simple listings that don't need or offer much functionality. Please leave the sophisticated, useful listing displays alone. This is a have-a-hammer-and-see-only-nails story.
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Sun, 26 Jan 2020 19:34:02 GMT) Full text and rfc822 format available.Message #11 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Stefan Kangas <stefan <at> marxist.se> To: Drew Adams <drew.adams <at> oracle.com> Cc: 39293 <at> debbugs.gnu.org Subject: Re: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Sun, 26 Jan 2020 20:33:26 +0100
Drew Adams <drew.adams <at> oracle.com> writes: >> Any comments, objections or suggestions? Thanks. > > I _strongly_ object to this. And I would say the same > thing if such a suggestion were made for Dired. - This is not about Dired, so let's please leave that to one side. - This is not just code cleanup. I think 'tabulated-list-mode' gives clear benefits to users today. Sorting is one example which will immediately improve. Mouse support is another. - I'm not convinced that it's necessary, but I don't see the problem with an interactive bookmark menu in bookmark+.el (or any third-party package really) that is not directly based on the code in Emacs. > 2. Not appreciating the specificity of the features > offered by libraries such as Dired and bookmarking. > > Not being well acquainted with such features, and > so supposing that they don't exist or they're no > big deal and not worth bothering about. Which features are you talking about? > Sorting in Dired or the bookmark list (at least with > Bookmark+) is _much more_ useful and flexible than > just sorting columns. Why? Because the things > displayed are of different kinds across rows, not > just across columns. The columns, for bookmark-list > display, are less interesting (this is less true of > Dired, but the same consideration holds). I don't understand this argument. It seems to be based on the misunderstanding that 'tabulated-list-mode' somehow limits you to sort by column. > Yes, I use it (e.g. in my library `apu.el'). But it > is very limited, and the limitations do not just stem > from a lack of more generic features. I use it only > when it's appropriate. To my mind, this is the most convincing argument you presented. Yet you have not shown any limitations in 'tabulated-list-mode' that makes it unsuitable for 'bookmark-bmenu-mode'. Could you please explain which limitations you see, and how you concretely see them affecting bookmark-bmenu-mode? Best regards, Stefan Kangas
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Sun, 26 Jan 2020 22:36:02 GMT) Full text and rfc822 format available.Message #14 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Drew Adams <drew.adams <at> oracle.com> To: Stefan Kangas <stefan <at> marxist.se> Cc: 39293 <at> debbugs.gnu.org Subject: RE: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Sun, 26 Jan 2020 14:35:23 -0800 (PST)
I think I've said what I have to say about this.
Noam Postavsky <npostavs <at> gmail.com>
to control <at> debbugs.gnu.org
.
(Thu, 27 Feb 2020 12:50:01 GMT) Full text and rfc822 format available.bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Sun, 26 Apr 2020 15:00:02 GMT) Full text and rfc822 format available.Message #19 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Stefan Kangas <stefan <at> marxist.se> To: Drew Adams <drew.adams <at> oracle.com> Cc: 39293 <at> debbugs.gnu.org Subject: Re: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Sun, 26 Apr 2020 16:59:48 +0200
Drew Adams <drew.adams <at> oracle.com> writes: > I think I've said what I have to say about this. Revisiting this now, I would have been interested to know the answer to this question: "Could you please explain which limitations you see, and how you concretely see them affecting bookmark-bmenu-mode?" I would also be interested to know if there is anything that could be done to improve 'tabulated-list-mode' to get rid of any such limitations. Best regards, Stefan Kangas
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Sat, 23 May 2020 20:32:01 GMT) Full text and rfc822 format available.Message #22 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Matthias Meulien <orontee <at> gmail.com> To: Stefan Kangas <stefan <at> marxist.se> Cc: 39293 <at> debbugs.gnu.org Subject: Re: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Sat, 23 May 2020 22:31:06 +0200
Stefan Kangas <stefan <at> marxist.se> writes: > The attached patch changes bookmark-bmenu-mode to be based on > tabulated-list-mode instead of special-mode. > > This allows us to simplify the code in several cases. In > addition, we get many features for free, such as sorting columns > by clicking on the column headers, changing size of columns. Both features looks interesting to me. Thanks for working on that! > In the future, this will obviously include any new feature added > to 'tabulated-list-mode'. > > The only functional step backwards is that we no longer support > the optional "inline" header line -- a bookmark.el-specific hack > to have a header without using 'header-line-format'. I don't > believe this feature is very useful since the lack of such > support for anything similar in e.g. 'package-menu-mode' has not > caused any problems. It seems to have been added together with > 'header-line-format' as a fire escape if the latter caused any > problems. I am the one who introduced `header-line-format` in bookmark.el (7a78e19f24) and I confirm that I kept the legacy header "just in case". The new behavior has been enabled by default since 2013, and I've not heard any complaint. -- Matthias Meulien
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Sat, 23 May 2020 21:02:01 GMT) Full text and rfc822 format available.Message #25 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Stefan Kangas <stefan <at> marxist.se> To: Matthias Meulien <orontee <at> gmail.com> Cc: 39293 <at> debbugs.gnu.org Subject: Re: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Sat, 23 May 2020 14:01:21 -0700
Matthias Meulien <orontee <at> gmail.com> writes: > Both features looks interesting to me. Thanks for working on that! Thanks! >> The only functional step backwards is that we no longer support >> the optional "inline" header line -- a bookmark.el-specific hack >> to have a header without using 'header-line-format'. I don't >> believe this feature is very useful since the lack of such >> support for anything similar in e.g. 'package-menu-mode' has not >> caused any problems. It seems to have been added together with >> 'header-line-format' as a fire escape if the latter caused any >> problems. > > I am the one who introduced `header-line-format` in bookmark.el > (7a78e19f24) and I confirm that I kept the legacy header "just in > case". The new behavior has been enabled by default since 2013, > and I've not heard any complaint. This is useful information which confirms that we can safely remove it. Best regards, Stefan Kangas
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Sat, 23 May 2020 21:19:01 GMT) Full text and rfc822 format available.Message #28 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Stefan Kangas <stefan <at> marxist.se> To: Drew Adams <drew.adams <at> oracle.com> Cc: 39293 <at> debbugs.gnu.org Subject: Re: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Sat, 23 May 2020 14:18:13 -0700
Hi Drew, Just to bring your attention to the below message, which your email provider bounced for some reason (and I missed it did so at the time). Stefan Kangas <stefan <at> marxist.se> writes: > Revisiting this now, I would have been interested to know the answer > to this question: "Could you please explain which limitations you see, > and how you concretely see them affecting bookmark-bmenu-mode?" > > I would also be interested to know if there is anything that could be > done to improve 'tabulated-list-mode' to get rid of any such > limitations. As you understand, my preference is in favour of the change. But I've been uncomfortable pushing a change that basically only received negative feedback. Now we have had another person express support for the change, which would make the consensus lean more in its favour. Ideally, of course, we would be able to resolve the issues involved to everyones satisfaction. I think we need to clarify the above questions to be able to figure out if that's doable or not. Thanks in advance. Best regards, Stefan Kangas
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Sat, 23 May 2020 21:29:01 GMT) Full text and rfc822 format available.Message #31 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Drew Adams <drew.adams <at> oracle.com> To: Matthias Meulien <orontee <at> gmail.com>, Stefan Kangas <stefan <at> marxist.se> Cc: Karl Fogel <kfogel <at> red-bean.com>, 39293 <at> debbugs.gnu.org Subject: RE: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Sat, 23 May 2020 14:26:17 -0700 (PDT)
> > The attached patch changes bookmark-bmenu-mode to be based on > > tabulated-list-mode instead of special-mode. > > > > This allows us to simplify the code in several cases. In > > addition, we get many features for free, such as sorting columns > > by clicking on the column headers, changing size of columns. > > Both features looks interesting to me. Thanks for working on that! > > > In the future, this will obviously include any new feature added > > to 'tabulated-list-mode'. > > > > The only functional step backwards is that we no longer support > > the optional "inline" header line -- a bookmark.el-specific hack > > to have a header without using 'header-line-format'. I don't > > believe this feature is very useful since the lack of such > > support for anything similar in e.g. 'package-menu-mode' has not > > caused any problems. It seems to have been added together with > > 'header-line-format' as a fire escape if the latter caused any > > problems. Please don't do that. As I said earlier: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=39293#8 this is totally incompatible with the many enhancements I've made to bookmarks. It will force me to include the (former) bookmark.el code in Bookmark+, and separate the latter completely from bookmark.el. Until now, it's been a compatible extension. Tabulated-list mode is a step backward for any feature that offers more. Just because something is capable of displaying some info in columns, that doesn't mean it should be reduced to the limitations of t-l mode. Thinking that t-l mode is for all columnar presentations and interactions is akin to thinking that a soda straw is appropriate for an oil pipeline. This is serious for me. If you don't care, too bad for me, and for anyone who cares about Bookmark+ and compatibility with vanilla Emacs. > I am the one who introduced `header-line-format in bookmark.el > (7a78e19f24) and I confirm that I kept the legacy header "just in > case". The new behavior has been enabled by default since 2013, > and I've not heard any complaint. I would have complained about it. But I've been able to ignore it, because a "legacy header" is still supported. Enabled only by default means that hearing no complaints signifies nothing. If a user option is provided to let users NOT have tabulated-list display imposed, and let them continue as before, then fine. Otherwise I'll be forced to fork, I'm afraid.
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Sat, 23 May 2020 21:32:02 GMT) Full text and rfc822 format available.Message #34 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Drew Adams <drew.adams <at> oracle.com> To: Stefan Kangas <stefan <at> marxist.se> Cc: 39293 <at> debbugs.gnu.org Subject: RE: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Sat, 23 May 2020 14:31:01 -0700 (PDT)
Hi Stefan, Our mails crossed just now. > Just to bring your attention to the below message, which your email > provider bounced for some reason (and I missed it did so at the > time). > > Stefan Kangas <stefan <at> marxist.se> writes: > > > Revisiting this now, I would have been interested to know the answer > > to this question: "Could you please explain which limitations you > > see, and how you concretely see them affecting bookmark-bmenu-mode?" See my message of Jan 26 (url in previous mail). > > I would also be interested to know if there is anything that could be > > done to improve 'tabulated-list-mode' to get rid of any such > > limitations. Provide a user option. Make use of tabulated-list mode optional. > As you understand, my preference is in favour of the change. But I've > been uncomfortable pushing a change that basically only received > negative feedback. Now we have had another person express support for > the change, which would make the consensus lean more in its favour. > > Ideally, of course, we would be able to resolve the issues involved to > everyones satisfaction. I think we need to clarify the above questions > to be able to figure out if that's doable or not.
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Sat, 23 May 2020 21:45:01 GMT) Full text and rfc822 format available.Message #37 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Drew Adams <drew.adams <at> oracle.com> To: Stefan Kangas <stefan <at> marxist.se> Cc: 39293 <at> debbugs.gnu.org Subject: RE: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Sat, 23 May 2020 14:44:16 -0700 (PDT)
BTW, I already lived through this with buff-menu.el. My library buff-menu+.el offered features it didn't. But unlike Bookmark+, it wasn't such a big deal that I cared to bother about it. So buff-menu+.el doesn't work with Emacs > release 24.1. Not a big deal, in that case, and tabulated-list mode is, in fact, a reasonable fit for the buffer menu, which is a small feature with few complications or bells and whistles. https://www.emacswiki.org/emacs/BufferMenuPlus But you'll notice that ibuffer.el has _not_ been stuffed into the tabulated-list mold. Why not? It certainly could be. But it's more feature-rich than buff-menu. (I don't have an ibuffer extension library, so I won't scream when someone decides to clean house and shove tabulated-list down ibuffer's throat.) The difference with Bookmark+ is that ibuffer.el is part of Emacs proper. Though Bookmark+ has users, their complaints wouldn't have the same effect as the complaints of ibuffer users.
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Sat, 23 May 2020 22:17:02 GMT) Full text and rfc822 format available.Message #40 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Stefan Kangas <stefan <at> marxist.se> To: Drew Adams <drew.adams <at> oracle.com> Cc: 39293 <at> debbugs.gnu.org Subject: RE: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Sat, 23 May 2020 15:16:12 -0700
Drew Adams <drew.adams <at> oracle.com> writes: >> > Revisiting this now, I would have been interested to know the answer >> > to this question: "Could you please explain which limitations you >> > see, and how you concretely see them affecting bookmark-bmenu-mode?" > > See my message of Jan 26 (url in previous mail). Thanks, that unfortunately takes us back to square one. As you know, I've tried to understand that email before, and posed a series of questions in a reply the same day. Could we please restart the discussion from there? 1. You wrote that I don't appreciate "the specificity of the features offered by libraries such as Dired and bookmarking". I replied: "Which features are you talking about?" Could you please clarify this point? 2. You wrote: > Sorting in Dired or the bookmark list (at least with > Bookmark+) is _much more_ useful and flexible than > just sorting columns. Why? Because the things > displayed are of different kinds across rows, not > just across columns. I replied that AFAIU you are not limited to sorting across columns using tabulated-list-mode. Can we therefore consider that part settled? 3. You wrote: > Yes, I use it (e.g. in my library `apu.el'). But it > is very limited, and the limitations do not just stem > from a lack of more generic features. I use it only > when it's appropriate. I replied: "Could you please explain which limitations you see, and how you concretely see them affecting bookmark-bmenu-mode?" Could you please clarify this point? > Provide a user option. Make use of tabulated-list > mode optional. Maybe, but first we need to understand why it is even needed in the first place. Best regards, Stefan Kangas
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Tue, 26 May 2020 17:44:02 GMT) Full text and rfc822 format available.Message #43 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Karl Fogel <kfogel <at> red-bean.com> To: Drew Adams <drew.adams <at> oracle.com> Cc: Stefan Kangas <stefan <at> marxist.se>, Matthias Meulien <orontee <at> gmail.com>, 39293 <at> debbugs.gnu.org Subject: Re: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Tue, 26 May 2020 12:43:52 -0500
I too would like to hear what specifically would be limited or broken by switching to use `tabulated-list-mode' here. This does not mean I'm necessarily in favor of applying the patch; I just think Stefan's question is important, and I don't see it given a concrete answer anywhere. What would break in Bookmark+ if this patch were applied to bookmark.el, and/or what new limitations would bookmark.el after this patch? IOW, I'd just like to understand the tradeoffs better here. Best regards, -Karl
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Tue, 26 May 2020 20:02:01 GMT) Full text and rfc822 format available.Message #46 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Drew Adams <drew.adams <at> oracle.com> To: Karl Fogel <kfogel <at> red-bean.com> Cc: Stefan Kangas <stefan <at> marxist.se>, Matthias Meulien <orontee <at> gmail.com>, 39293 <at> debbugs.gnu.org Subject: RE: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Tue, 26 May 2020 13:02:04 -0700 (PDT)
> I too would like to hear what specifically would be limited or broken > by switching to use `tabulated-list-mode' here. > > This does not mean I'm necessarily in favor of applying the patch; I > just think Stefan's question is important, and I don't see it given a > concrete answer anywhere. What would break in Bookmark+ if this patch > were applied to bookmark.el, and/or what new limitations would > bookmark.el after this patch? > > IOW, I'd just like to understand the tradeoffs better here. Sorry, I really don't have the time to deal with this now. Bookmark+ has lots of added features, and many have to do with the bookmark-list display. If you're interested in that, please see its doc. Maybe at some point later I'll have the time and will to tick off things in Bookmark+ that t-l-mode interferes with or prohibits. I think at this point you'll just have to take my word for it (or not) that it would be far too much work, too risky, and for no benefit, for me to try to rewrite such features to adapt to t-l-mode. And I'm pretty sure that some things would need to be sacrificed. In my experience just with `apu.el' (which uses t-l-mode) I ran into limitations that I had to work around (no, I don't recall what they were). [Can a t-l-mode buffer even have a title (not just column headings)? With Bookmark+ the listing reflects the current sorting and filterings, and the title at the top tells you what the listing is about.] I'm sorry to say it, but I won't try, for Bookmark+. I don't have the time to waste on that. Sorry. Do what you think you have to do. I'll do what I have to do, given my limited resources. I expect I'll likely just incorporate the former bookmark.el code that Bookmark+ currently takes for granted. ___ FWIW, I also don't think that bookmark.el's list of bookmarks is a great candidate for t-l-mode. I don't think it adds anything important for such a simple list with 2-3 columns. As I said in my Jan 26 mail, sorting by those columns (which is really all that t-l-mode offers here) is not so helpful. (OK, it has some use.) Maybe consider ibuffer.el instead? As I said earlier, trying ibuffer, which offers a bit more than a rudimentary listing, might point to some t-l-mode limitations or complications. And if it doesn't then so much the better. ;-) ___ To be clear, I don't think I said that anything would be limited or broken in _bookmark.el_ by using t-l-mode. Potential uses of its features, and existing uses by 3rd-party libraries (e.g. Bookmark+), could be limited or broken. But I doubt that anything bookmark.el offers out of the box would be affected much, if at all. (bookmark.el could probably drop a good deal of its code without breaking anything that people use much). Thx - Drew
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Tue, 26 May 2020 20:39:02 GMT) Full text and rfc822 format available.Message #49 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Karl Fogel <kfogel <at> red-bean.com> To: Drew Adams <drew.adams <at> oracle.com> Cc: Stefan Kangas <stefan <at> marxist.se>, Matthias Meulien <orontee <at> gmail.com>, 39293 <at> debbugs.gnu.org Subject: Re: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Tue, 26 May 2020 15:38:16 -0500
On 26 May 2020, Drew Adams wrote: >Sorry, I really don't have the time to deal with >this now. > >Bookmark+ has lots of added features, and many >have to do with the bookmark-list display. If >you're interested in that, please see its doc. > >Maybe at some point later I'll have the time and >will to tick off things in Bookmark+ that t-l-mode >interferes with or prohibits. > >I think at this point you'll just have to take my >word for it (or not) that it would be far too much >work, too risky, and for no benefit, for me to try >to rewrite such features to adapt to t-l-mode. Okay, fair enough -- I understand about limited time. >And I'm pretty sure that some things would need to >be sacrificed. In my experience just with `apu.el' >(which uses t-l-mode) I ran into limitations that >I had to work around (no, I don't recall what they >were). > >[Can a t-l-mode buffer even have a title (not just >column headings)? With Bookmark+ the listing >reflects the current sorting and filterings, and >the title at the top tells you what the listing is >about.] > >I'm sorry to say it, but I won't try, for Bookmark+. >I don't have the time to waste on that. Sorry. > >Do what you think you have to do. I'll do what I >have to do, given my limited resources. I expect >I'll likely just incorporate the former bookmark.el >code that Bookmark+ currently takes for granted. Well, I don't think we "have to" convert bookmark.el to use t-l-mode at all. >FWIW, I also don't think that bookmark.el's list >of bookmarks is a great candidate for t-l-mode. >I don't think it adds anything important for such >a simple list with 2-3 columns. As I said in my >Jan 26 mail, sorting by those columns (which is >really all that t-l-mode offers here) is not so >helpful. (OK, it has some use.) > >Maybe consider ibuffer.el instead? As I said >earlier, trying ibuffer, which offers a bit more >than a rudimentary listing, might point to some >t-l-mode limitations or complications. And if >it doesn't then so much the better. ;-) Well, I mean, bookmark.el seems to be working fine the way it is right now, without t-l-mode nor ibuffer. So I'm not sure the proposed change is warranted. Stefan, is there a strong motivation here, other than the obvious attractions of re-using code and avoiding multiple implementations of similar functionality? (If we decide not to make the change, then we should add a comment to bookmark.el pointing to this bug-ticket discussion and explaining *why* we have left the code as-is.) >To be clear, I don't think I said that anything >would be limited or broken in _bookmark.el_ by >using t-l-mode. Potential uses of its features, >and existing uses by 3rd-party libraries (e.g. >Bookmark+), could be limited or broken. But I >doubt that anything bookmark.el offers out of >the box would be affected much, if at all. >(bookmark.el could probably drop a good deal of >its code without breaking anything that people >use much). Agreed; that's what I understood you to be saying. Best regards, -Karl
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Tue, 26 May 2020 21:44:02 GMT) Full text and rfc822 format available.Message #52 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Drew Adams <drew.adams <at> oracle.com> To: Karl Fogel <kfogel <at> red-bean.com> Cc: Stefan Kangas <stefan <at> marxist.se>, Matthias Meulien <orontee <at> gmail.com>, 39293 <at> debbugs.gnu.org Subject: RE: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Tue, 26 May 2020 14:41:12 -0700 (PDT)
Sounds reasonable. Thanks for listening. Obviously I'd prefer to keep Bookmark+ as an extension of bookmark.el. Among other things, that might make my job easier (dunno), and I'd pay attention more to changes (e.g. improvements) to bookmark.el. The *bmenu* stuff is only one part of bookmarking. It would be kind of a shame for me to give up on bookmark.el just for that. - d
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Wed, 27 May 2020 09:51:02 GMT) Full text and rfc822 format available.Message #55 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Stefan Kangas <stefan <at> marxist.se> To: Karl Fogel <kfogel <at> red-bean.com>, Drew Adams <drew.adams <at> oracle.com> Cc: 39293 <at> debbugs.gnu.org, Matthias Meulien <orontee <at> gmail.com> Subject: Re: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Wed, 27 May 2020 02:50:07 -0700
Karl Fogel <kfogel <at> red-bean.com> writes: > Stefan, is there a strong motivation here, other than the obvious > attractions of re-using code and avoiding multiple implementations of > similar functionality? I don't see any strong motivation here, no. Some features of tabulated-list-mode would be nice to have, such as sorting by column and better mouse support. Best regards, Stefan Kangas
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Fri, 12 Jun 2020 11:56:01 GMT) Full text and rfc822 format available.Message #58 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: "Basil L. Contovounesios" <contovob <at> tcd.ie> To: Stefan Kangas <stefan <at> marxist.se> Cc: Karl Fogel <kfogel <at> red-bean.com>, 39293 <at> debbugs.gnu.org, Matthias Meulien <orontee <at> gmail.com>, Drew Adams <drew.adams <at> oracle.com> Subject: Re: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Fri, 12 Jun 2020 12:55:31 +0100
Stefan Kangas <stefan <at> marxist.se> writes: > Karl Fogel <kfogel <at> red-bean.com> writes: > >> Stefan, is there a strong motivation here, other than the obvious >> attractions of re-using code and avoiding multiple implementations of >> similar functionality? > > I don't see any strong motivation here, no. > > Some features of tabulated-list-mode would be nice to have, such as > sorting by column and better mouse support. FWIW I'm generally in favour of reusing core, general infrastructure wherever applicable, because IMO the benefits outweigh the downsides, but I don't know enough about bookmark-bmenu-mode or tabulated-list-mode to comment on that part of this discussion. What I will comment on is that, unless bookmark.el provides a specific public API (not just functions, but rather any format that's documented and can be relied on externally), then external extensions to it should not rely on its internal implementation. Packages should not be limited by assumptions made by external extensions. Besides, why are these extensions external to bookmark.el to begin with? Surely useful extensions should be included upstream. Thanks, -- Basil
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Fri, 12 Jun 2020 20:58:02 GMT) Full text and rfc822 format available.Message #61 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Drew Adams <drew.adams <at> oracle.com> To: "Basil L. Contovounesios" <contovob <at> tcd.ie>, Stefan Kangas <stefan <at> marxist.se> Cc: Karl Fogel <kfogel <at> red-bean.com>, 39293 <at> debbugs.gnu.org, Matthias Meulien <orontee <at> gmail.com> Subject: RE: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Fri, 12 Jun 2020 11:03:05 -0700 (PDT)
> unless bookmark.el provides a specific > public API (not just functions, but rather any format > that's documented and can be relied on externally), then > external extensions to it should not rely on its internal > implementation. Packages should not be limited by > assumptions made by external extensions. Besides, why > are these extensions external to bookmark.el to begin > with? Surely useful extensions should be included upstream. 1. I wonder how familiar you are with bookmark.el, its code, and the bookmark formats it defines. 2. Basing the bookmark-list display on `tabulated-list-mode' could not, by any stretch of the imagination, be considered "internal implementation". The immediate behavior change for users would be minor, yes. But the repercussions of the change would be large for users, in terms of limiting future behavior enhancements. 3. My opinion is anyway that there's nothing "internal" in GNU Emacs, or in free software in general. You say, "Packages should not be limited by assumptions made by external extensions." You meant that for packages distributed as part of Emacs. But the reverse is also true: 3rd-party packages should not be limited by all of the assumptions made by vanilla Emacs code at any given time. Sure, no one forces anyone else to abide by any sense of cooperation. But there has been some mutual cooperation and respect over the years. 3rd-party code depends, at any given time, on what Emacs provides, not vice versa, of course. But in the long run what Emacs provides depends in part on what goes on outside its parochial development. An attitude that "core" Emacs development shouldn't care to look at what's happening in the wider community is self-limiting. Telling 3rd-party developers that you don't need to listen to them, don't need to care about what they say or ask for is, yes, within your rights. Core Emacs need not care, in any sense of legal obligation. But then don't wonder about separation between the core and the larger community. 4. The format of bookmarks _is_ documented, in bookmark.el comments. Bookmark+ respects that format, and extends it. That format has changed over time, and Bookmark+ has adapted, to handle the old formats as well as the new ones. 5. Anyone is free to extend the "format" of bookmarks. That's entirely expected. New kinds of bookmarks can be defined, and any new fields can be added. What's important is that the basic structure defined by/for bookmark.el be respected, but nothing prevents adding additional fields etc. That's as it should be. There is also nothing wrong with enhancing or otherwise changing the use (behavior) of existing fields, as long as such behavior changes are clearly documented for users and use of the 3rd-party library is optional. 6. I'm very conservative in my enhancements of vanilla behavior. I try as much as possible not to modify existing code, etc. (But some of my code that tries be compatible with older Emacs releases can't use nadvice etc., so there is more actual modification.) I avoid changing things gratuitously for several reasons, not the least of which is the maintenance burden of updating my code as Emacs code changes. You have only to notice that many of my libraries are named ____+.el, where the ____ is the name of a standard Emacs library. Those `+' libraries of mine typically start out as minor add-ons to the existing vanilla code, sometimes as a result of not being able to persuade "core" to add some feature, and sometimes because the library is, so far, only for my own use - just personal customization. Bookmark+ started out that way, for example. From 2004 to 2009 it consisted only of some code I used myself, to remain compatible with Emacs 20. Starting in 2009 were added (1) the ability to bookmark a region and (2) display-list commands to list only W3M, Info, Gnus, files, and region bookmarks. And so on - more features were added. The point is that I always based the library on vanilla bookmark.el. Even as many more features were added, and the bookmark.el code it made use of accounted for little, I kept the dependency. Why? To be sure it continued to work well with vanilla bookmarks, and to oblige it to keep up-to-date with any new features that bookmark.el might add. 7. Everything in Bookmark+ has, from the beginning, been offered to vanilla Emacs for inclusion. Everything and anything it does could be added to GNU Emacs. I've even offered it as a whole, as a drop-in replacement for bookmark.el (after incorporating the bit of code from that file that Bookmark+ makes use of). Stefan Monnier said at one point that such replacement would be good to do. Other than that comment by Stefan, there hasn't been any interest by Emacs Dev in the code or features provided by Bookmark+. So I continue to maintain it "outside" of Emacs. So be it. (I may have forgotten some minor uptake of Bookmark+ features; Karl can correct me.) 8. My arguments against basing Emacs bookmark-list display on `tabulated-list-mode' go beyond nuisance to Bookmark+. If bookmark.el changes to base its bookmark-list display on `t-l-mode' then I'll just have to change Bookmark+ so that it works around that, e.g., by incorporating the former bookmark.el code that's relevant. IOW, I'll need to abandon dependence on bookmark.el. Not the end of the world. My argument against `t-l-mode' for bookmark.el is that almost nothing is gained, and much of what could otherwise be possible is lost. Not that anything in the current bookmark.el display-list would be lost, but that its improvement potential would be reduced - a sacrifice for very little gain (sorting by clicking column heads). `t-l-mode' is rudimentary. It's not built for, or adaptable to use with, "tabular" displays that are more sophisticated than just what it provides/expects. You can't even use `t-l-mode' to control only part of a buffer - it has to own the whole buffer. You can't even add a title or other text to a buffer displaying tabular info, if you give control to `t-l-mode'. I do use `t-l-mode' myself - in my library apu.el, for instance. But even for that simple case I had to jump through a few hoops to work around some simplistic behavior & limitations. Nothing dramatic; just sayin'. `t-l-mode' is what it is. It isn't bad; it's limited - and limiting. Those wanting to convert Emacs buffers that apparently use columns to `t-l-mode' are, IMO, too often suffering from near-sightedness and have-a-hammer-see-only-nails syndrome. They might do well to focus their attention on some of the many other things that need improving in Emacs. Once you impose `t-l-mode' on a buffer you've limited what you can do with it - then and thereafter. And it makes zero sense to impose it on a buffer that already offers behavior not supported by `t-l-mode'. (The prime example of this is Dired mode.) Just because you see some columns, it doesn't follow that `t-l-mode' is called for, or a wise addition. You have to consider what `t-l-mode' locks you into. Could `t-l-mode' be improved, to allow it to play well and flexibly with other columnar-list displays? Maybe. But I'm not counting on it. Too much in its design depends on total control, I believe. Perhaps its effect could be limited to a particular buffer zone, but even then I think it would be imposing limiting behavior on that zone. Anyway, that's a different discussion, and no doubt someone else would need to take that up, not I. 9. Much, perhaps most, of the progress in Emacs over the decades has been outside the "core". Yes, there have also been great developments within the core, including in the last couple of decades. But the widespread use of 3rd-party code speaks to the fact that much that's progressive and creative in Emacs development happens in the larger community, outside the core - for whatever reasons. IMO that's a fact, for better, worse, or both. Rather than lament the fact that a library like Bookmark+ has introduced new features outside the core, it would be better to look at what it has to offer - at least as food for thought, and perhaps for simple adoption/integration.
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Fri, 12 Jun 2020 21:42:02 GMT) Full text and rfc822 format available.Message #64 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: "Basil L. Contovounesios" <contovob <at> tcd.ie> To: Drew Adams <drew.adams <at> oracle.com> Cc: Karl Fogel <kfogel <at> red-bean.com>, 39293 <at> debbugs.gnu.org, Matthias Meulien <orontee <at> gmail.com>, Stefan Kangas <stefan <at> marxist.se> Subject: Re: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Fri, 12 Jun 2020 22:40:56 +0100
Drew Adams <drew.adams <at> oracle.com> writes: >> unless bookmark.el provides a specific >> public API (not just functions, but rather any format >> that's documented and can be relied on externally), then >> external extensions to it should not rely on its internal >> implementation. Packages should not be limited by >> assumptions made by external extensions. Besides, why >> are these extensions external to bookmark.el to begin >> with? Surely useful extensions should be included upstream. > > 1. I wonder how familiar you are with bookmark.el, its code, > and the bookmark formats it defines. Very little, hence my (attempt at) careful phrasing. > 2. Basing the bookmark-list display on `tabulated-list-mode' > could not, by any stretch of the imagination, be > considered "internal implementation". The immediate > behavior change for users would be minor, yes. But the > repercussions of the change would be large for users, > in terms of limiting future behavior enhancements. In what ways exactly would future enhancements be limited? > 3. My opinion is anyway that there's nothing "internal" > in GNU Emacs, or in free software in general. > > You say, "Packages should not be limited by assumptions > made by external extensions." > > You meant that for packages distributed as part of Emacs. No, I meant that for any package. > But the reverse is also true: 3rd-party packages should > not be limited by all of the assumptions made by vanilla > Emacs code at any given time. > > Sure, no one forces anyone else to abide by any sense of > cooperation. But there has been some mutual cooperation > and respect over the years. 3rd-party code depends, at > any given time, on what Emacs provides, not vice versa, > of course. > > But in the long run what Emacs provides depends in part > on what goes on outside its parochial development. An > attitude that "core" Emacs development shouldn't care to > look at what's happening in the wider community is > self-limiting. > > Telling 3rd-party developers that you don't need to > listen to them, don't need to care about what they say > or ask for is, yes, within your rights. Core Emacs need > not care, in any sense of legal obligation. But then > don't wonder about separation between the core and the > larger community. I neither said nor suggested anything like this. > 4. The format of bookmarks _is_ documented, in bookmark.el > comments. There's a difference between comments intended for general readership that document a stable API, and those that explain what code is doing. Which kind are you referring to? > Bookmark+ respects that format, and extends > it. That format has changed over time, and Bookmark+ > has adapted, to handle the old formats as well as the > new ones. Then in theory it could also handle future changes, no? [...] > 7. Everything in Bookmark+ has, from the beginning, been > offered to vanilla Emacs for inclusion. Everything and > anything it does could be added to GNU Emacs. I've even > offered it as a whole, as a drop-in replacement for > bookmark.el (after incorporating the bit of code from > that file that Bookmark+ makes use of). > > Stefan Monnier said at one point that such replacement > would be good to do. Other than that comment by Stefan, > there hasn't been any interest by Emacs Dev in the code > or features provided by Bookmark+. So I continue to > maintain it "outside" of Emacs. So be it. (I may have > forgotten some minor uptake of Bookmark+ features; Karl > can correct me.) Do you have any links to these discussions, or would you be willing to bring them up again? I don't see why it should be too late to discuss these inclusions again, especially if that would mean smoother integration with whatever ways bookmark.el evolves in the future. > 8. My arguments against basing Emacs bookmark-list display > on `tabulated-list-mode' go beyond nuisance to Bookmark+. > If bookmark.el changes to base its bookmark-list display > on `t-l-mode' then I'll just have to change Bookmark+ > so that it works around that, e.g., by incorporating the > former bookmark.el code that's relevant. IOW, I'll need > to abandon dependence on bookmark.el. Not the end of > the world. > > My argument against `t-l-mode' for bookmark.el is that > almost nothing is gained, and much of what could > otherwise be possible is lost. Not that anything in the > current bookmark.el display-list would be lost, but that > its improvement potential would be reduced - a sacrifice > for very little gain (sorting by clicking column heads). That's just one superficial gain. There are other benefits for both developers and users from reusing core infrastructure, such as better integration, uniform UIs and customisations, etc. You could say "improvement potential would be reduced" any time any decision is made. Is there a real current use case under threat? > `t-l-mode' is rudimentary. It's not built for, or > adaptable to use with, "tabular" displays that are more > sophisticated than just what it provides/expects. > > You can't even use `t-l-mode' to control only part of a > buffer - it has to own the whole buffer. You can't even > add a title or other text to a buffer displaying tabular > info, if you give control to `t-l-mode'. > > I do use `t-l-mode' myself - in my library apu.el, for > instance. But even for that simple case I had to jump > through a few hoops to work around some simplistic > behavior & limitations. Nothing dramatic; just sayin'. > `t-l-mode' is what it is. It isn't bad; it's limited - > and limiting. > > Those wanting to convert Emacs buffers that apparently > use columns to `t-l-mode' are, IMO, too often suffering > from near-sightedness and have-a-hammer-see-only-nails > syndrome. They might do well to focus their attention > on some of the many other things that need improving > in Emacs. There is no need to discourage such contributions. Even if the current proposal is not installed, it's worthwhile to make it. > Once you impose `t-l-mode' on a buffer you've limited > what you can do with it - then and thereafter. And it > makes zero sense to impose it on a buffer that already > offers behavior not supported by `t-l-mode'. (The > prime example of this is Dired mode.) Just because you > see some columns, it doesn't follow that `t-l-mode' is > called for, or a wise addition. You have to consider > what `t-l-mode' locks you into. Of course. > Could `t-l-mode' be improved, to allow it to play well > and flexibly with other columnar-list displays? Maybe. > But I'm not counting on it. Too much in its design > depends on total control, I believe. Perhaps its > effect could be limited to a particular buffer zone, > but even then I think it would be imposing limiting > behavior on that zone. Anyway, that's a different > discussion, and no doubt someone else would need to > take that up, not I. > > 9. Much, perhaps most, of the progress in Emacs over the > decades has been outside the "core". Yes, there have > also been great developments within the core, including > in the last couple of decades. But the widespread use > of 3rd-party code speaks to the fact that much that's > progressive and creative in Emacs development happens in > the larger community, outside the core - for whatever > reasons. IMO that's a fact, for better, worse, or both. > > Rather than lament the fact that a library like Bookmark+ > has introduced new features outside the core, it would be > better to look at what it has to offer - at least as food > for thought, and perhaps for simple adoption/integration. Yes, of course, I'm always in favour of importing good features. -- Basil
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Sat, 13 Jun 2020 00:08:02 GMT) Full text and rfc822 format available.Message #67 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Drew Adams <drew.adams <at> oracle.com> To: "Basil L. Contovounesios" <contovob <at> tcd.ie> Cc: Karl Fogel <kfogel <at> red-bean.com>, 39293 <at> debbugs.gnu.org, Matthias Meulien <orontee <at> gmail.com>, Stefan Kangas <stefan <at> marxist.se> Subject: RE: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Fri, 12 Jun 2020 17:05:31 -0700 (PDT)
> > 2. Basing the bookmark-list display on `tabulated-list-mode' > > could not, by any stretch of the imagination, be > > considered "internal implementation". The immediate > > behavior change for users would be minor, yes. But the > > repercussions of the change would be large for users, > > in terms of limiting future behavior enhancements. > > In what ways exactly would future enhancements be limited? Covered in the rest of the msg you quoted. `t-l-mode' takes over a buffer. It sees only dumb columns with, at most, an associated data type (by which it can sort). > > 3. My opinion is anyway that there's nothing "internal" > > in GNU Emacs, or in free software in general. > > > > You say, "Packages should not be limited by assumptions > > made by external extensions." > > > > You meant that for packages distributed as part of Emacs. > > No, I meant that for any package. But in particular, for `emacs -Q' packages, I think - you were generalizing the idea that bookmark.el shouldn't be limited by any assumptions made by Bookmark+. And you specifically spoke of "internal implementation". The meaning I took was that outside code shouldn't depend on "internal implementation". Isn't that what you meant? > > But the reverse is also true: 3rd-party packages should > > not be limited by all of the assumptions made by vanilla > > Emacs code at any given time. > > > > Sure, no one forces anyone else to abide by any sense of > > cooperation. But there has been some mutual cooperation > > and respect over the years. 3rd-party code depends, at > > any given time, on what Emacs provides, not vice versa, > > of course. > > > > But in the long run what Emacs provides depends in part > > on what goes on outside its parochial development. An > > attitude that "core" Emacs development shouldn't care to > > look at what's happening in the wider community is > > self-limiting. > > > > Telling 3rd-party developers that you don't need to > > listen to them, don't need to care about what they say > > or ask for is, yes, within your rights. Core Emacs need > > not care, in any sense of legal obligation. But then > > don't wonder about separation between the core and the > > larger community. > > I neither said nor suggested anything like this. Again, I interpret your "Packages should not be limited by assumptions made by external extensions." to be an argument that statements about impact of changes to bookmark.el on Bookmark+ shouldn't be taken into account. And that Bookmark+ shouldn't depend on the implementation of bookmark.el. To that, I said fine, if that's the way you want it. But in that case the reverse is true too. A spirit of cooperation matters. Or it doesn't. > > 4. The format of bookmarks _is_ documented, in bookmark.el > > comments. > > There's a difference between comments intended for general readership > that document a stable API, and those that explain what code is doing. > Which kind are you referring to? Call the comments in bookmark.el what you will. I didn't write them (though I might have filed a bug or two to offer some improvement for them). But I understand them to let users, as well as developers, of bookmark.el, know what the structure of a bookmark is, as well as what's important about it and what's not. > > Bookmark+ respects that format, and extends > > it. That format has changed over time, and Bookmark+ > > has adapted, to handle the old formats as well as the > > new ones. > > Then in theory it could also handle future changes, no? Future changes to the structure of a bookmark? "In theory", I'd try to accommodate that, yes. Why - did you have something in mind? Are you thinking about changing the bookmark format? That's not really the subject of this enhancement request. (And bonjour les degats, if you do - you may hear from some bookmark users and library authors.) > > 7. Everything in Bookmark+ has, from the beginning, been > > offered to vanilla Emacs for inclusion. Everything and > > anything it does could be added to GNU Emacs. I've even > > offered it as a whole, as a drop-in replacement for > > bookmark.el (after incorporating the bit of code from > > that file that Bookmark+ makes use of). > > > > Stefan Monnier said at one point that such replacement > > would be good to do. Other than that comment by Stefan, > > there hasn't been any interest by Emacs Dev in the code > > or features provided by Bookmark+. So I continue to > > maintain it "outside" of Emacs. So be it. (I may have > > forgotten some minor uptake of Bookmark+ features; Karl > > can correct me.) > > Do you have any links to these discussions, or would you be willing to > bring them up again? No. The code is there. It's well documented. And if there's interest and there are specific questions then I'll try to find the time to answer them. I'm OK with vanilla Emacs including Bookmark+. And I'd remove, e.g., code that provides for compatibility with older releases. And I'm OK with instead continuing as is, regardless of whether bookmark.el switches to `t-l-mode'. (In the latter case, Bookmark+, or at least its display list, will need to be separated from bookmark.el.) But I won't spend a lot of time helping integrate this or that piece of Bookmark+. I'll help someone understand something that Bookmark+ does, of course. > I don't see why it should be too late to discuss > these inclusions again, especially if that would mean smoother > integration with whatever ways bookmark.el evolves in the future. See above. You, or others, are welcome to start. > > 8. My arguments against basing Emacs bookmark-list display > > on `tabulated-list-mode' go beyond nuisance to Bookmark+. > > If bookmark.el changes to base its bookmark-list display > > on `t-l-mode' then I'll just have to change Bookmark+ > > so that it works around that, e.g., by incorporating the > > former bookmark.el code that's relevant. IOW, I'll need > > to abandon dependence on bookmark.el. Not the end of > > the world. > > > > My argument against `t-l-mode' for bookmark.el is that > > almost nothing is gained, and much of what could > > otherwise be possible is lost. Not that anything in the > > current bookmark.el display-list would be lost, but that > > its improvement potential would be reduced - a sacrifice > > for very little gain (sorting by clicking column heads). > > That's just one superficial gain. That's the only gain for _users_. And sure, that includes the fact that if they know about clicking column headers to sort then they'll know how to use that (sole) feature `t-l-mode' provides. > There are other benefits for both > developers and users from reusing core infrastructure, such as better > integration, uniform UIs and customisations, etc. There are no deffaces or defcustoms. OK, there's one hook. No customization, and nothing special for users to gain, other than click-to-sort column headers. See what I said in my first post of this thread, starting with "This kind of proposal is, IMO, a consequence of one or both of the following:" > You could say "improvement potential would be reduced" > any time any decision is made. You can say that. I didn't say that. You seem to want to argue by making generalizations. > Is there a real current use case under threat? I mentioned titles. There are many other display-list features whose implementation would need to be totally revamped (reimplemented using `t-l-mode' "features"). I mentioned sorting in my first message in this thread. `t-l-mode' lets you sort in only one way, by column type. What's the column type for a bookmark name or a target location? Click the bookmark-name column header - what does it sort by? Not much that's useful. Many of the features Dired provides exist in Bookmark+, from omitting (with show/hide) to specialized markings. And there are other display-list features that Dired doesn't have: interactive filtering, help, editing, and highlighting of entries, etc. Can some of those accommodate `t-l-mode' or be reimplemented to do so? Maybe. I probably won't try/bother, sorry. > > `t-l-mode' is rudimentary. It's not built for, or > > adaptable to use with, "tabular" displays that are more > > sophisticated than just what it provides/expects. > > > > You can't even use `t-l-mode' to control only part of a > > buffer - it has to own the whole buffer. You can't even > > add a title or other text to a buffer displaying tabular > > info, if you give control to `t-l-mode'. > > > > I do use `t-l-mode' myself - in my library apu.el, for > > instance. But even for that simple case I had to jump > > through a few hoops to work around some simplistic > > behavior & limitations. Nothing dramatic; just sayin'. > > `t-l-mode' is what it is. It isn't bad; it's limited - > > and limiting. > > > > Those wanting to convert Emacs buffers that apparently > > use columns to `t-l-mode' are, IMO, too often suffering > > from near-sightedness and have-a-hammer-see-only-nails > > syndrome. They might do well to focus their attention > > on some of the many other things that need improving > > in Emacs. > > There is no need to discourage such contributions. Even if > the current proposal is not installed, it's worthwhile to make it. Sure, it was worthwhile to make it. It was worthwhile to disagree with it. And it's worthwhile not to do it. There's nothing wrong with proposing _any_ particular change to Emacs. > > Once you impose `t-l-mode' on a buffer you've limited > > what you can do with it - then and thereafter. And it > > makes zero sense to impose it on a buffer that already > > offers behavior not supported by `t-l-mode'. (The > > prime example of this is Dired mode.) Just because you > > see some columns, it doesn't follow that `t-l-mode' is > > called for, or a wise addition. You have to consider > > what `t-l-mode' locks you into. > > Of course. Glad you agree. > > Could `t-l-mode' be improved, to allow it to play well > > and flexibly with other columnar-list displays? Maybe. > > But I'm not counting on it. Too much in its design > > depends on total control, I believe. Perhaps its > > effect could be limited to a particular buffer zone, > > but even then I think it would be imposing limiting > > behavior on that zone. Anyway, that's a different > > discussion, and no doubt someone else would need to > > take that up, not I. > > > > 9. Much, perhaps most, of the progress in Emacs over the > > decades has been outside the "core". Yes, there have > > also been great developments within the core, including > > in the last couple of decades. But the widespread use > > of 3rd-party code speaks to the fact that much that's > > progressive and creative in Emacs development happens in > > the larger community, outside the core - for whatever > > reasons. IMO that's a fact, for better, worse, or both. > > > > Rather than lament the fact that a library like Bookmark+ > > has introduced new features outside the core, it would be > > better to look at what it has to offer - at least as food > > for thought, and perhaps for simple adoption/integration. > > Yes, of course, I'm always in favour of importing good features. There are tons of good features out there, in libraries by lots of people, and with GPL. A motherlode to mine. But there's little such mining that goes on by Emacs "core" developers. What there is, is some contribution of 3rd-party libraries to GNU ELPA. But "core" pulling in features from 3rd-party code - "importing"? Not so much. When was the last time you saw a "core" developer import some feature from a 3rd-party library? Library authors and maintainers often find it more worthwhile to just keep working on their stuff outside the "core". And "core" Emacs developers often expect 3rd-party authors to do the work of integrating their features into Emacs. The motherlode's still out there, and growing, for those who are "always in favour of importing good features."
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Sat, 13 Jun 2020 12:18:02 GMT) Full text and rfc822 format available.Message #70 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: "Basil L. Contovounesios" <contovob <at> tcd.ie> To: Drew Adams <drew.adams <at> oracle.com> Cc: Karl Fogel <kfogel <at> red-bean.com>, 39293 <at> debbugs.gnu.org, Matthias Meulien <orontee <at> gmail.com>, Stefan Kangas <stefan <at> marxist.se> Subject: Re: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Sat, 13 Jun 2020 13:17:02 +0100
Drew Adams <drew.adams <at> oracle.com> writes: >> > 2. Basing the bookmark-list display on `tabulated-list-mode' >> > could not, by any stretch of the imagination, be >> > considered "internal implementation". The immediate >> > behavior change for users would be minor, yes. But the >> > repercussions of the change would be large for users, >> > in terms of limiting future behavior enhancements. >> >> In what ways exactly would future enhancements be limited? > > Covered in the rest of the msg you quoted. `t-l-mode' > takes over a buffer. It sees only dumb columns with, > at most, an associated data type (by which it can sort). Does bookmark-bmenu-mode provide any other features over tabulated-list-mode? >> > 3. My opinion is anyway that there's nothing "internal" >> > in GNU Emacs, or in free software in general. >> > >> > You say, "Packages should not be limited by assumptions >> > made by external extensions." >> > >> > You meant that for packages distributed as part of Emacs. >> >> No, I meant that for any package. > > But in particular, for `emacs -Q' packages, I think - > you were generalizing the idea that bookmark.el > shouldn't be limited by any assumptions made by > Bookmark+. Yes, but that generalises to any package, not just core ones. > And you specifically spoke of "internal implementation". > The meaning I took was that outside code shouldn't > depend on "internal implementation". Isn't that what > you meant? Yes, which also applies to any package. >> > But the reverse is also true: 3rd-party packages should >> > not be limited by all of the assumptions made by vanilla >> > Emacs code at any given time. >> > >> > Sure, no one forces anyone else to abide by any sense of >> > cooperation. But there has been some mutual cooperation >> > and respect over the years. 3rd-party code depends, at >> > any given time, on what Emacs provides, not vice versa, >> > of course. >> > >> > But in the long run what Emacs provides depends in part >> > on what goes on outside its parochial development. An >> > attitude that "core" Emacs development shouldn't care to >> > look at what's happening in the wider community is >> > self-limiting. >> > >> > Telling 3rd-party developers that you don't need to >> > listen to them, don't need to care about what they say >> > or ask for is, yes, within your rights. Core Emacs need >> > not care, in any sense of legal obligation. But then >> > don't wonder about separation between the core and the >> > larger community. >> >> I neither said nor suggested anything like this. > > Again, I interpret your "Packages should not be limited > by assumptions made by external extensions." to be an > argument that statements about impact of changes to > bookmark.el on Bookmark+ shouldn't be taken into account. I didn't say they shouldn't be taken into account. But if Bookmark+ relies on internal features then it is unfair to let that limit bookmark.el development. > And that Bookmark+ shouldn't depend on the implementation > of bookmark.el. Yes, just like any package shouldn't rely on internal details of another package if it cares about longterm compatibility. > To that, I said fine, if that's the way you want it. > But in that case the reverse is true too. A spirit > of cooperation matters. Or it doesn't. It does. >> > 4. The format of bookmarks _is_ documented, in bookmark.el >> > comments. >> >> There's a difference between comments intended for general readership >> that document a stable API, and those that explain what code is doing. >> Which kind are you referring to? > > Call the comments in bookmark.el what you will. > I didn't write them (though I might have filed a bug > or two to offer some improvement for them). > > But I understand them to let users, as well as > developers, of bookmark.el, know what the structure of > a bookmark is, as well as what's important about it and > what's not. Sure, but not all comments should be taken as gospel. >> > Bookmark+ respects that format, and extends >> > it. That format has changed over time, and Bookmark+ >> > has adapted, to handle the old formats as well as the >> > new ones. >> >> Then in theory it could also handle future changes, no? > > Future changes to the structure of a bookmark? "In > theory", I'd try to accommodate that, yes. > > Why - did you have something in mind? Are you thinking > about changing the bookmark format? That's not really > the subject of this enhancement request. I was referring to the currently proposed patch in particular and future changes in general. I realise now you were referring specifically to the sexp data that bookmark.el uses, which was not under discussion. > (And bonjour les degats, if you do - you may hear from > some bookmark users and library authors.) > >> > 7. Everything in Bookmark+ has, from the beginning, been >> > offered to vanilla Emacs for inclusion. Everything and >> > anything it does could be added to GNU Emacs. I've even >> > offered it as a whole, as a drop-in replacement for >> > bookmark.el (after incorporating the bit of code from >> > that file that Bookmark+ makes use of). >> > >> > Stefan Monnier said at one point that such replacement >> > would be good to do. Other than that comment by Stefan, >> > there hasn't been any interest by Emacs Dev in the code >> > or features provided by Bookmark+. So I continue to >> > maintain it "outside" of Emacs. So be it. (I may have >> > forgotten some minor uptake of Bookmark+ features; Karl >> > can correct me.) >> >> Do you have any links to these discussions, or would you be willing to >> bring them up again? > > No. The code is there. It's well documented. And if > there's interest and there are specific questions then > I'll try to find the time to answer them. > > I'm OK with vanilla Emacs including Bookmark+. And I'd > remove, e.g., code that provides for compatibility with > older releases. > > And I'm OK with instead continuing as is, regardless of > whether bookmark.el switches to `t-l-mode'. (In the > latter case, Bookmark+, or at least its display list, > will need to be separated from bookmark.el.) > > But I won't spend a lot of time helping integrate this > or that piece of Bookmark+. I'll help someone understand > something that Bookmark+ does, of course. Thanks, that could make an interesting project for anyone interested. >> I don't see why it should be too late to discuss >> these inclusions again, especially if that would mean smoother >> integration with whatever ways bookmark.el evolves in the future. > > See above. You, or others, are welcome to start. > >> > 8. My arguments against basing Emacs bookmark-list display >> > on `tabulated-list-mode' go beyond nuisance to Bookmark+. >> > If bookmark.el changes to base its bookmark-list display >> > on `t-l-mode' then I'll just have to change Bookmark+ >> > so that it works around that, e.g., by incorporating the >> > former bookmark.el code that's relevant. IOW, I'll need >> > to abandon dependence on bookmark.el. Not the end of >> > the world. >> > >> > My argument against `t-l-mode' for bookmark.el is that >> > almost nothing is gained, and much of what could >> > otherwise be possible is lost. Not that anything in the >> > current bookmark.el display-list would be lost, but that >> > its improvement potential would be reduced - a sacrifice >> > for very little gain (sorting by clicking column heads). >> >> That's just one superficial gain. > > That's the only gain for _users_. And sure, that includes > the fact that if they know about clicking column headers > to sort then they'll know how to use that (sole) feature > `t-l-mode' provides. That's not the only feature tabulated-list-mode provides. >> There are other benefits for both >> developers and users from reusing core infrastructure, such as better >> integration, uniform UIs and customisations, etc. > > There are no deffaces or defcustoms. OK, there's one hook. > No customization, and nothing special for users to gain, > other than click-to-sort column headers. I count 4 defcustoms, 1 defface, 2 key maps, 1 hook, several functions that can be advised, and a common UI in Emacs 27/28. Furthermore, any future developments to tabulated-list-mode will benefit all modes based on it. All of these are big gains in my book, for both users and developers. > See what I said in my first post of this thread, starting > with "This kind of proposal is, IMO, a consequence of one > or both of the following:" > >> You could say "improvement potential would be reduced" >> any time any decision is made. > > You can say that. I didn't say that. You seem to want > to argue by making generalizations. I made general comments because I'm not familiar enough with the relevant code to make more specific ones, and for whatever they're worth. >> Is there a real current use case under threat? > > I mentioned titles. There are many other display-list > features whose implementation would need to be totally > revamped (reimplemented using `t-l-mode' "features"). I'm asking specifically about bookmark-bmenu-mode, which is what is being discussed. > I mentioned sorting in my first message in this thread. > `t-l-mode' lets you sort in only one way, by column > type. What's the column type for a bookmark name or > a target location? Click the bookmark-name column > header - what does it sort by? Not much that's useful. Every tabulated-list-mode column can be defined with its own custom sorting predicate. > Many of the features Dired provides exist in Bookmark+, > from omitting (with show/hide) to specialized markings. > And there are other display-list features that Dired > doesn't have: interactive filtering, help, editing, and > highlighting of entries, etc. Can some of those > accommodate `t-l-mode' or be reimplemented to do so? > Maybe. I probably won't try/bother, sorry. [...] >> > 9. Much, perhaps most, of the progress in Emacs over the >> > decades has been outside the "core". Yes, there have >> > also been great developments within the core, including >> > in the last couple of decades. But the widespread use >> > of 3rd-party code speaks to the fact that much that's >> > progressive and creative in Emacs development happens in >> > the larger community, outside the core - for whatever >> > reasons. IMO that's a fact, for better, worse, or both. >> > >> > Rather than lament the fact that a library like Bookmark+ >> > has introduced new features outside the core, it would be >> > better to look at what it has to offer - at least as food >> > for thought, and perhaps for simple adoption/integration. >> >> Yes, of course, I'm always in favour of importing good features. > > There are tons of good features out there, in libraries > by lots of people, and with GPL. A motherlode to mine. > > But there's little such mining that goes on by Emacs > "core" developers. What there is, is some contribution > of 3rd-party libraries to GNU ELPA. But "core" pulling > in features from 3rd-party code - "importing"? Not so > much. When was the last time you saw a "core" developer > import some feature from a 3rd-party library? > > Library authors and maintainers often find it more > worthwhile to just keep working on their stuff outside > the "core". And "core" Emacs developers often expect > 3rd-party authors to do the work of integrating their > features into Emacs. The motherlode's still out there, > and growing, for those who are "always in favour of > importing good features." I don't see how this relates to the current discussion. -- Basil
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Tue, 18 Aug 2020 15:25:02 GMT) Full text and rfc822 format available.Message #73 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Lars Ingebrigtsen <larsi <at> gnus.org> To: Stefan Kangas <stefan <at> marxist.se> Cc: 39293 <at> debbugs.gnu.org Subject: Re: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Tue, 18 Aug 2020 17:24:04 +0200
Stefan Kangas <stefan <at> marxist.se> writes: > The attached patch changes bookmark-bmenu-mode to be based on > tabulated-list-mode instead of special-mode. > > This allows us to simplify the code in several cases. In addition, we > get many features for free, such as sorting columns by clicking on the > column headers, changing size of columns. In the future, this will > obviously include any new feature added to 'tabulated-list-mode'. Makes sense to me. There was an objection about packages that extend bookmark.el would no longer work... but I don't think that's an objection we have to heed. As far as I can see, the public interface isn't changed (i.e., the commands are still the same), which is the only this we try to keep compatible. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Tue, 13 Oct 2020 03:42:02 GMT) Full text and rfc822 format available.Message #76 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Lars Ingebrigtsen <larsi <at> gnus.org> To: Stefan Kangas <stefan <at> marxist.se> Cc: 39293 <at> debbugs.gnu.org Subject: Re: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Tue, 13 Oct 2020 05:41:42 +0200
Lars Ingebrigtsen <larsi <at> gnus.org> writes: > Makes sense to me. There was an objection about packages that > extend bookmark.el would no longer work... but I don't think that's an > objection we have to heed. As far as I can see, the public interface > isn't changed (i.e., the commands are still the same), which is the only > this we try to keep compatible. The patch no longer applied, so I tried to respin it for the current emacs, but this leads to three test failures: 3 unexpected results: FAILED bookmark-test-bmenu-delete-all FAILED bookmark-test-bmenu-mark-all FAILED bookmark-test-bmenu-unmark-all Stefan, could you take a look at this, and then we can get it onto the trunk? diff --git a/etc/NEWS b/etc/NEWS index 79a8d119f3..5bdf18cf23 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -96,6 +96,16 @@ groups. Setting it to nil forces the redisplay to do its job even in the initial frame used in batch mode. +--- +** The 'list-bookmark' menu is now based on 'tabulated-list-mode'. +The interactive bookmark list will now benefit from features in +'tabulated-list-mode' like sorting columns or changing column width. + +Support for the optional "inline" header line, allowing for a header +without using 'header-line-format', has been dropped. Consequently, +the variables 'bookmark-bmenu-use-header-line' and +'bookmark-bmenu-inline-header-height' are now declared obsolete. + --- ** Support for the 'strike-through' face attribute on TTY frames. If your terminal's termcap or terminfo database entry has the 'smxx' diff --git a/lisp/bookmark.el b/lisp/bookmark.el index dcb03adadd..7d1cfa0e53 100644 --- a/lisp/bookmark.el +++ b/lisp/bookmark.el @@ -32,6 +32,7 @@ ;;; Code: (require 'pp) +(require 'tabulated-list) (require 'text-property-search) (eval-when-compile (require 'cl-lib)) @@ -126,16 +127,16 @@ bookmark-automatically-show-annotations (defconst bookmark-bmenu-buffer "*Bookmark List*" "Name of buffer used for Bookmark List.") -(defcustom bookmark-bmenu-use-header-line t +(defvar bookmark-bmenu-use-header-line t "Non-nil means to use an immovable header line. -This is as opposed to inline text at the top of the buffer." - :version "24.4" - :type 'boolean) +This is as opposed to inline text at the top of the buffer.") +(make-obsolete-variable 'bookmark-bmenu-use-header-line "no longer used." "28.1") (defconst bookmark-bmenu-inline-header-height 2 "Number of lines used for the *Bookmark List* header. \(This is only significant when `bookmark-bmenu-use-header-line' is nil.)") +(make-obsolete-variable 'bookmark-bmenu-inline-header-height "no longer used." "28.1") (defconst bookmark-bmenu-marks-width 2 "Number of columns (chars) used for the *Bookmark List* marks column. @@ -165,6 +166,7 @@ bookmark-search-delay "Time before `bookmark-bmenu-search' updates the display." :type 'number) +;; FIXME: Should be declared obsolete. (defface bookmark-menu-heading '((t (:inherit font-lock-type-face))) "Face used to highlight the heading in bookmark menu buffers." @@ -976,7 +978,7 @@ bookmark-send-edited-annotation (when from-bookmark-list (pop-to-buffer (get-buffer bookmark-bmenu-buffer)) (goto-char (point-min)) - (text-property-search-forward 'bookmark-name-prop bookmark-name)) + (bookmark-bmenu-bookmark)) (kill-buffer old-buffer))) @@ -1587,7 +1589,7 @@ bookmark-bmenu-hidden-bookmarks (defvar bookmark-bmenu-mode-map (let ((map (make-keymap))) - (set-keymap-parent map special-mode-map) + (set-keymap-parent map tabulated-list-mode-map) (define-key map "v" 'bookmark-bmenu-select) (define-key map "w" 'bookmark-bmenu-locate) (define-key map "5" 'bookmark-bmenu-other-frame) @@ -1607,8 +1609,6 @@ bookmark-bmenu-mode-map (define-key map "d" 'bookmark-bmenu-delete) (define-key map "D" 'bookmark-bmenu-delete-all) (define-key map " " 'next-line) - (define-key map "n" 'next-line) - (define-key map "p" 'previous-line) (define-key map "\177" 'bookmark-bmenu-backup-unmark) (define-key map "u" 'bookmark-bmenu-unmark) (define-key map "U" 'bookmark-bmenu-unmark-all) @@ -1676,6 +1676,30 @@ bookmark-bmenu-surreptitiously-rebuild-list (save-window-excursion (bookmark-bmenu-list))))) +(defun bookmark-bmenu--revert () + "Re-populate `tabulated-list-entries'." + (let (entries) + (dolist (full-record (bookmark-maybe-sort-alist)) + (let* ((name (bookmark-name-from-full-record full-record)) + (annotation (bookmark-get-annotation full-record)) + (location (bookmark-location full-record))) + (push (list + full-record + `[,(if (and annotation (not (string-equal annotation ""))) + "*" "") + ,(if (display-mouse-p) + (propertize name + 'font-lock-face 'bookmark-menu-bookmark + 'mouse-face 'highlight + 'follow-link t + 'help-echo "mouse-2: go to this bookmark in other window") + name) + ,@(if bookmark-bmenu-toggle-filenames + (list location))]) + entries))) + (tabulated-list-init-header) + (setq tabulated-list-entries entries)) + (tabulated-list-print t)) ;;;###autoload (defun bookmark-bmenu-get-buffer () @@ -1702,70 +1726,15 @@ bookmark-bmenu-list (if (called-interactively-p 'interactive) (switch-to-buffer buf) (set-buffer buf))) - (let ((inhibit-read-only t)) - (erase-buffer) - (if (not bookmark-bmenu-use-header-line) - (insert "% Bookmark\n- --------\n")) - (add-text-properties (point-min) (point) - '(font-lock-face bookmark-menu-heading)) - (dolist (full-record (bookmark-maybe-sort-alist)) - (let ((name (bookmark-name-from-full-record full-record)) - (annotation (bookmark-get-annotation full-record)) - (start (point)) - end) - ;; if a bookmark has an annotation, prepend a "*" - ;; in the list of bookmarks. - (insert (if (and annotation (not (string-equal annotation ""))) - " *" " ") - name) - (setq end (point)) - (put-text-property - (+ bookmark-bmenu-marks-width start) end 'bookmark-name-prop name) - (when (display-mouse-p) - (add-text-properties - (+ bookmark-bmenu-marks-width start) end - '(font-lock-face bookmark-menu-bookmark - mouse-face highlight - follow-link t - help-echo "mouse-2: go to this bookmark in other window"))) - (insert "\n"))) - (set-buffer-modified-p (not (= bookmark-alist-modification-count 0))) - (goto-char (point-min)) - (bookmark-bmenu-mode) - (if bookmark-bmenu-use-header-line - (bookmark-bmenu-set-header) - (forward-line bookmark-bmenu-inline-header-height)) - (when (and bookmark-alist bookmark-bmenu-toggle-filenames) - (bookmark-bmenu-toggle-filenames t)))) + (bookmark-bmenu-mode) + (bookmark-bmenu--revert)) ;;;###autoload (defalias 'list-bookmarks 'bookmark-bmenu-list) ;;;###autoload (defalias 'edit-bookmarks 'bookmark-bmenu-list) -;; FIXME: This could also display the current default bookmark file -;; according to `bookmark-bookmarks-timestamp'. -(defun bookmark-bmenu-set-header () - "Set the immutable header line." - (let ((header (copy-sequence "%% Bookmark"))) - (when bookmark-bmenu-toggle-filenames - (setq header (concat header - (make-string (- bookmark-bmenu-file-column - (- (length header) 3)) ?\s) - "File"))) - (let ((pos 0)) - (while (string-match "[ \t\n]+" header pos) - (setq pos (match-end 0)) - (put-text-property (match-beginning 0) pos 'display - (list 'space :align-to (- pos 1)) - header))) - (put-text-property 0 2 'face 'fixed-pitch header) - (setq header (concat (propertize " " 'display '(space :align-to 0)) - header)) - ;; Code derived from `buff-menu.el'. - (setq header-line-format header))) - -(define-derived-mode bookmark-bmenu-mode special-mode "Bookmark Menu" +(define-derived-mode bookmark-bmenu-mode tabulated-list-mode "Bookmark Menu" "Major mode for editing a list of bookmarks. Each line describes one of the bookmarks in Emacs. Letters do not insert themselves; instead, they are commands. @@ -1803,8 +1772,30 @@ bookmark-bmenu-mode \\[bookmark-bmenu-show-all-annotations] -- show the annotations of all bookmarks in another buffer. \\[bookmark-bmenu-edit-annotation] -- edit the annotation for the current bookmark. \\[bookmark-bmenu-search] -- incrementally search for bookmarks." - (setq truncate-lines t) - (setq buffer-read-only t)) + ;; FIXME: The header could also display the current default bookmark file + ;; according to `bookmark-bookmarks-timestamp'. + (setq tabulated-list-format + `[("" 1) ;; Space to add "*" for bookmark with annotation + ("Bookmark" ,bookmark-bmenu-file-column bookmark-bmenu--name-predicate) + ,@(if bookmark-bmenu-toggle-filenames + '(("File" 0 bookmark-bmenu--file-predicate)))]) + (setq tabulated-list-padding bookmark-bmenu-marks-width) + (setq tabulated-list-sort-key '("Bookmark" . nil)) + (add-hook 'tabulated-list-revert-hook #'bookmark-bmenu--revert nil t)' + (setq revert-buffer-function 'bookmark-bmenu--revert) + (tabulated-list-init-header)) + + +(defun bookmark-bmenu--name-predicate (a b) + "Predicate to sort \"*Bookmark List*\" buffer by the name column. +This is used for `tabulated-list-format' in `bookmark-bmenu-mode'." + (string< (caar a) (caar b))) + + +(defun bookmark-bmenu--file-predicate (a b) + "Predicate to sort \"*Bookmark List*\" buffer by the file column. +This is used for `tabulated-list-format' in `bookmark-bmenu-mode'." + (string< (bookmark-location (car a)) (bookmark-location (car b)))) (defun bookmark-bmenu-toggle-filenames (&optional show) @@ -1813,100 +1804,42 @@ bookmark-bmenu-toggle-filenames (interactive) (cond (show - (setq bookmark-bmenu-toggle-filenames nil) - (bookmark-bmenu-show-filenames) (setq bookmark-bmenu-toggle-filenames t)) (bookmark-bmenu-toggle-filenames - (bookmark-bmenu-hide-filenames) (setq bookmark-bmenu-toggle-filenames nil)) (t - (bookmark-bmenu-show-filenames) (setq bookmark-bmenu-toggle-filenames t))) - (when bookmark-bmenu-use-header-line - (bookmark-bmenu-set-header))) + (bookmark-bmenu-surreptitiously-rebuild-list)) -(defun bookmark-bmenu-show-filenames (&optional force) - "In an interactive bookmark list, show filenames along with bookmarks. -Non-nil FORCE forces a redisplay showing the filenames. FORCE is used -mainly for debugging, and should not be necessary in normal use." - (if (and (not force) bookmark-bmenu-toggle-filenames) - nil ;already shown, so do nothing - (with-buffer-modified-unmodified - (save-excursion - (save-window-excursion - (goto-char (point-min)) - (if (not bookmark-bmenu-use-header-line) - (forward-line bookmark-bmenu-inline-header-height)) - (setq bookmark-bmenu-hidden-bookmarks ()) - (let ((inhibit-read-only t)) - (while (< (point) (point-max)) - (let ((bmrk (bookmark-bmenu-bookmark))) - (push bmrk bookmark-bmenu-hidden-bookmarks) - (let ((start (line-end-position))) - (move-to-column bookmark-bmenu-file-column t) - ;; Strip off `mouse-face' from the white spaces region. - (if (display-mouse-p) - (remove-text-properties start (point) - '(mouse-face nil help-echo nil)))) - (delete-region (point) (progn (end-of-line) (point))) - (insert " ") - ;; Pass the NO-HISTORY arg: - (bookmark-insert-location bmrk t) - (forward-line 1))))))))) - - -(defun bookmark-bmenu-hide-filenames (&optional force) - "In an interactive bookmark list, hide the filenames of the bookmarks. -Non-nil FORCE forces a redisplay showing the filenames. FORCE is used -mainly for debugging, and should not be necessary in normal use." - (when (and (not force) bookmark-bmenu-toggle-filenames) - ;; nothing to hide if above is nil - (with-buffer-modified-unmodified - (save-excursion - (goto-char (point-min)) - (if (not bookmark-bmenu-use-header-line) - (forward-line bookmark-bmenu-inline-header-height)) - (setq bookmark-bmenu-hidden-bookmarks - (nreverse bookmark-bmenu-hidden-bookmarks)) - (let ((inhibit-read-only t)) - (while bookmark-bmenu-hidden-bookmarks - (move-to-column bookmark-bmenu-marks-width t) - (bookmark-kill-line) - (let ((name (pop bookmark-bmenu-hidden-bookmarks)) - (start (point))) - (insert name) - (put-text-property start (point) 'bookmark-name-prop name) - (if (display-mouse-p) - (add-text-properties - start (point) - '(font-lock-face bookmark-menu-bookmark - mouse-face highlight - follow-link t help-echo - "mouse-2: go to this bookmark in other window")))) - (forward-line 1))))))) +(defun bookmark-bmenu-show-filenames (&optional _) + "In an interactive bookmark list, show filenames along with bookmarks." + (setq bookmark-bmenu-toggle-filenames t) + (bookmark-bmenu-surreptitiously-rebuild-list)) + + +(defun bookmark-bmenu-hide-filenames (&optional _) + "In an interactive bookmark list, hide the filenames of the bookmarks." + (setq bookmark-bmenu-toggle-filenames nil) + (bookmark-bmenu-surreptitiously-rebuild-list)) (defun bookmark-bmenu-ensure-position () "If point is not on a bookmark line, move it to one. -If before the first bookmark line, move to the first; if after the -last full line, move to the last full line. The return value is undefined." - (cond ((and (not bookmark-bmenu-use-header-line) - (< (count-lines (point-min) (point)) - bookmark-bmenu-inline-header-height)) - (goto-char (point-min)) - (forward-line bookmark-bmenu-inline-header-height)) - ((and (bolp) (eobp)) +If after the last full line, move to the last full line. The +return value is undefined." + (cond ((and (bolp) (eobp)) (beginning-of-line 0)))) (defun bookmark-bmenu-bookmark () "Return the bookmark for this line in an interactive bookmark list buffer." (bookmark-bmenu-ensure-position) - (save-excursion - (beginning-of-line) - (forward-char bookmark-bmenu-marks-width) - (get-text-property (point) 'bookmark-name-prop))) + (let* ((id (tabulated-list-get-id)) + (entry (and id (assoc id tabulated-list-entries)))) + (if entry + (caar entry) + ""))) (defun bookmark-show-annotation (bookmark-name-or-record) @@ -1954,14 +1887,8 @@ bookmark-show-all-annotations (defun bookmark-bmenu-mark () "Mark bookmark on this line to be displayed by \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-select]." (interactive) - (beginning-of-line) (bookmark-bmenu-ensure-position) - (with-buffer-modified-unmodified - (let ((inhibit-read-only t)) - (delete-char 1) - (insert ?>) - (forward-line 1) - (bookmark-bmenu-ensure-position)))) + (tabulated-list-put-tag ">" t)) (defun bookmark-bmenu-mark-all () @@ -2126,17 +2053,12 @@ bookmark-bmenu-unmark "Cancel all requested operations on bookmark on this line and move down. Optional BACKUP means move up." (interactive "P") - (beginning-of-line) + ;; any flags to reset according to circumstances? How about a + ;; flag indicating whether this bookmark is being visited? + ;; well, we don't have this now, so maybe later. (bookmark-bmenu-ensure-position) - (with-buffer-modified-unmodified - (let ((inhibit-read-only t)) - (delete-char 1) - ;; any flags to reset according to circumstances? How about a - ;; flag indicating whether this bookmark is being visited? - ;; well, we don't have this now, so maybe later. - (insert " ")) - (forward-line (if backup -1 1)) - (bookmark-bmenu-ensure-position))) + (tabulated-list-put-tag " ") + (forward-line (if backup -1 1))) (defun bookmark-bmenu-backup-unmark () @@ -2167,14 +2089,8 @@ bookmark-bmenu-delete "Mark bookmark on this line to be deleted. To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]." (interactive) - (beginning-of-line) (bookmark-bmenu-ensure-position) - (with-buffer-modified-unmodified - (let ((inhibit-read-only t)) - (delete-char 1) - (insert ?D) - (forward-line 1) - (bookmark-bmenu-ensure-position)))) + (tabulated-list-put-tag "D" t)) (defun bookmark-bmenu-delete-backwards () @@ -2182,10 +2098,7 @@ bookmark-bmenu-delete-backwards To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]." (interactive) (bookmark-bmenu-delete) - (forward-line -2) - (bookmark-bmenu-ensure-position) - (forward-line 1) - (bookmark-bmenu-ensure-position)) + (forward-line -2)) (defun bookmark-bmenu-delete-all () @@ -2217,8 +2130,6 @@ bookmark-bmenu-execute-deletions (progn (end-of-line) (point)))))) (o-col (current-column))) (goto-char (point-min)) - (unless bookmark-bmenu-use-header-line - (forward-line 1)) (while (re-search-forward "^D" (point-max) t) (bookmark-delete (bookmark-bmenu-bookmark) t)) ; pass BATCH arg (bookmark-bmenu-list) @@ -2343,8 +2254,6 @@ bookmark-menu-popup-paned-menu ;; We MUST autoload EACH form used to set up this variable's value, so ;; that the whole job is done in loaddefs.el. -;; Emacs menubar stuff. - ;;;###autoload (defvar menu-bar-bookmark-map (let ((map (make-sparse-keymap "Bookmark functions"))) diff --git a/test/lisp/bookmark-tests.el b/test/lisp/bookmark-tests.el index c5959e46d8..1d24a9012b 100644 --- a/test/lisp/bookmark-tests.el +++ b/test/lisp/bookmark-tests.el @@ -479,6 +479,8 @@ bookmark-test-bmenu-send-edited-annotation/restore-focus (insert "foo") (bookmark-send-edited-annotation) (should (equal (buffer-name (current-buffer)) bookmark-bmenu-buffer)) + (beginning-of-line) + (forward-char 4) (should (looking-at "name")))) (ert-deftest bookmark-test-bmenu-toggle-filenames () @@ -511,6 +513,7 @@ bookmark-test-bmenu-bookmark (ert-deftest bookmark-test-bmenu-mark () (with-bookmark-bmenu-test (bookmark-bmenu-mark) + (forward-line -1) (beginning-of-line) (should (looking-at "^>")))) @@ -571,6 +574,7 @@ bookmark-test-bmenu-unmark (bookmark-bmenu-mark) (goto-char (point-min)) (bookmark-bmenu-unmark) + (forward-line -1) (beginning-of-line) (should (looking-at "^ ")))) -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Tue, 13 Oct 2020 09:16:01 GMT) Full text and rfc822 format available.Message #79 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Stefan Kangas <stefan <at> marxist.se> To: Lars Ingebrigtsen <larsi <at> gnus.org> Cc: 39293 <at> debbugs.gnu.org Subject: Re: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Tue, 13 Oct 2020 11:14:47 +0200
Lars Ingebrigtsen <larsi <at> gnus.org> writes: > Stefan, could you take a look at this, and then we can get it onto the > trunk? Yup, this is already on my TODO list. But I can't promise I'll get to it soon.
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Tue, 13 Oct 2020 15:37:02 GMT) Full text and rfc822 format available.Message #82 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Drew Adams <drew.adams <at> oracle.com> To: Lars Ingebrigtsen <larsi <at> gnus.org>, Stefan Kangas <stefan <at> marxist.se> Cc: 39293 <at> debbugs.gnu.org Subject: RE: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Tue, 13 Oct 2020 08:36:17 -0700 (PDT)
> > Makes sense to me. There was an objection about packages that > > extend bookmark.el would no longer work... but I don't think that's an > > objection we have to heed. As far as I can see, the public interface > > isn't changed (i.e., the commands are still the same), which is the only > > this we try to keep compatible. > > The patch no longer applied, so I tried to respin it for the current > emacs, but this leads to three test failures: ... > Stefan, could you take a look at this, and then we can get it onto the > trunk? I repeat that this is yet another step backward for Emacs. And a push to separate Bookmark+ from being an extension of bookmark.el.
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Wed, 14 Oct 2020 03:43:02 GMT) Full text and rfc822 format available.Message #85 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Lars Ingebrigtsen <larsi <at> gnus.org> To: Stefan Kangas <stefan <at> marxist.se> Cc: 39293 <at> debbugs.gnu.org Subject: Re: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Wed, 14 Oct 2020 05:42:28 +0200
Stefan Kangas <stefan <at> marxist.se> writes: > Lars Ingebrigtsen <larsi <at> gnus.org> writes: > >> Stefan, could you take a look at this, and then we can get it onto the >> trunk? > > Yup, this is already on my TODO list. But I can't promise I'll get to > it soon. OK, no problem. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no
bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Sat, 17 Oct 2020 15:59:02 GMT) Full text and rfc822 format available.Message #88 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Stefan Kangas <stefan <at> marxist.se> To: Lars Ingebrigtsen <larsi <at> gnus.org> Cc: 39293 <at> debbugs.gnu.org Subject: Re: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Sat, 17 Oct 2020 15:58:43 +0000
close 39293 28.1 thanks Lars Ingebrigtsen <larsi <at> gnus.org> writes: >>> Stefan, could you take a look at this, and then we can get it onto the >>> trunk? >> >> Yup, this is already on my TODO list. But I can't promise I'll get to >> it soon. > > OK, no problem. Now pushed to master as commit 61e51fee9c.
Stefan Kangas <stefan <at> marxist.se>
to control <at> debbugs.gnu.org
.
(Sat, 17 Oct 2020 15:59:02 GMT) Full text and rfc822 format available.bug-gnu-emacs <at> gnu.org
:bug#39293
; Package emacs
.
(Sun, 18 Oct 2020 08:19:01 GMT) Full text and rfc822 format available.Message #93 received at 39293 <at> debbugs.gnu.org (full text, mbox):
From: Lars Ingebrigtsen <larsi <at> gnus.org> To: Stefan Kangas <stefan <at> marxist.se> Cc: 39293 <at> debbugs.gnu.org Subject: Re: bug#39293: [PATCH] Base bookmark-bmenu-mode on 'tabulated-list-mode' Date: Sun, 18 Oct 2020 10:17:49 +0200
Stefan Kangas <stefan <at> marxist.se> writes: > Now pushed to master as commit 61e51fee9c. Thanks; looks good. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no
Debbugs Internal Request <help-debbugs <at> gnu.org>
to internal_control <at> debbugs.gnu.org
.
(Sun, 15 Nov 2020 12:24:08 GMT) Full text and rfc822 format available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.