* [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) @ 2024-04-19 5:19 Karl Fogel 2024-04-19 8:01 ` Dani Moncayo 2024-04-19 8:01 ` Dani Moncayo 0 siblings, 2 replies; 15+ messages in thread From: Karl Fogel @ 2024-04-19 5:19 UTC (permalink / raw) To: Emacs Developers; +Cc: 70019, Dani Moncayo [-- Attachment #1: Type: text/plain, Size: 309 bytes --] I'd appreciate some review on this patch. I've no prior experience with fringe mark code (someone else added fringe mark support to bookmark.el). This should fix bug#70019 and a separate-but-sort-of-related bug. I'll wait at least a few days for comments/review before committing. Best regards, -Karl [-- Attachment #2: Fix two bugs in removing bookmark fringe marks (bug#70019) --] [-- Type: text/plain, Size: 2768 bytes --] From a7c9618efdd423134990c22d62513eac50ab98e3 Mon Sep 17 00:00:00 2001 From: Karl Fogel <kfogel@red-bean.com> Date: Thu, 18 Apr 2024 23:49:29 -0500 Subject: [PATCH] Fix two bugs in removing bookmark fringe marks This fixes bug#70019 and a separate fringe-mark removal bug that affected bookmarks in Info "dir" nodes. * lisp/bookmark.el (bookmark--remove-fringe-mark): Fix bug#70019 by temporarily widening in order to ensure we fetch the right overlays. Also, don't tamper with the filename as received from the bookmark object, since we will compare the received filename against one generated dynamically by the same means (`bookmark-buffer-file-name') to determine whether or not we're in the right buffer. --- lisp/bookmark.el | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/lisp/bookmark.el b/lisp/bookmark.el index bf2357207d8..5d01fe24991 100644 --- a/lisp/bookmark.el +++ b/lisp/bookmark.el @@ -515,15 +515,30 @@ bookmark--remove-fringe-mark (non-essential t) overlays found temp) (when (and pos filename) - (setq filename (abbreviate-file-name (expand-file-name filename))) (dolist (buf (buffer-list)) (with-current-buffer buf (when (equal filename (ignore-errors (bookmark-buffer-file-name))) (setq overlays (save-excursion - (goto-char pos) - (overlays-in (pos-bol) (1+ (pos-bol))))) + (save-restriction + ;; Suppose bookmark "foo" was earlier set at + ;; location X in a file, but now the file is + ;; narrowed such that X is outside the restriction. + ;; Then the `goto-char' below would go to the + ;; wrong place and thus the wrong overlays would + ;; be fetched. This is why we temporarily `widen' + ;; before fetching. + ;; + ;; (This circumstance can easily arise when a + ;; bookmark was set on Info node X but now the + ;; "*info*" buffer is showing some other node Y, + ;; with X and Y physically located in the same + ;; file, as is often the case with Info nodes. + ;; See bug #70019, for example.) + (widen) + (goto-char pos) + (overlays-in (pos-bol) (1+ (pos-bol)))))) (while (and (not found) (setq temp (pop overlays))) (when (eq 'bookmark (overlay-get temp 'category)) (delete-overlay (setq found temp)))))))))) -- 2.43.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) 2024-04-19 5:19 [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) Karl Fogel @ 2024-04-19 8:01 ` Dani Moncayo 2024-04-20 0:54 ` Karl Fogel 2024-04-20 0:54 ` Karl Fogel 2024-04-19 8:01 ` Dani Moncayo 1 sibling, 2 replies; 15+ messages in thread From: Dani Moncayo @ 2024-04-19 8:01 UTC (permalink / raw) To: Karl Fogel; +Cc: Emacs Developers, 70019 > This should fix bug#70019 and a separate-but-sort-of-related bug. > > I'll wait at least a few days for comments/review before > committing. I've applied the patch to my source tree and re-made my out-of-tree build. But I still see the same problems reported in this bug ticket. Maybe I've made some mistake... ¿Don't you see the problems with the patch applied? -- Dani Moncayo ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) 2024-04-19 8:01 ` Dani Moncayo @ 2024-04-20 0:54 ` Karl Fogel 2024-04-20 6:50 ` bug#70019: " Dani Moncayo 2024-04-20 6:50 ` Dani Moncayo 2024-04-20 0:54 ` Karl Fogel 1 sibling, 2 replies; 15+ messages in thread From: Karl Fogel @ 2024-04-20 0:54 UTC (permalink / raw) To: Dani Moncayo; +Cc: Emacs Developers, 70019 [-- Attachment #1: Type: text/plain, Size: 420 bytes --] On 19 Apr 2024, Dani Moncayo wrote: >I've applied the patch to my source tree and re-made my >out-of-tree build. >But I still see the same problems reported in this bug ticket. >Maybe I've made some mistake... ¿Don't you see the problems with >the >patch applied? Nope, you didn't make any mistake. I see the problem; please try this revised version, and thank you for testing. Best regards, -Karl [-- Attachment #2: 0001-Fix-two-bugs-in-removing-bookmark-fringe-marks.patch --] [-- Type: text/plain, Size: 3987 bytes --] From 705eeb968c85cf8d865c893778cc0df6a6db6034 Mon Sep 17 00:00:00 2001 From: Karl Fogel <kfogel@red-bean.com> Date: Thu, 18 Apr 2024 23:49:29 -0500 Subject: [PATCH] Fix two bugs in removing bookmark fringe marks This fixes bug#70019 and a separate fringe-mark removal bug that also affected bookmarks in certain Info nodes. * lisp/bookmark.el (bookmark--remove-fringe-mark): Fix bug#70019 by temporarily widening in order to ensure we fetch the right overlays. Also, normalize both filenames before comparing, to avoid spurious failure to match. --- lisp/bookmark.el | 47 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/lisp/bookmark.el b/lisp/bookmark.el index bf2357207d8..06f8e24b518 100644 --- a/lisp/bookmark.el +++ b/lisp/bookmark.el @@ -515,18 +515,45 @@ bookmark--remove-fringe-mark (non-essential t) overlays found temp) (when (and pos filename) - (setq filename (abbreviate-file-name (expand-file-name filename))) (dolist (buf (buffer-list)) (with-current-buffer buf - (when (equal filename - (ignore-errors (bookmark-buffer-file-name))) - (setq overlays - (save-excursion - (goto-char pos) - (overlays-in (pos-bol) (1+ (pos-bol))))) - (while (and (not found) (setq temp (pop overlays))) - (when (eq 'bookmark (overlay-get temp 'category)) - (delete-overlay (setq found temp)))))))))) + (let ((bkmk-fname (ignore-errors (bookmark-buffer-file-name)))) + (when bkmk-fname + ;; Normalize both filenames before comparing, because the + ;; filename we receive from the bookmark wasn't + ;; necessarily generated by `bookmark-buffer-file-name'. + ;; For example, bookmarks set in Info nodes get a filename + ;; based on `Info-current-file', and under certain + ;; circumstances that can be an unexpanded path (e.g., + ;; when the Info page was under your home directory). + (let ((this-fname-normalized (expand-file-name filename)) + (bkmk-fname-normalized (expand-file-name bkmk-fname))) + (when (equal this-fname-normalized bkmk-fname-normalized) + (setq overlays + (save-excursion + (save-restriction + ;; Suppose bookmark "foo" was earlier set at + ;; location X in a file, but now the file is + ;; narrowed such that X is outside the + ;; restriction. Then the `goto-char' below + ;; would go to the wrong place and thus the + ;; wrong overlays would be fetched. This is + ;; why we temporarily `widen' before + ;; fetching. + ;; + ;; (This circumstance can easily arise when + ;; a bookmark was set on Info node X but now + ;; the "*info*" buffer is showing some other + ;; node Y, with X and Y physically located + ;; in the same file, as is often the case + ;; with Info nodes. See bug #70019, for + ;; example.) + (widen) + (goto-char pos) + (overlays-in (pos-bol) (1+ (pos-bol)))))) + (while (and (not found) (setq temp (pop overlays))) + (when (eq 'bookmark (overlay-get temp 'category)) + (delete-overlay (setq found temp))))))))))))) (defun bookmark-maybe-sort-alist () "Return `bookmark-alist' for display. -- 2.43.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#70019: [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) 2024-04-20 0:54 ` Karl Fogel @ 2024-04-20 6:50 ` Dani Moncayo 2024-04-20 6:50 ` Dani Moncayo 1 sibling, 0 replies; 15+ messages in thread From: Dani Moncayo @ 2024-04-20 6:50 UTC (permalink / raw) To: Karl Fogel; +Cc: 70019, Emacs Developers On Sat, Apr 20, 2024 at 2:54 AM Karl Fogel <kfogel@red-bean.com> wrote: > Nope, you didn't make any mistake. I see the problem; please try > this revised version, and thank you for testing. OK. With this second patch, I don't see the first problem [1] anymore, but I keep seeing the second one [2]. -- Dani Moncayo [1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=70019#5 [2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=70019#8 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) 2024-04-20 0:54 ` Karl Fogel 2024-04-20 6:50 ` bug#70019: " Dani Moncayo @ 2024-04-20 6:50 ` Dani Moncayo 2024-04-22 3:51 ` bug#70019: " Karl Fogel 2024-04-22 3:51 ` Karl Fogel 1 sibling, 2 replies; 15+ messages in thread From: Dani Moncayo @ 2024-04-20 6:50 UTC (permalink / raw) To: Karl Fogel; +Cc: Emacs Developers, 70019 On Sat, Apr 20, 2024 at 2:54 AM Karl Fogel <kfogel@red-bean.com> wrote: > Nope, you didn't make any mistake. I see the problem; please try > this revised version, and thank you for testing. OK. With this second patch, I don't see the first problem [1] anymore, but I keep seeing the second one [2]. -- Dani Moncayo [1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=70019#5 [2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=70019#8 ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#70019: [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) 2024-04-20 6:50 ` Dani Moncayo @ 2024-04-22 3:51 ` Karl Fogel 2024-04-22 3:51 ` Karl Fogel 1 sibling, 0 replies; 15+ messages in thread From: Karl Fogel @ 2024-04-22 3:51 UTC (permalink / raw) To: Dani Moncayo; +Cc: 70019, Emacs Developers On 20 Apr 2024, Dani Moncayo wrote: >On Sat, Apr 20, 2024 at 2:54 AM Karl Fogel <kfogel@red-bean.com> >wrote: >> Nope, you didn't make any mistake. I see the problem; please >> try this revised version, and thank you for testing. > >OK. With this second patch, I don't see the first problem [1] >anymore, but I keep seeing the second one [2]. Ah, I'm sorry, I meant to discuss that second problem here. For reference, here's your description of it: > 1. Open some *info* manual and set a named bookmark there. > E.g. [C-h r C-x r m f o o RET]. > 2. Kill the *info* buffer and open it again: [C-x k RET C-h r]. > > When I do that, I don't see the bookmark icon in the fringe > (wrong). > > But I _do_ see the icon if I _jump_ to the bookmark: [C-x r b f > o o RET] This is the expected behavior, I think. Let me explain why I believe that. If you or someone can point out why this reasoning is wrong, I'd appreciate it. In general, there's no mechanism for a bookmark fringe mark to be shown when one goes to a bookmarked location by some means *other* than through a bookmark function. For example, 1. Find a file "SomeFile" in your home directory. 2. Create new bookmark "foo" on the first line. (Now you see a fringe mark.) 3. Kill the buffer entirely. 4. Use [C-x C-f] to find "SomeFile" into a buffer again. (Now you don't see a fringe mark.) 5. Kill the buffer again. 6. Use [C-x r b] (`bookmark-jump') to visit bookmark "foo" (Now you see the fringe mark again.) The reason the fringe mark appears in Step 6 is because the bookmark code had a chance to get involved (i.e., `bookmark--set-fringe-mark' got called somewhere along the way from `bookmark-jump'). In general, if you set a bookmark in a buffer, and you merely *leave* the buffer but don't kill the buffer (e.g., you do `bury-buffer' or something), then that fringe mark will still be there when you come back. The fringe mark will also be placed if you get to the location via `bookmark-jump', and if you set a bookmark there with `bookmark-set'. Also, if you retarget bookmark "foo" to point to a new location, then if there's some buffer currently displaying the old target of "foo", that old buffer's corresponding fringe mark for "foo" will get removed (bug #1 that I just fixed was about a special case of this behavior). Now let's look at how this all works with Info nodes: Often, two Info nodes are actually in the same file. This was the case with the two that you used in your reproduction recipe: the "Distrib" node and the "Intro" node in the Emacs manual -- both are in emacs.info.gz, which is located at /usr/local/share/info/emacs.info.gz for me (but might be somewhere else for you). When two Info nodes X and Y are in the same underlying file, then if you put a bookmark in node X, the fringe mark will stay there even if you go visit node Y and then come back to X (assuming you didn't do anything to kill the "*info*" buffer along the way). This is because the buffer has never been killed and the underlying file that it's visiting has not actually changed. But imagine that your two nodes X and Y were in two different Info files. If you set a bookmark in X and then go visit Y, and then come back to X (but not by using `bookmark-jump'), the fringe mark will be gone from from X. So in your reproduction recipe quoted above, when you kill the "*info*" buffer, you're killing the buffer that's visiting that particular Info file. When you visit that same Info node again via a non-bookmark route, there's no code that magically knows that there is a bookmark located here, and so no fringe mark is displayed. On the other hand, when you use `bookmark-jump' to jump to the bookmark, then of course the bookmark code has a chance to put the fringe mark back. By the way, I am not claiming that this behavior is ideal. It would be wonderful if all of Emacs *did* magically have a way to know when a line associated with some bookmark is being displayed, so we could show a fringe mark there. But I can't think of any reasonable way to implement that. By "reasonable", I mean worth the complexity involved (I'm also not sure how to do it without an unacceptable performance penalty, but I haven't thought hard about it, because I'm pretty sure that if one *were* able to do it without a big performance hit, then the solution would be far more complex than a fringe mark is worth). Best regards, -Karl ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) 2024-04-20 6:50 ` Dani Moncayo 2024-04-22 3:51 ` bug#70019: " Karl Fogel @ 2024-04-22 3:51 ` Karl Fogel 2024-04-22 6:26 ` Dani Moncayo 2024-04-22 6:26 ` Dani Moncayo 1 sibling, 2 replies; 15+ messages in thread From: Karl Fogel @ 2024-04-22 3:51 UTC (permalink / raw) To: Dani Moncayo; +Cc: Emacs Developers, 70019 On 20 Apr 2024, Dani Moncayo wrote: >On Sat, Apr 20, 2024 at 2:54 AM Karl Fogel <kfogel@red-bean.com> >wrote: >> Nope, you didn't make any mistake. I see the problem; please >> try this revised version, and thank you for testing. > >OK. With this second patch, I don't see the first problem [1] >anymore, but I keep seeing the second one [2]. Ah, I'm sorry, I meant to discuss that second problem here. For reference, here's your description of it: > 1. Open some *info* manual and set a named bookmark there. > E.g. [C-h r C-x r m f o o RET]. > 2. Kill the *info* buffer and open it again: [C-x k RET C-h r]. > > When I do that, I don't see the bookmark icon in the fringe > (wrong). > > But I _do_ see the icon if I _jump_ to the bookmark: [C-x r b f > o o RET] This is the expected behavior, I think. Let me explain why I believe that. If you or someone can point out why this reasoning is wrong, I'd appreciate it. In general, there's no mechanism for a bookmark fringe mark to be shown when one goes to a bookmarked location by some means *other* than through a bookmark function. For example, 1. Find a file "SomeFile" in your home directory. 2. Create new bookmark "foo" on the first line. (Now you see a fringe mark.) 3. Kill the buffer entirely. 4. Use [C-x C-f] to find "SomeFile" into a buffer again. (Now you don't see a fringe mark.) 5. Kill the buffer again. 6. Use [C-x r b] (`bookmark-jump') to visit bookmark "foo" (Now you see the fringe mark again.) The reason the fringe mark appears in Step 6 is because the bookmark code had a chance to get involved (i.e., `bookmark--set-fringe-mark' got called somewhere along the way from `bookmark-jump'). In general, if you set a bookmark in a buffer, and you merely *leave* the buffer but don't kill the buffer (e.g., you do `bury-buffer' or something), then that fringe mark will still be there when you come back. The fringe mark will also be placed if you get to the location via `bookmark-jump', and if you set a bookmark there with `bookmark-set'. Also, if you retarget bookmark "foo" to point to a new location, then if there's some buffer currently displaying the old target of "foo", that old buffer's corresponding fringe mark for "foo" will get removed (bug #1 that I just fixed was about a special case of this behavior). Now let's look at how this all works with Info nodes: Often, two Info nodes are actually in the same file. This was the case with the two that you used in your reproduction recipe: the "Distrib" node and the "Intro" node in the Emacs manual -- both are in emacs.info.gz, which is located at /usr/local/share/info/emacs.info.gz for me (but might be somewhere else for you). When two Info nodes X and Y are in the same underlying file, then if you put a bookmark in node X, the fringe mark will stay there even if you go visit node Y and then come back to X (assuming you didn't do anything to kill the "*info*" buffer along the way). This is because the buffer has never been killed and the underlying file that it's visiting has not actually changed. But imagine that your two nodes X and Y were in two different Info files. If you set a bookmark in X and then go visit Y, and then come back to X (but not by using `bookmark-jump'), the fringe mark will be gone from from X. So in your reproduction recipe quoted above, when you kill the "*info*" buffer, you're killing the buffer that's visiting that particular Info file. When you visit that same Info node again via a non-bookmark route, there's no code that magically knows that there is a bookmark located here, and so no fringe mark is displayed. On the other hand, when you use `bookmark-jump' to jump to the bookmark, then of course the bookmark code has a chance to put the fringe mark back. By the way, I am not claiming that this behavior is ideal. It would be wonderful if all of Emacs *did* magically have a way to know when a line associated with some bookmark is being displayed, so we could show a fringe mark there. But I can't think of any reasonable way to implement that. By "reasonable", I mean worth the complexity involved (I'm also not sure how to do it without an unacceptable performance penalty, but I haven't thought hard about it, because I'm pretty sure that if one *were* able to do it without a big performance hit, then the solution would be far more complex than a fringe mark is worth). Best regards, -Karl ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) 2024-04-22 3:51 ` Karl Fogel @ 2024-04-22 6:26 ` Dani Moncayo 2024-04-22 7:40 ` bug#70019: " Eli Zaretskii 2024-04-22 7:40 ` Eli Zaretskii 2024-04-22 6:26 ` Dani Moncayo 1 sibling, 2 replies; 15+ messages in thread From: Dani Moncayo @ 2024-04-22 6:26 UTC (permalink / raw) To: Karl Fogel; +Cc: Emacs Developers, 70019 On Mon, Apr 22, 2024 at 5:51 AM Karl Fogel <kfogel@red-bean.com> wrote: > [...] > > In general, there's no mechanism for a bookmark fringe mark to be > shown when one goes to a bookmarked location by some means *other* > than through a bookmark function. > > [...] Hi Karl. OK, Thanks for your work on this, and for your explanations, which I understand. I intuitively expected the fringe icon to be shown at its current location, whenever that location is visible, regardless of other considerations. We both agree that it would be a nice feature, but not at any price. I'm OK with your fix, then. Thanks. -- Dani Moncayo ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#70019: [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) 2024-04-22 6:26 ` Dani Moncayo @ 2024-04-22 7:40 ` Eli Zaretskii 2024-04-22 7:40 ` Eli Zaretskii 1 sibling, 0 replies; 15+ messages in thread From: Eli Zaretskii @ 2024-04-22 7:40 UTC (permalink / raw) To: Dani Moncayo; +Cc: kfogel, 70019, emacs-devel > Cc: 70019@debbugs.gnu.org, Emacs Developers <emacs-devel@gnu.org> > From: Dani Moncayo <dmoncayo@gmail.com> > Date: Mon, 22 Apr 2024 08:26:41 +0200 > > On Mon, Apr 22, 2024 at 5:51 AM Karl Fogel <kfogel@red-bean.com> wrote: > > [...] > > > > In general, there's no mechanism for a bookmark fringe mark to be > > shown when one goes to a bookmarked location by some means *other* > > than through a bookmark function. > > > > [...] > > Hi Karl. > > OK, Thanks for your work on this, and for your explanations, which I understand. > > I intuitively expected the fringe icon to be shown at its current > location, whenever that location is visible, regardless of other > considerations. > > We both agree that it would be a nice feature, but not at any price. > > I'm OK with your fix, then. Thanks. Thanks. Karl, please install the patch on the master branch, and close the bug when you do. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: bug#70019: [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) 2024-04-22 6:26 ` Dani Moncayo 2024-04-22 7:40 ` bug#70019: " Eli Zaretskii @ 2024-04-22 7:40 ` Eli Zaretskii 2024-04-22 22:21 ` Karl Fogel 2024-04-22 22:21 ` Karl Fogel 1 sibling, 2 replies; 15+ messages in thread From: Eli Zaretskii @ 2024-04-22 7:40 UTC (permalink / raw) To: Dani Moncayo; +Cc: kfogel, 70019, emacs-devel > Cc: 70019@debbugs.gnu.org, Emacs Developers <emacs-devel@gnu.org> > From: Dani Moncayo <dmoncayo@gmail.com> > Date: Mon, 22 Apr 2024 08:26:41 +0200 > > On Mon, Apr 22, 2024 at 5:51 AM Karl Fogel <kfogel@red-bean.com> wrote: > > [...] > > > > In general, there's no mechanism for a bookmark fringe mark to be > > shown when one goes to a bookmarked location by some means *other* > > than through a bookmark function. > > > > [...] > > Hi Karl. > > OK, Thanks for your work on this, and for your explanations, which I understand. > > I intuitively expected the fringe icon to be shown at its current > location, whenever that location is visible, regardless of other > considerations. > > We both agree that it would be a nice feature, but not at any price. > > I'm OK with your fix, then. Thanks. Thanks. Karl, please install the patch on the master branch, and close the bug when you do. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: bug#70019: [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) 2024-04-22 7:40 ` Eli Zaretskii @ 2024-04-22 22:21 ` Karl Fogel 2024-04-22 22:21 ` Karl Fogel 1 sibling, 0 replies; 15+ messages in thread From: Karl Fogel @ 2024-04-22 22:21 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Dani Moncayo, 70019-close, emacs-devel On 22 Apr 2024, Eli Zaretskii wrote: >Thanks. Karl, please install the patch on the master branch, and >close the bug when you do. Installed in commit 63765a74f15e, and this email should close the report. Best regards, -Karl ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#70019: [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) 2024-04-22 7:40 ` Eli Zaretskii 2024-04-22 22:21 ` Karl Fogel @ 2024-04-22 22:21 ` Karl Fogel 1 sibling, 0 replies; 15+ messages in thread From: Karl Fogel @ 2024-04-22 22:21 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 70019-close, emacs-devel, Dani Moncayo On 22 Apr 2024, Eli Zaretskii wrote: >Thanks. Karl, please install the patch on the master branch, and >close the bug when you do. Installed in commit 63765a74f15e, and this email should close the report. Best regards, -Karl ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#70019: [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) 2024-04-22 3:51 ` Karl Fogel 2024-04-22 6:26 ` Dani Moncayo @ 2024-04-22 6:26 ` Dani Moncayo 1 sibling, 0 replies; 15+ messages in thread From: Dani Moncayo @ 2024-04-22 6:26 UTC (permalink / raw) To: Karl Fogel; +Cc: 70019, Emacs Developers On Mon, Apr 22, 2024 at 5:51 AM Karl Fogel <kfogel@red-bean.com> wrote: > [...] > > In general, there's no mechanism for a bookmark fringe mark to be > shown when one goes to a bookmarked location by some means *other* > than through a bookmark function. > > [...] Hi Karl. OK, Thanks for your work on this, and for your explanations, which I understand. I intuitively expected the fringe icon to be shown at its current location, whenever that location is visible, regardless of other considerations. We both agree that it would be a nice feature, but not at any price. I'm OK with your fix, then. Thanks. -- Dani Moncayo ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#70019: [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) 2024-04-19 8:01 ` Dani Moncayo 2024-04-20 0:54 ` Karl Fogel @ 2024-04-20 0:54 ` Karl Fogel 1 sibling, 0 replies; 15+ messages in thread From: Karl Fogel @ 2024-04-20 0:54 UTC (permalink / raw) To: Dani Moncayo; +Cc: 70019, Emacs Developers [-- Attachment #1: Type: text/plain, Size: 420 bytes --] On 19 Apr 2024, Dani Moncayo wrote: >I've applied the patch to my source tree and re-made my >out-of-tree build. >But I still see the same problems reported in this bug ticket. >Maybe I've made some mistake... ¿Don't you see the problems with >the >patch applied? Nope, you didn't make any mistake. I see the problem; please try this revised version, and thank you for testing. Best regards, -Karl [-- Attachment #2: 0001-Fix-two-bugs-in-removing-bookmark-fringe-marks.patch --] [-- Type: text/plain, Size: 3987 bytes --] From 705eeb968c85cf8d865c893778cc0df6a6db6034 Mon Sep 17 00:00:00 2001 From: Karl Fogel <kfogel@red-bean.com> Date: Thu, 18 Apr 2024 23:49:29 -0500 Subject: [PATCH] Fix two bugs in removing bookmark fringe marks This fixes bug#70019 and a separate fringe-mark removal bug that also affected bookmarks in certain Info nodes. * lisp/bookmark.el (bookmark--remove-fringe-mark): Fix bug#70019 by temporarily widening in order to ensure we fetch the right overlays. Also, normalize both filenames before comparing, to avoid spurious failure to match. --- lisp/bookmark.el | 47 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/lisp/bookmark.el b/lisp/bookmark.el index bf2357207d8..06f8e24b518 100644 --- a/lisp/bookmark.el +++ b/lisp/bookmark.el @@ -515,18 +515,45 @@ bookmark--remove-fringe-mark (non-essential t) overlays found temp) (when (and pos filename) - (setq filename (abbreviate-file-name (expand-file-name filename))) (dolist (buf (buffer-list)) (with-current-buffer buf - (when (equal filename - (ignore-errors (bookmark-buffer-file-name))) - (setq overlays - (save-excursion - (goto-char pos) - (overlays-in (pos-bol) (1+ (pos-bol))))) - (while (and (not found) (setq temp (pop overlays))) - (when (eq 'bookmark (overlay-get temp 'category)) - (delete-overlay (setq found temp)))))))))) + (let ((bkmk-fname (ignore-errors (bookmark-buffer-file-name)))) + (when bkmk-fname + ;; Normalize both filenames before comparing, because the + ;; filename we receive from the bookmark wasn't + ;; necessarily generated by `bookmark-buffer-file-name'. + ;; For example, bookmarks set in Info nodes get a filename + ;; based on `Info-current-file', and under certain + ;; circumstances that can be an unexpanded path (e.g., + ;; when the Info page was under your home directory). + (let ((this-fname-normalized (expand-file-name filename)) + (bkmk-fname-normalized (expand-file-name bkmk-fname))) + (when (equal this-fname-normalized bkmk-fname-normalized) + (setq overlays + (save-excursion + (save-restriction + ;; Suppose bookmark "foo" was earlier set at + ;; location X in a file, but now the file is + ;; narrowed such that X is outside the + ;; restriction. Then the `goto-char' below + ;; would go to the wrong place and thus the + ;; wrong overlays would be fetched. This is + ;; why we temporarily `widen' before + ;; fetching. + ;; + ;; (This circumstance can easily arise when + ;; a bookmark was set on Info node X but now + ;; the "*info*" buffer is showing some other + ;; node Y, with X and Y physically located + ;; in the same file, as is often the case + ;; with Info nodes. See bug #70019, for + ;; example.) + (widen) + (goto-char pos) + (overlays-in (pos-bol) (1+ (pos-bol)))))) + (while (and (not found) (setq temp (pop overlays))) + (when (eq 'bookmark (overlay-get temp 'category)) + (delete-overlay (setq found temp))))))))))))) (defun bookmark-maybe-sort-alist () "Return `bookmark-alist' for display. -- 2.43.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#70019: [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) 2024-04-19 5:19 [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) Karl Fogel 2024-04-19 8:01 ` Dani Moncayo @ 2024-04-19 8:01 ` Dani Moncayo 1 sibling, 0 replies; 15+ messages in thread From: Dani Moncayo @ 2024-04-19 8:01 UTC (permalink / raw) To: Karl Fogel; +Cc: 70019, Emacs Developers > This should fix bug#70019 and a separate-but-sort-of-related bug. > > I'll wait at least a few days for comments/review before > committing. I've applied the patch to my source tree and re-made my out-of-tree build. But I still see the same problems reported in this bug ticket. Maybe I've made some mistake... ¿Don't you see the problems with the patch applied? -- Dani Moncayo ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-04-22 22:21 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-04-19 5:19 [PATCH] Fix two bugs in removing bookmark fringe marks (bug#70019) Karl Fogel 2024-04-19 8:01 ` Dani Moncayo 2024-04-20 0:54 ` Karl Fogel 2024-04-20 6:50 ` bug#70019: " Dani Moncayo 2024-04-20 6:50 ` Dani Moncayo 2024-04-22 3:51 ` bug#70019: " Karl Fogel 2024-04-22 3:51 ` Karl Fogel 2024-04-22 6:26 ` Dani Moncayo 2024-04-22 7:40 ` bug#70019: " Eli Zaretskii 2024-04-22 7:40 ` Eli Zaretskii 2024-04-22 22:21 ` Karl Fogel 2024-04-22 22:21 ` Karl Fogel 2024-04-22 6:26 ` Dani Moncayo 2024-04-20 0:54 ` Karl Fogel 2024-04-19 8:01 ` Dani Moncayo
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.