bug#21201: 25.0.50; (buffer-modified-p) errors inside cl-letf

bug#21201: 25.0.50; (buffer-modified-p) errors inside cl-letf

[Artur Malabarba]

From emacs -Q, run the following:

(cl-letf (((buffer-modified-p)))
  (insert "ookok"))

You'll get an error from trying to set a nil buffer.

Using `(buffer-modified-p (current-buffer))' instead works.

Backtrace:
Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  set-buffer(nil)
  (save-current-buffer (set-buffer nil) (set-buffer-modified-p old))
  (unwind-protect (insert "ookok") (save-current-buffer (set-buffer
nil) (set-buffer-modified-p old)))
  (let* ((old (buffer-modified-p))) (unwind-protect (insert "ookok")
(save-current-buffer (set-buffer nil) (set-buffer-modified-p old))))
  eval((let* ((old (buffer-modified-p))) (unwind-protect (insert
"ookok") (save-current-buffer (set-buffer nil) (set-buffer-modified-p
old)))) nil)
  elisp--eval-last-sexp(nil)
  eval-last-sexp(nil)
  funcall-interactively(eval-last-sexp nil)
  call-interactively(eval-last-sexp nil nil)
  command-execute(eval-last-sexp)

bug#21201: 25.0.50; (buffer-modified-p) errors inside cl-letf

[Nicolas Richard]

Artur Malabarba <bruce.connor.am@gmail.com> writes:

> From emacs -Q, run the following:
>
> (cl-letf (((buffer-modified-p)))
>   (insert "ookok"))
>
> You'll get an error from trying to set a nil buffer.

Here is a fix:
--8<---------------cut here---------------start------------->8---
modified   lisp/emacs-lisp/cl-lib.el
@@ -634,8 +634,10 @@ If ALIST is non-nil, the new pairs are prepended to it."
 ;; Some more Emacs-related place types.
 (gv-define-simple-setter buffer-file-name set-visited-file-name t)
 (gv-define-setter buffer-modified-p (flag &optional buf)
-  `(with-current-buffer ,buf
-     (set-buffer-modified-p ,flag)))
+  (if buf
+      `(with-current-buffer ,buf
+         (set-buffer-modified-p ,flag))
+    `(set-buffer-modified-p ,flag)))
 (gv-define-simple-setter buffer-name rename-buffer t)
 (gv-define-setter buffer-string (store)
   `(insert (prog1 ,store (erase-buffer))))
--8<---------------cut here---------------end--------------->8---

-- 
Nico

bug#21201: 25.0.50; (buffer-modified-p) errors inside cl-letf

[Artur Malabarba]

2015-08-06 21:07 GMT+01:00 Nicolas Richard <youngfrog@members.fsf.org>:
> Artur Malabarba <bruce.connor.am@gmail.com> writes:
>
>> From emacs -Q, run the following:
>>
>> (cl-letf (((buffer-modified-p)))
>>   (insert "ookok"))
>>
>> You'll get an error from trying to set a nil buffer.
>
> Here is a fix:
> --8<---------------cut here---------------start------------->8---
> modified   lisp/emacs-lisp/cl-lib.el
> @@ -634,8 +634,10 @@ If ALIST is non-nil, the new pairs are prepended to it."
>  ;; Some more Emacs-related place types.
>  (gv-define-simple-setter buffer-file-name set-visited-file-name t)
>  (gv-define-setter buffer-modified-p (flag &optional buf)
> -  `(with-current-buffer ,buf
> -     (set-buffer-modified-p ,flag)))
> +  (if buf
> +      `(with-current-buffer ,buf
> +         (set-buffer-modified-p ,flag))
> +    `(set-buffer-modified-p ,flag)))

Don't know if it's possible, but shouldn't it try to ensure that it's
in the same buffer as before?

bug#21201: 25.0.50; (buffer-modified-p) errors inside cl-letf

[Nicolas Richard]

Artur Malabarba <bruce.connor.am@gmail.com> writes:
> 2015-08-06 21:07 GMT+01:00 Nicolas Richard <youngfrog@members.fsf.org>:
>>  (gv-define-setter buffer-modified-p (flag &optional buf)
>> -  `(with-current-buffer ,buf
>> -     (set-buffer-modified-p ,flag)))
>> +  (if buf
>> +      `(with-current-buffer ,buf
>> +         (set-buffer-modified-p ,flag))
>> +    `(set-buffer-modified-p ,flag)))
>
> Don't know if it's possible, but shouldn't it try to ensure that it's
> in the same buffer as before?

I agree with you, but I don't know how to do that.

bug#21201: 25.0.50; (buffer-modified-p) errors inside cl-letf

[Noam Postavsky]

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

tags 21201 + patch
quit

Nicolas Richard <youngfrog@members.fsf.org> writes:

> Artur Malabarba <bruce.connor.am@gmail.com> writes:
>> 2015-08-06 21:07 GMT+01:00 Nicolas Richard <youngfrog@members.fsf.org>:
>>>  (gv-define-setter buffer-modified-p (flag &optional buf)
>>> -  `(with-current-buffer ,buf
>>> -     (set-buffer-modified-p ,flag)))
>>> +  (if buf
>>> +      `(with-current-buffer ,buf
>>> +         (set-buffer-modified-p ,flag))
>>> +    `(set-buffer-modified-p ,flag)))
>>
>> Don't know if it's possible, but shouldn't it try to ensure that it's
>> in the same buffer as before?
>
> I agree with you, but I don't know how to do that.

This seems to work.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 1153 bytes --]

From 1481f8893ff04a83b578f403cd22c7bdd2ec65f0 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Mon, 16 Jul 2018 22:14:32 -0400
Subject: [PATCH] Allow setf of buffer-modified-p without argument (Bug#21201)

* lisp/emacs-lisp/cl-lib.el (setf buffer-modified-p): Take current
buffer if optional argument BUF is not passed.
---
 lisp/emacs-lisp/cl-lib.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/cl-lib.el b/lisp/emacs-lisp/cl-lib.el
index 381784d1fd..ba5f9d64a1 100644
--- a/lisp/emacs-lisp/cl-lib.el
+++ b/lisp/emacs-lisp/cl-lib.el
@@ -521,8 +521,9 @@ cl-pairlis
 ;; Some more Emacs-related place types.
 (gv-define-simple-setter buffer-file-name set-visited-file-name t)
 (gv-define-setter buffer-modified-p (flag &optional buf)
-  `(with-current-buffer ,buf
-     (set-buffer-modified-p ,flag)))
+  (macroexp-let2 nil buffer `(or ,buf (current-buffer))
+    `(with-current-buffer ,buffer
+       (set-buffer-modified-p ,flag))))
 (gv-define-simple-setter buffer-name rename-buffer t)
 (gv-define-setter buffer-string (store)
   `(insert (prog1 ,store (erase-buffer))))
-- 
2.11.0

bug#21201: 25.0.50; (buffer-modified-p) errors inside cl-letf

[Noam Postavsky]

tags 21201 fixed
close 21201 27.1
quit

Noam Postavsky <npostavs@gmail.com> writes:

>  (gv-define-setter buffer-modified-p (flag &optional buf)
> -  `(with-current-buffer ,buf
> -     (set-buffer-modified-p ,flag)))
> +  (macroexp-let2 nil buffer `(or ,buf (current-buffer))
> +    `(with-current-buffer ,buffer
> +       (set-buffer-modified-p ,flag))))

Pushed to master.

[1: c8b86362d4]: 2018-08-27 18:54:23 -0400
  Allow setf of buffer-modified-p without argument (Bug#21201)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=c8b86362d45a07e0aec0041cade551c3c663ea8c