Subject: Re: bug#25340: Save-some-buffers gets called when installing packages.

From: [email-masked]
Date: Sun, 08 Jan 2017 00:33:41 -0500

Nikolay Kudryavtsev <[email-masked]> writes:

> I had the idea of doing this:
>
> (defun byte-recompile-directory (directory &optional arg force
> limit-buffer-saving)
>
> ...
>
> If forth argument LIMIT-BUFFER-SAVING is 'directory, only files within
> DIRECTORY would be saved.  When LIMIT-BUFFER-SAVING is t no existing buffers
> would be checked for modification.
>
>
> But I found out out that having LIMIT-BUFFER-SAVING with value t like
> this would not work without an accompanying change to
> byte-recompile-file - since byte-recompile-file already checks whether
> there's a buffer for that file and that buffer is modified. So, since
> I don't think that doing changes to byte-recompile-file is the best
> idea, here's my current version.

Ah, it's in byte-compile-file, not byte-REcompile-file.  Since with the
current code, if there is a buffer you don't want to save before
compiling, you have to answer "no" twice, I think changing
byte-compile-file would be a good thing to do.  But we could leave it
for later.

>
> Also, note that the check in byte-recompile-file actually makes
> calling save-some-buffers somewhat redundant.

I would say the redundancy is in the other direction, but yes, agreed.

Patch looks okay, just a few minor comments, below.

> * doc/lispref/compile.texi: Documented `limit-buffer-saving' argument
                              ^^^^^^^^^^
Use present tense ("Document `foo' argument...").

> 
> +By default, user is prompted before recompilation to save modified
> +buffers one by one. Setting @var{limit-buffer-saving} to @code{t} checks
                     ^^^
End sentences with double space.

> +If forth argument LIMIT-BUFFER-SAVING is t only buffers within
      ^^^^^                                  ^
      fourth                                comma here, I think

> +DIRECTORY would be checked for modification."
             ^^^^^^^^^^^^^^^^
I think that should be "are checked".

> +    (if limit-buffer-saving
> +      (save-some-buffers
> +       nil
> +       (lambda ()
> +         (string-prefix-p (expand-file-name directory)
> +                          (expand-file-name buffer-file-name))))
> +      (save-some-buffers))

I would rather avoid doubling the call to save-some-buffers, so use
something like

    (save-some-buffers
     nil
     (if limit-buffer-saving
         (lambda () ...)))