GNU bug report logs -
#78777
30.1; insert-file-contents should not set buffer-file-name to nil
Previous Next
Full log
Message #56 received at 78777 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
> In article <jwvtt4fag1z.fsf-monnier+emacs <at> gnu.org>, Stefan Monnier
> <monnier <at> iro.umontreal.ca> writes:
> [...]
>> (Finsert_file_contents): On replacing, temporarily bind
>> buffer-file-name to Qnil before calling insert_from_buffer.
>
>> diff --git a/src/fileio.c b/src/fileio.c
>> --- a/src/fileio.c
>> +++ b/src/fileio.c
>> @@ -4359,8 +4359,12 @@
>> inserted_chars
>> = (buf_bytepos_to_charpos (XBUFFER (conversion_buffer),
>> same_at_start + inserted)
>> - same_at_start_charpos);
>> + /* This binding is to avoid ask-user-about-supersession-threat
>> + being called in insert_from_buffer (via in
>> + prepare_to_modify_buffer). */
>> + specbind (intern ("buffer-file-name"), Qnil);
>> insert_from_buffer (XBUFFER (conversion_buffer),
>> same_at_start_charpos, inserted_chars, 0);
>> /* Set `inserted' to the number of inserted characters. */
>> inserted = PT - temp;
>> ```
>
>> I'm not sure exactly what was the motivation for the change.
>> I mean, the comment explains that the purpose is to silence a potential
>> `ask-user-about-supersession-threat`, but I don't know why we should
>> care about such a possibility and why it would be right to silence
>> it here.
> I don't remember, sorry.
AFAICT this is used only when REPLACE is non-nil but I wonder whether it
should be limited also to the case where VISIT is non-nil.
E.g. after removing the three `specbind (Qbuffer_file_name", Qnil)` we
have in the current code (which are the descendants of the above
`specbind`), I see that I can get a "changed on disk; really edit"
prompt if I do the following:
% emacs -Q src/regex-emacs.c &
% echo foo >>src/regex-emacs.c
in Emacs do: M-x revert-buffer RET
which is indeed undesirable and might be the original reason for
commit d07af40d8826.
But if I use the patch below to restrict the `specbind` to the case
where VISIT is non-nil, then the above recipe still works as it should.
>> Nor do I understand why it was added to the code path where coding
>> conversion was needed but not to the other one.
>
> ??? It seems that the code above is where coding conversion is not
> needed.
I don't understand: the patch (still quoted above) touches code which
refers to `buf_bytepos_to_charpos` and `conversion_buffer`, so it's
clearly a part of the code which had to deal with coding conversion.
I'm not a great fan of this `specbind` and maybe we could/should
inhibit the ask-supersession prompt in a more direct way, but I think
the patch below is an improvement (and should fix the "lsp-mode +
reformatter" case that triggered the current bug report).
Any objection to pushing it to `master`?
Stefan
[insert-file-contents.patch (text/x-diff, inline)]
diff --git a/src/fileio.c b/src/fileio.c
index 95bfa979d13..73c97b2999e 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -4568,14 +4568,19 @@ DEFUN ("insert-file-contents", Finsert_file_contents, Sinsert_file_contents,
beg_offset += same_at_start - BEGV_BYTE;
end_offset -= ZV_BYTE - same_at_end;
- /* This binding is to avoid ask-user-about-supersession-threat
- being called in insert_from_buffer or del_range_bytes (via
- prepare_to_modify_buffer).
- AFAICT we could avoid ask-user-about-supersession-threat by setting
- current_buffer->modtime earlier, but we could still end up calling
- ask-user-about-supersession-threat if the file is modified while
- we read it, so we bind buffer-file-name instead. */
- specbind (Qbuffer_file_name, Qnil);
+ if (!NILP (visit))
+ /* This binding is to avoid ask-user-about-supersession-threat
+ being called in insert_from_buffer or del_range_bytes (via
+ prepare_to_modify_buffer).
+ This prompt makes no sense if we're VISITing the file,
+ since the insertion makes the buffer *more* like the file
+ rather than the reverse.
+ AFAICT we could avoid ask-user-about-supersession-threat by
+ setting current_buffer->modtime earlier, but we could still
+ end up calling ask-user-about-supersession-threat if the file
+ is modified while we read it, so we bind buffer-file-name
+ instead. */
+ specbind (Qbuffer_file_name, Qnil);
del_range_byte (same_at_start, same_at_end);
/* Insert from the file at the proper position. */
temp = BYTE_TO_CHAR (same_at_start);
@@ -4685,7 +4690,8 @@ DEFUN ("insert-file-contents", Finsert_file_contents, Sinsert_file_contents,
if (same_at_start != same_at_end)
{
/* See previous specbind for the reason behind this. */
- specbind (Qbuffer_file_name, Qnil);
+ if (!NILP (visit))
+ specbind (Qbuffer_file_name, Qnil);
del_range_byte (same_at_start, same_at_end);
}
inserted = 0;
@@ -4735,7 +4741,8 @@ DEFUN ("insert-file-contents", Finsert_file_contents, Sinsert_file_contents,
inserted -= (ZV_BYTE - same_at_end) + (same_at_start - BEGV_BYTE);
/* See previous specbind for the reason behind this. */
- specbind (Qbuffer_file_name, Qnil);
+ if (!NILP (visit))
+ specbind (Qbuffer_file_name, Qnil);
if (same_at_end != same_at_start)
{
del_range_byte (same_at_start, same_at_end);
This bug report was last modified 56 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.