bug#49341: 28.0.50; bookmark--unfontify with nil filename throws wrong-type error

bug#49341: 28.0.50; bookmark--unfontify with nil filename throws wrong-type error

[Trey Peacock]

I am unable to use `bookmark-delete` with bookmarks that do not return
alist values for 'filename or 'position when called with
`bookmark-get-bookmark`. I believe it is due to the May 4th commit
ab6cb65cb2b6d11a7b690dfcea8d98611290fad9 where bookmark--unfontify was
introduced. The unfontify function is unable to handle nil values for
these fields and results in calling `(overlay-at pos)` which shows a
wrong-type error.



--
Trey Peacock
treypeacock.com

bug#49341: 28.0.50; bookmark--unfontify with nil filename throws wrong-type error

[Eli Zaretskii]

> Date: Fri, 02 Jul 2021 21:08:06 +0000
> From: Trey Peacock <gpg@treypeacock.com>
> 
> I am unable to use `bookmark-delete` with bookmarks that do not return
> alist values for 'filename or 'position when called with
> `bookmark-get-bookmark`.

Can you show a recipe for reproducing the problem, please?

> I believe it is due to the May 4th commit
> ab6cb65cb2b6d11a7b690dfcea8d98611290fad9 where bookmark--unfontify
> was introduced. The unfontify function is unable to handle nil
> values for these fields and results in calling `(overlay-at pos)`
> which shows a wrong-type error.

Boruch, could you please take a look?

Thanks.

bug#49341: 28.0.50; bookmark--unfontify with nil filename throws wrong-type error

[Trey Peacock]

"Eli Zaretskii" <eliz@gnu.org> writes:

> Can you show a recipe for reproducing the problem, please?

emacs -Q -l temp.el

temp.el

```
(require 'package)
(add-to-list 'package-archives
             '("melpa" . "https://melpa.org/packages/") t)

(package-initialize)
(package-install 'use-package)
(package-install 'quelpa-use-package)
(quelpa
 '(quelpa-use-package
   :fetcher git
   :url "https://github.com/quelpa/quelpa-use-package.git"))
(require 'quelpa-use-package)
(use-package burly
  :quelpa (burly :fetcher github :repo "alphapapa/burly.el"))

(split-window-horizontally)
(switch-to-buffer "*Messages*")
(burly-bookmark-windows "temp")
(bookmark-delete "temp-book")
```

This should throw the wrong type error.

bug#49341: 28.0.50; bookmark--unfontify with nil filename throws wrong-type error

[Lars Ingebrigtsen]

Trey Peacock <gpg@treypeacock.com> writes:

> This should throw the wrong type error.

Can you post the backtrace you get, too?  (With `debug-on-error' set.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49341: 28.0.50; bookmark--unfontify with nil filename throws wrong-type error

[Trey Peacock]

"Lars Ingebrigtsen" <larsi@gnus.org> writes:

> Can you post the backtrace you get, too?  (With `debug-on-error' set.)

```
Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil)
  bookmark--unfontify(("temp-book" (url
  . "emacs+burly+windows:?%28%28%28min-height%20.%204%2...") (handler
  . burly-bookmark-handler)))
  bookmark-delete("temp-book")
  load-with-code-conversion("/home/morpheus/.config/emacs/test-init.el" "/home/morpheus/.config/emacs/test-init.el" nil t)
  command-line-1(("-l" "/home/morpheus/.config/emacs/test-init.el"))
  command-line()
  normal-top-level()
```

```
(defun bookmark--unfontify (bm)
  "Remove a bookmark's colorized overlay.
BM is a bookmark as returned from function `bookmark-get-bookmark'.
See user option `bookmark-fontify'."
  (let ((filename (assq 'filename bm))
        (pos      (assq 'position bm))
        overlays found temp)
    (when filename (setq filename (expand-file-name (cdr filename))))
    (when pos (setq pos (cdr pos)))
    (dolist (buf (buffer-list))
      (with-current-buffer buf
        (when (equal filename buffer-file-name)
          (setq overlays (overlays-at pos))
          (while (and (not found) (setq temp (pop overlays)))
            (when (eq 'bookmark (overlay-get temp 'category))
              (delete-overlay (setq found temp)))))))))
```

The bookmark has no 'filename' in its alist, so the value is nil. When
compared to any non file-backed buffer in the buffer-list that will
cause `(setq overlays (overlay-at pos))`. This results in the error with
`(overlays-at nil)`.

bug#49341: 28.0.50; bookmark--unfontify with nil filename throws wrong-type error

[Lars Ingebrigtsen]

Trey Peacock <gpg@treypeacock.com> writes:

> "Lars Ingebrigtsen" <larsi@gnus.org> writes:
>
>> Can you post the backtrace you get, too?  (With `debug-on-error' set.)
>
> ```
> Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil)
>   bookmark--unfontify(("temp-book" (url
>   . "emacs+burly+windows:?%28%28%28min-height%20.%204%2...") (handler
>   . burly-bookmark-handler)))
>   bookmark-delete("temp-book")

Thanks; I've now pushed a change that should make `bookmark--unfontify'
more robust.  Can you check whether this fixes this problem?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49341: 28.0.50; bookmark--unfontify with nil filename throws wrong-type error

[Trey Peacock]

"Lars Ingebrigtsen" <larsi@gnus.org> writes:

> Thanks; I've now pushed a change that should make `bookmark--unfontify'
> more robust.  Can you check whether this fixes this problem?

Yes, this change has fixed the issue I was seeing. Thanks!

bug#49341: 28.0.50; bookmark--unfontify with nil filename throws wrong-type error

[Lars Ingebrigtsen]

Trey Peacock <gpg@treypeacock.com> writes:

> "Lars Ingebrigtsen" <larsi@gnus.org> writes:
>
>> Thanks; I've now pushed a change that should make `bookmark--unfontify'
>> more robust.  Can you check whether this fixes this problem?
>
> Yes, this change has fixed the issue I was seeing. Thanks!

Thanks for checking; I'm closing this bug report, then.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49341: 28.0.50; bookmark--unfontify with nil filename throws wrong-type error

[Boruch Baum]

On 2021-07-03 09:08, Eli Zaretskii wrote:
> Boruch, could you please take a look?

Basil has owned this, since[1].

See my specific warning to him about this very condition[2].

Both 2021-05-05.

[1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=48179#66
[2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=48179#72

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#49341: 28.0.50; bookmark--unfontify with nil filename throws wrong-type error

[Basil L. Contovounesios]

Boruch Baum <boruch_baum@gmx.com> writes:

> On 2021-07-03 09:08, Eli Zaretskii wrote:
>> Boruch, could you please take a look?
>
> Basil has owned this, since[1].

No he has not, he is confused about the origin of this bizarre idea, and
he has repeatedly explained as much in the very same discussion:

https://bugs.gnu.org/48179#129
https://bugs.gnu.org/48179#135
https://bugs.gnu.org/48179#141

The entire extent of Basil's practical involvement in this feature has
been and wishes to remain a handful of minor documentation and style
fixes:

; Fix and simplify last change in bookmark.el.
7d0067f297 2021-05-04 10:54:24 +0100
https://git.sv.gnu.org/cgit/emacs.git/commit/?id=7d0067f297b131c98a5198eb52d49891a83ac5aa

> See my specific warning to him about this very condition[2].
>
> Both 2021-05-05.
>
> [1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=48179#66
> [2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=48179#72

Thanks,

-- 
Basil