* [PATCH] New with-file-buffer macro
@ 2020-12-31 19:13 Adam Porter
2020-12-31 19:48 ` Eli Zaretskii
2021-01-01 14:32 ` Arthur Miller
0 siblings, 2 replies; 3+ messages in thread
From: Adam Porter @ 2020-12-31 19:13 UTC (permalink / raw)
To: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 4255 bytes --]
Hi,
Attached is a patch which adds a new macro, with-file-buffer. It's
similar to the macro with-temp-file, and it simplifies some common
cases, like reading a file's contents into a buffer and returning a
value from forms evaluated in its buffer, writing to a file that
shouldn't already exist, overwriting an existing file, etc.
For example, here's a recently updated function in package-build.el
which uses with-temp-buffer and write-region:
#+BEGIN_SRC elisp
(defun package-build--write-pkg-readme (name files directory)
(when-let ((commentary
(let* ((file (concat name ".el"))
(file (or (car (rassoc file files)) file))
(file (and file (expand-file-name file directory))))
(and (file-exists-p file)
(lm-commentary file)))))
(with-temp-buffer
(if (>= emacs-major-version 27)
(insert commentary)
;; Taken from 27.1's `lm-commentary'.
(insert
(replace-regexp-in-string ; Get rid of...
"[[:blank:]]*$" "" ; trailing white-space
(replace-regexp-in-string
(format "%s\\|%s\\|%s"
;; commentary header
(concat "^;;;[[:blank:]]*\\("
lm-commentary-header
"\\):[[:blank:]\n]*")
"^;;[[:blank:]]*" ; double semicolon prefix
"[[:blank:]\n]*\\'") ; trailing new-lines
"" commentary))))
(unless (= (char-before) ?\n)
(insert ?\n))
(let ((coding-system-for-write buffer-file-coding-system))
(write-region nil nil
(expand-file-name (concat name "-readme.txt")
package-build-archive-dir))))))
#+END_SRC
Here's how it could work using this new with-file-buffer macro:
#+BEGIN_SRC elisp
(defun package-build--write-pkg-readme (name files directory)
(when-let ((commentary
(let* ((file (concat name ".el"))
(file (or (car (rassoc file files)) file))
(file (and file (expand-file-name file directory))))
(and (file-exists-p file)
(lm-commentary file)))))
(let ((coding-system-for-write buffer-file-coding-system))
(with-file-buffer (expand-file-name (concat name "-readme.txt")
package-build-archive-dir)
(:insert nil :write t :overwrite t)
(if (>= emacs-major-version 27)
(insert commentary)
;; Taken from 27.1's `lm-commentary'.
(insert
(replace-regexp-in-string ; Get rid of...
"[[:blank:]]*$" "" ; trailing white-space
(replace-regexp-in-string
(format "%s\\|%s\\|%s"
;; commentary header
(concat "^;;;[[:blank:]]*\\("
lm-commentary-header
"\\):[[:blank:]\n]*")
"^;;[[:blank:]]*" ; double semicolon prefix
"[[:blank:]\n]*\\'") ; trailing new-lines
"" commentary))))
(unless (= (char-before) ?\n)
(insert ?\n))))))
#+END_SRC
This example isn't the most dramatically different, but it shows a few
improvements: the options list after the filename shows clearly what the
form does (write a file, overwriting if it already exists), whereas
calling write-region directly requires reading to the end of the body
forms (which might not fit within the window); and the options list also
makes arguments to write-region easier to use, as well as making it easy
to control write-region-inhibit-fsync and file-precious-flag. Without
this macro, these operations and options are, IMO, harder to use and
easy to overlook, so I think this would be a worthwhile addition.
If this idea seems useful, I'm sure various improvements may be made, so
I would appreciate any feedback. For example, it might be better for
":insert nil" to be the default, so ":insert t" would be specified to
read the file's contents into the buffer.
Thanks,
Adam
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch adding with-file-buffer macro --]
[-- Type: text/x-diff, Size: 5857 bytes --]
From 37df936cc1c9e3f630a060c8347a4aeba46fc7d0 Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Thu, 31 Dec 2020 12:56:25 -0600
Subject: [PATCH] * lisp/subr.el (with-file-buffer): New macro.
* lisp/subr.el (with-file-buffer): New macro.
* doc/lispref/files.texi (Writing to Files): Document it.
* etc/NEWS: Add news entry.
---
doc/lispref/files.texi | 48 ++++++++++++++++++++++++++++++++++++++++++
etc/NEWS | 6 ++++++
lisp/subr.el | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 111 insertions(+)
diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index 6949ca2..1617bbf 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -708,6 +708,54 @@ Writing to Files
(@pxref{Buffer List}).
@end defmac
+@defmac with-file-buffer file options body@dots{}
+@anchor{Definition of with-file-buffer}
+Like the @code{with-temp-file} macro (which see), the
+@code{with-file-buffer} macro evaluates the @var{body} forms with a
+temporary buffer as the current buffer and then returns the value of
+the last form in @var{body}. The @var{options} offer control over
+various aspects of the operations:
+
+@table @code
+@item :insert
+When non-nil (the default, when unspecified), insert @var{file}'s contents
+before evaluating @var{body}, leaving point before the contents.
+
+@item :must-exist
+When non-nil, signal an error if @var{file} does not exist.
+
+@item :write
+When non-nil, write the contents of the buffer to @var{file} after
+evaluating @var{body}.
+
+@item :overwrite
+When nil (the default, when unspecified), signal an error instead of
+overwriting an existing file. If @code{ask}, ask for confirmation
+before overwriting an existing file. If @code{t}, overwrite
+@var{file} unconditionally.
+
+@item :visit
+Passed to function @code{write-region}, which see.
+
+@item :lockname
+Passed to function @code{write-region}, which see.
+
+@item :append
+Passed to function @code{write-region}, which see. (When using this
+option, you will probably want to specify @code{:insert nil} as well,
+otherwise the file's contents would be duplicated.)
+
+@item :fsync
+When non-nil (the default, when unspecified), bind
+@var{write-region-inhibit-fsync} (which see) to this value.
+
+@item :precious
+Bind @var{file-precious-flag} (which see) to this
+value (when unspecified, nil).
+@end table
+
+@end defmac
+
@node File Locks
@section File Locks
@cindex file locks
diff --git a/etc/NEWS b/etc/NEWS
index 62907a6..9b8288f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2311,6 +2311,12 @@ locales. They are also available as aliases 'ebcdic-cp-*' (e.g.,
'cp278' for 'ibm278'). There are also new charsets 'ibm2xx' to
support these coding-systems.
+---
+** New macro 'with-file-buffer'.
+The new macro 'with-file-buffer' simplifies operating on the contents
+of files and/or writing to them, compared to using 'with-temp-buffer',
+'insert-file-contents', 'write-region', and various options
+separately.
\f
* Changes in Emacs 28.1 on Non-Free Operating Systems
diff --git a/lisp/subr.el b/lisp/subr.el
index 77b142c..0f6da02 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3992,6 +3992,63 @@ with-temp-buffer
(and (buffer-name ,temp-buffer)
(kill-buffer ,temp-buffer)))))))
+(defmacro with-file-buffer (file options &rest body)
+ "Evaluate BODY and return its value in a temp buffer for FILE.
+OPTIONS is a plist of the following options:
+
+`:insert': When non-nil (the default, when unspecified), insert
+file's contents before evaluating BODY, leaving point before the
+contents.
+
+`:must-exist': When non-nil, signal an error if no file exists at
+FILE.
+
+`:write': When non-nil, write the contents of the buffer to FILE
+after evaluating BODY.
+
+`:overwrite': When nil (the default, when unspecified), signal an
+error instead of overwriting an existing file at FILE. If `ask',
+ask for confirmation before overwriting an existing file. If t,
+overwrite a file at FILE unconditionally.
+
+`:visit': Passed to function `write-region', which see.
+
+`:lockname:' Passed to function `write-region', which see.
+
+`:append': Passed to function `write-region', which see. (When
+using this option, you will probably want to specify `:insert
+nil' as well.)
+
+`:fsync': When non-nil (the default, when unspecified), bind
+`write-region-inhibit-fsync' (which see) to this value.
+
+`:precious': Bind `file-precious-flag' (which see) to this
+value (when unspecified, nil)."
+ (declare (indent 2) (debug (stringp form body)))
+ `(let ((write-region-inhibit-fsync ,(when (plist-member options :fsync)
+ (not (plist-get options :fsync))))
+ (file-precious-flag ,(plist-get options :precious)))
+ (with-temp-buffer
+ ,(when (or (not (plist-member options :insert))
+ (plist-get options :insert))
+ `(if (file-readable-p ,file)
+ (save-excursion
+ (insert-file-contents ,file))
+ (when ,(plist-get options :must-exist)
+ (error "File not readable: %s" ,file))))
+ (prog1
+ (progn
+ ,@body)
+ ,(when (plist-get options :write)
+ `(write-region nil nil ,file
+ ,(plist-get options :append)
+ ,(plist-get options :visit)
+ ,(plist-get options :lockname)
+ ,(pcase-exhaustive (plist-get options :overwrite)
+ ('nil ''excl)
+ ((or 'ask ''ask) ''ask)
+ ('t nil))))))))
+
(defmacro with-silent-modifications (&rest body)
"Execute BODY, pretending it does not modify the buffer.
This macro is typically used around modifications of
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] New with-file-buffer macro
2020-12-31 19:13 [PATCH] New with-file-buffer macro Adam Porter
@ 2020-12-31 19:48 ` Eli Zaretskii
2021-01-01 14:32 ` Arthur Miller
1 sibling, 0 replies; 3+ messages in thread
From: Eli Zaretskii @ 2020-12-31 19:48 UTC (permalink / raw)
To: Adam Porter; +Cc: emacs-devel
> From: Adam Porter <adam@alphapapa.net>
> Date: Thu, 31 Dec 2020 13:13:57 -0600
>
> (let ((coding-system-for-write buffer-file-coding-system))
> (with-file-buffer (expand-file-name (concat name "-readme.txt")
> package-build-archive-dir)
> (:insert nil :write t :overwrite t)
The binding for coding-system-for-write will pick up the value from
the wrong buffer, won't it?
In general, it looks to me like this macro is not very useful: it
basically saves you two function calls. Is it really such a big deal?
Most of the churn with the paradigm of reading a file and doing
something with it is not in these two function calls, it is elsewhere,
and what your macro does is just leave all of those tedious details to
the user, just using a different syntax (plist instead of setting or
binding variables).
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] New with-file-buffer macro
2020-12-31 19:13 [PATCH] New with-file-buffer macro Adam Porter
2020-12-31 19:48 ` Eli Zaretskii
@ 2021-01-01 14:32 ` Arthur Miller
1 sibling, 0 replies; 3+ messages in thread
From: Arthur Miller @ 2021-01-01 14:32 UTC (permalink / raw)
To: Adam Porter; +Cc: emacs-devel
Adam Porter <adam@alphapapa.net> writes:
> Hi,
>
> Attached is a patch which adds a new macro, with-file-buffer. It's
> similar to the macro with-temp-file, and it simplifies some common
> cases, like reading a file's contents into a buffer and returning a
> value from forms evaluated in its buffer, writing to a file that
> shouldn't already exist, overwriting an existing file, etc.
>
> For example, here's a recently updated function in package-build.el
> which uses with-temp-buffer and write-region:
>
> #+BEGIN_SRC elisp
> (defun package-build--write-pkg-readme (name files directory)
> (when-let ((commentary
> (let* ((file (concat name ".el"))
> (file (or (car (rassoc file files)) file))
> (file (and file (expand-file-name file directory))))
> (and (file-exists-p file)
> (lm-commentary file)))))
> (with-temp-buffer
> (if (>= emacs-major-version 27)
> (insert commentary)
> ;; Taken from 27.1's `lm-commentary'.
> (insert
> (replace-regexp-in-string ; Get rid of...
> "[[:blank:]]*$" "" ; trailing white-space
> (replace-regexp-in-string
> (format "%s\\|%s\\|%s"
> ;; commentary header
> (concat "^;;;[[:blank:]]*\\("
> lm-commentary-header
> "\\):[[:blank:]\n]*")
> "^;;[[:blank:]]*" ; double semicolon prefix
> "[[:blank:]\n]*\\'") ; trailing new-lines
> "" commentary))))
> (unless (= (char-before) ?\n)
> (insert ?\n))
> (let ((coding-system-for-write buffer-file-coding-system))
> (write-region nil nil
> (expand-file-name (concat name "-readme.txt")
> package-build-archive-dir))))))
> #+END_SRC
>
>
> Here's how it could work using this new with-file-buffer macro:
>
> #+BEGIN_SRC elisp
> (defun package-build--write-pkg-readme (name files directory)
> (when-let ((commentary
> (let* ((file (concat name ".el"))
> (file (or (car (rassoc file files)) file))
> (file (and file (expand-file-name file directory))))
> (and (file-exists-p file)
> (lm-commentary file)))))
> (let ((coding-system-for-write buffer-file-coding-system))
> (with-file-buffer (expand-file-name (concat name "-readme.txt")
> package-build-archive-dir)
> (:insert nil :write t :overwrite t)
> (if (>= emacs-major-version 27)
> (insert commentary)
> ;; Taken from 27.1's `lm-commentary'.
> (insert
> (replace-regexp-in-string ; Get rid of...
> "[[:blank:]]*$" "" ; trailing white-space
> (replace-regexp-in-string
> (format "%s\\|%s\\|%s"
> ;; commentary header
> (concat "^;;;[[:blank:]]*\\("
> lm-commentary-header
> "\\):[[:blank:]\n]*")
> "^;;[[:blank:]]*" ; double semicolon prefix
> "[[:blank:]\n]*\\'") ; trailing new-lines
> "" commentary))))
> (unless (= (char-before) ?\n)
> (insert ?\n))))))
> #+END_SRC
>
> This example isn't the most dramatically different, but it shows a few
> improvements: the options list after the filename shows clearly what the
> form does (write a file, overwriting if it already exists), whereas
> calling write-region directly requires reading to the end of the body
> forms (which might not fit within the window); and the options list also
> makes arguments to write-region easier to use, as well as making it easy
> to control write-region-inhibit-fsync and file-precious-flag. Without
> this macro, these operations and options are, IMO, harder to use and
> easy to overlook, so I think this would be a worthwhile addition.
>
> If this idea seems useful, I'm sure various improvements may be made, so
> I would appreciate any feedback. For example, it might be better for
> ":insert nil" to be the default, so ":insert t" would be specified to
> read the file's contents into the buffer.
>
> Thanks,
> Adam
>
> From 37df936cc1c9e3f630a060c8347a4aeba46fc7d0 Mon Sep 17 00:00:00 2001
> From: Adam Porter <adam@alphapapa.net>
> Date: Thu, 31 Dec 2020 12:56:25 -0600
> Subject: [PATCH] * lisp/subr.el (with-file-buffer): New macro.
>
> * lisp/subr.el (with-file-buffer): New macro.
> * doc/lispref/files.texi (Writing to Files): Document it.
> * etc/NEWS: Add news entry.
> ---
> doc/lispref/files.texi | 48 ++++++++++++++++++++++++++++++++++++++++++
> etc/NEWS | 6 ++++++
> lisp/subr.el | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 111 insertions(+)
>
> diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
> index 6949ca2..1617bbf 100644
> --- a/doc/lispref/files.texi
> +++ b/doc/lispref/files.texi
> @@ -708,6 +708,54 @@ Writing to Files
> (@pxref{Buffer List}).
> @end defmac
>
> +@defmac with-file-buffer file options body@dots{}
> +@anchor{Definition of with-file-buffer}
> +Like the @code{with-temp-file} macro (which see), the
> +@code{with-file-buffer} macro evaluates the @var{body} forms with a
> +temporary buffer as the current buffer and then returns the value of
> +the last form in @var{body}. The @var{options} offer control over
> +various aspects of the operations:
> +
> +@table @code
> +@item :insert
> +When non-nil (the default, when unspecified), insert @var{file}'s contents
> +before evaluating @var{body}, leaving point before the contents.
> +
> +@item :must-exist
> +When non-nil, signal an error if @var{file} does not exist.
> +
> +@item :write
> +When non-nil, write the contents of the buffer to @var{file} after
> +evaluating @var{body}.
> +
> +@item :overwrite
> +When nil (the default, when unspecified), signal an error instead of
> +overwriting an existing file. If @code{ask}, ask for confirmation
> +before overwriting an existing file. If @code{t}, overwrite
> +@var{file} unconditionally.
> +
> +@item :visit
> +Passed to function @code{write-region}, which see.
> +
> +@item :lockname
> +Passed to function @code{write-region}, which see.
> +
> +@item :append
> +Passed to function @code{write-region}, which see. (When using this
> +option, you will probably want to specify @code{:insert nil} as well,
> +otherwise the file's contents would be duplicated.)
> +
> +@item :fsync
> +When non-nil (the default, when unspecified), bind
> +@var{write-region-inhibit-fsync} (which see) to this value.
> +
> +@item :precious
> +Bind @var{file-precious-flag} (which see) to this
> +value (when unspecified, nil).
> +@end table
> +
> +@end defmac
> +
> @node File Locks
> @section File Locks
> @cindex file locks
> diff --git a/etc/NEWS b/etc/NEWS
> index 62907a6..9b8288f 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -2311,6 +2311,12 @@ locales. They are also available as aliases 'ebcdic-cp-*' (e.g.,
> 'cp278' for 'ibm278'). There are also new charsets 'ibm2xx' to
> support these coding-systems.
>
> +---
> +** New macro 'with-file-buffer'.
> +The new macro 'with-file-buffer' simplifies operating on the contents
> +of files and/or writing to them, compared to using 'with-temp-buffer',
> +'insert-file-contents', 'write-region', and various options
> +separately.
> \f
> * Changes in Emacs 28.1 on Non-Free Operating Systems
>
> diff --git a/lisp/subr.el b/lisp/subr.el
> index 77b142c..0f6da02 100644
> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -3992,6 +3992,63 @@ with-temp-buffer
> (and (buffer-name ,temp-buffer)
> (kill-buffer ,temp-buffer)))))))
>
> +(defmacro with-file-buffer (file options &rest body)
> + "Evaluate BODY and return its value in a temp buffer for FILE.
> +OPTIONS is a plist of the following options:
> +
> +`:insert': When non-nil (the default, when unspecified), insert
> +file's contents before evaluating BODY, leaving point before the
> +contents.
> +
> +`:must-exist': When non-nil, signal an error if no file exists at
> +FILE.
> +
> +`:write': When non-nil, write the contents of the buffer to FILE
> +after evaluating BODY.
> +
> +`:overwrite': When nil (the default, when unspecified), signal an
> +error instead of overwriting an existing file at FILE. If `ask',
> +ask for confirmation before overwriting an existing file. If t,
> +overwrite a file at FILE unconditionally.
> +
> +`:visit': Passed to function `write-region', which see.
> +
> +`:lockname:' Passed to function `write-region', which see.
> +
> +`:append': Passed to function `write-region', which see. (When
> +using this option, you will probably want to specify `:insert
> +nil' as well.)
> +
> +`:fsync': When non-nil (the default, when unspecified), bind
> +`write-region-inhibit-fsync' (which see) to this value.
> +
> +`:precious': Bind `file-precious-flag' (which see) to this
> +value (when unspecified, nil)."
> + (declare (indent 2) (debug (stringp form body)))
> + `(let ((write-region-inhibit-fsync ,(when (plist-member options :fsync)
> + (not (plist-get options :fsync))))
> + (file-precious-flag ,(plist-get options :precious)))
> + (with-temp-buffer
> + ,(when (or (not (plist-member options :insert))
> + (plist-get options :insert))
> + `(if (file-readable-p ,file)
> + (save-excursion
> + (insert-file-contents ,file))
> + (when ,(plist-get options :must-exist)
> + (error "File not readable: %s" ,file))))
> + (prog1
> + (progn
> + ,@body)
> + ,(when (plist-get options :write)
> + `(write-region nil nil ,file
> + ,(plist-get options :append)
> + ,(plist-get options :visit)
> + ,(plist-get options :lockname)
> + ,(pcase-exhaustive (plist-get options :overwrite)
> + ('nil ''excl)
> + ((or 'ask ''ask) ''ask)
> + ('t nil))))))))
> +
> (defmacro with-silent-modifications (&rest body)
> "Execute BODY, pretending it does not modify the buffer.
> This macro is typically used around modifications of
I am not an expert in Lisp; but I personally think those 'with-*' macros
are more idiomatic ot Lisp. They are also communicate clearly the scope
where a variable (in this case buffer or file are used). I understand
Eli's remark that thay don't remove much to the burden of user in LoCs
the user have to write, but they do document the user intention and
helps writing better self-documenting code. I compare this personally to
std::copy from C++ vs raw for-loop. std::copy does not make us write
less code; usually if the loop body is big we will refactor it in a
callback, but it does document the intention. Elisp is not as efficient
as c++ compiler to eliminate the overhead, but macros will be expanded
when byte-compiled, so it really is just the memory overhead of defining
the body of mcaro.
I personally use, beside the macro I posted in documentation thread,
those two macros, which are admittedly bad, but works for me personally:
#+BEGIN_SRC elisp
(defmacro with-file-prepend (file &rest body)
(declare (indent 1) (debug t))
`(let ((buffer (get-buffer-create ,file)))
(unwind-protect
(prog1
(with-current-buffer buffer
,@body
(goto-char (point-max))
(if (file-readable-p ,file)
(insert-file-contents ,file))
(with-current-buffer buffer
(write-region nil nil ,file nil 0)))
(and (buffer-name buffer)
(kill-buffer buffer))))))
(defmacro with-file-append (file &rest body)
(declare (indent 1) (debug t))
`(let ((buffer (get-buffer-create ,file)))
(unwind-protect
(prog1
(with-current-buffer buffer
(if (file-readable-p ,file)
(insert-file-contents ,file))
(goto-char (point-max))
,@body)
(with-current-buffer buffer
(write-region nil nil ,file nil 0)))
(and (buffer-name buffer)
(kill-buffer buffer)))))
#+END_SRC
I can then use them like:
#+BEGIN_SRC elisp
(with-file-prepend "early-init.el"
(insert (concat ";;; early-init.el -*- lexical-binding: t; -*-\n"
";;; This file is machine generated by init file generator, don't edit\n"
";;; manually, edit instead file init.org and generate new init file from it\n\n")))
(with-file-append "init.el"
(insert (concat "\n;; Local Variables:\n"
";; byte-compile-warnings: (not free-vars unresolved))\n"
";; End:\n")))
#+END_SRC
I don't suggest to use/includ those, just that I find it personally
handy, and more idiomatic then using say f.el or write-region directly
which is kind-of more C/Java-like APIs.
I personally am not qualified to say if plist implementation of
'with-file-buffer' macro is good or not; but I think the intention is
certainly good. If you are already buffing string library, make you
could buff file API too.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-01-01 14:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-31 19:13 [PATCH] New with-file-buffer macro Adam Porter
2020-12-31 19:48 ` Eli Zaretskii
2021-01-01 14:32 ` Arthur Miller
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).