GNU bug report logs - #39190
28.0.50; two buffers with same buffer-file-name (diff-syntax-fontify-props)

Previous Next

Full log

Message #71 received at 39190 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 39190 <at> debbugs.gnu.org, felician.nemeth <at> gmail.com, juri <at> linkov.net
Subject: Re: bug#39190: 28.0.50; two buffers with same buffer-file-name
 (diff-syntax-fontify-props)
Date: Thu, 30 Jan 2020 09:34:28 -0500

>> >  . change set-auto-mode to accept another optional argument, the file
>> >    name to use to look up the mode
>> >
>> >  . perform look up of auto-mode-alist, then invoke the mode directly
>> 
>> The issue is that we also want to obey dir-locals and both above options
>> seem to become more invasive once we try and handle those (the first
>> above option is the first one I proposed, since I prefer
>> lexically-scoped args over dynamically-scoped vars ;-)
>
> What is the problem of the first alternative wrt dir-locals?  I don't
> think I understand that.

As I wrote: it's more invasive because you have to pass that extra arg
through various functions.

>> > Also, setting the pseudo-filename is not guaranteed to turn on the
>> > mode according to that file name.  Not sure if this matters in these
>> > cases.
>> Not sure what you mean here.
> set-auto-mode looks on other mode-determining stuff, before looking at
> the file name.

Ah, right.  This is a side-issue, yes: in cases like
`vc-revision-other-window` it does exactly what we want (i.e. it takes
advantage of the buffer's content (file-local vars, magic strings, ...)
to guess the mode), but in cases like diff-mode it can be detrimental
(since we don't have the full file content in that case, but only
a chunk of it, which means that things like file-local vars not only
will usually not be found but can even be incorrectly found).

I don't think it makes much difference to the current discussion, tho
(it's largely orthogonal).

>> > And finally, I cannot say that I like this part of the patch:
>> >
>> >   @@ -3459,6 +3460,8 @@ hack-local-variables-confirm
>> >        (let ((name (cond (dir-name)
>> > 			(buffer-file-name
>> > 			 (file-name-nondirectory buffer-file-name))
>> >   +		      (buffer-file-name-for-mode
>> >   +		       (file-name-nondirectory buffer-file-name-for-mode))
>> > 			((concat "buffer " (buffer-name)))))
>> > 	    (offer-save (and (eq enable-local-variables t)
>> > 			     unsafe-vars))
>> >
>> > If buffer-file-name-for-mode is not really a file name, we shouldn't
>> > call file-name-nondirectory on it.
>> 
>> It is supposed to be a file name.  It's only that the buffer is not
>> supposed to be the buffer corresponding to that file.
>
> Yes, but having leading directories in it feels... unclean,

Why so?  The leading directories are crucial for dir-locals support, and
they can also be important for auto-mode-alist.

>> That code is predicated on `buffer-file-name-for-mode` being
>> non-nil, AFAICT, so I think we're OK in this regard.
> Non-nil doesn't mean it's a string.

If someone sets it to non-nil and not a string, they'll get what
they deserve.  Same holds already for `buffer-file-name`, so it's not
a new problem introduced with this change.


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.