GNU bug report logs - #42431
28.0.50; browse-url-of-dired-file confusing messaging

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 42431 in the body.
You can then email your comments to 42431 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; browse-url-of-dired-file confusing messaging
Date: Sun, 19 Jul 2020 21:34:02 +0200

Create a file /tmp/a.txt, and visit /tmp.
M-x browse-url-of-dired-file on the file.

Emacs will say:

File exists, but cannot be read

and then display the file in a text-mode buffer.

If you then go to the real a.txt buffer, Emacs will then say

/tmp/a.txt and file:///tmp/a.txt are the same file

All this is very confusing, because the doc string of the command is

--
In Dired, ask a WWW browser to display the file named on this line.
--

So it should just call `browse-url' on the file:// URL, and not do
anything about these buffers?



In GNU Emacs 28.0.50 (build 8, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, cairo version 1.16.0)
 of 2020-07-19 built on xo
Repository revision: 17f646128f04e9e8590f0371026a14d516f21c63
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: Debian GNU/Linux bullseye/sid


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: 42431 <at> debbugs.gnu.org
Subject: Re: bug#42431: 28.0.50; browse-url-of-dired-file confusing messaging
Date: Sun, 19 Jul 2020 21:37:38 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> All this is very confusing, because the doc string of the command is
>
> --
> In Dired, ask a WWW browser to display the file named on this line.
> --
>
> So it should just call `browse-url' on the file:// URL, and not do
> anything about these buffers?

Ah, it's because of this --

(defvar browse-url-default-handlers
  '(("\\`mailto:" . browse-url--mailto)
    ("\\`man:" . browse-url--man)
    (browse-url--non-html-file-url-p . browse-url-emacs))

browse-url on a file that doesn't have .html in it will now not open a
browser at all, but instead an Emacs buffer.

In any case, the messaging is misleading and the buffer name is
confusing.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: 42431 <at> debbugs.gnu.org
Cc: Michael Albinus <michael.albinus <at> gmx.de>
Subject: Re: bug#42431: 28.0.50; browse-url-of-dired-file confusing messaging
Date: Tue, 28 Jul 2020 00:24:23 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Create a file /tmp/a.txt, and visit /tmp.
> M-x browse-url-of-dired-file on the file.
>
> Emacs will say:
>
> File exists, but cannot be read
>
> and then display the file in a text-mode buffer.

Here's a simpler reproduction.  Ensure that /tmp/a.txt exists, and then
eval:

(browse-url-of-file "/tmp/a.txt")

The message originates from here, sort of:

(defun find-file-noselect-1 (buf filename nowarn rawfile truename number)

[...]

	  (condition-case ()
	      (let ((inhibit-read-only t))
		(insert-file-contents-literally filename t))
	    (file-error
	     (when (and (file-exists-p filename)
			(not (file-readable-p filename)))
	       (kill-buffer buf)
	       (signal 'file-error (list "File is not readable"
					 filename)))

If we remove that condition-case, we get:

Debugger entered--Lisp error: (file-error "Opening input file" "Success")
  insert-file-contents("file:///tmp/a.txt" t)

So the insert-file-contents-literally signals a "Success" file-error
when using url-handler-mode.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: michael.albinus <at> gmx.de, 42431 <at> debbugs.gnu.org
Subject: Re: bug#42431: 28.0.50; browse-url-of-dired-file confusing messaging
Date: Sat, 08 Aug 2020 10:46:55 +0300

> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Date: Tue, 28 Jul 2020 00:24:23 +0200
> Cc: Michael Albinus <michael.albinus <at> gmx.de>
> 
> (browse-url-of-file "/tmp/a.txt")
> 
> The message originates from here, sort of:
> 
> (defun find-file-noselect-1 (buf filename nowarn rawfile truename number)
> 
> [...]
> 
> 	  (condition-case ()
> 	      (let ((inhibit-read-only t))
> 		(insert-file-contents-literally filename t))
> 	    (file-error
> 	     (when (and (file-exists-p filename)
> 			(not (file-readable-p filename)))
> 	       (kill-buffer buf)
> 	       (signal 'file-error (list "File is not readable"
> 					 filename)))

Is this because file-readable-p returns nil for file:// URLs?

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: michael.albinus <at> gmx.de, 42431 <at> debbugs.gnu.org
Subject: Re: bug#42431: 28.0.50; browse-url-of-dired-file confusing messaging
Date: Sat, 08 Aug 2020 12:05:04 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> (defun find-file-noselect-1 (buf filename nowarn rawfile truename number)
>> 
>> [...]
>> 
>> 	  (condition-case ()
>> 	      (let ((inhibit-read-only t))
>> 		(insert-file-contents-literally filename t))
>> 	    (file-error
>> 	     (when (and (file-exists-p filename)
>> 			(not (file-readable-p filename)))
>> 	       (kill-buffer buf)
>> 	       (signal 'file-error (list "File is not readable"
>> 					 filename)))
>
> Is this because file-readable-p returns nil for file:// URLs?

That's the direct cause of the message, but the underlying reason is
that insert-file-contents-literally signalled a file-error here (after
inserting the contents).  I haven't yet chased down why.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: michael.albinus <at> gmx.de, 42431 <at> debbugs.gnu.org
Subject: Re: bug#42431: 28.0.50; browse-url-of-dired-file confusing messaging
Date: Sat, 08 Aug 2020 13:22:47 +0300

> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Cc: michael.albinus <at> gmx.de,  42431 <at> debbugs.gnu.org
> Date: Sat, 08 Aug 2020 12:05:04 +0200
> 
> >> 	  (condition-case ()
> >> 	      (let ((inhibit-read-only t))
> >> 		(insert-file-contents-literally filename t))
> >> 	    (file-error
> >> 	     (when (and (file-exists-p filename)
> >> 			(not (file-readable-p filename)))
> >> 	       (kill-buffer buf)
> >> 	       (signal 'file-error (list "File is not readable"
> >> 					 filename)))
> >
> > Is this because file-readable-p returns nil for file:// URLs?
> 
> That's the direct cause of the message, but the underlying reason is
> that insert-file-contents-literally signalled a file-error here (after
> inserting the contents).  I haven't yet chased down why.

I guess that's because expand-file-name doesn't convert file:// URLs
into local file names, and then insert-file-contents chokes on the
value produced by expand-file-name.  (insert-file-contents-literally
is just a thin wrapper around insert-file-contents.)

So one solution would be to convert file:// URLs into local file names
in the above snippet, before calling insert-file-contents-literally.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: michael.albinus <at> gmx.de, 42431 <at> debbugs.gnu.org
Subject: Re: bug#42431: 28.0.50; browse-url-of-dired-file confusing messaging
Date: Sun, 09 Aug 2020 11:35:25 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Lars Ingebrigtsen <larsi <at> gnus.org>
>> Cc: michael.albinus <at> gmx.de,  42431 <at> debbugs.gnu.org
>> Date: Sat, 08 Aug 2020 12:05:04 +0200
>> 
>> >> 	  (condition-case ()
>> >> 	      (let ((inhibit-read-only t))
>> >> 		(insert-file-contents-literally filename t))
>> >> 	    (file-error
>> >> 	     (when (and (file-exists-p filename)
>> >> 			(not (file-readable-p filename)))
>> >> 	       (kill-buffer buf)
>> >> 	       (signal 'file-error (list "File is not readable"
>> >> 					 filename)))
>> >
>> > Is this because file-readable-p returns nil for file:// URLs?
>> 
>> That's the direct cause of the message, but the underlying reason is
>> that insert-file-contents-literally signalled a file-error here (after
>> inserting the contents).  I haven't yet chased down why.

My analysis here was kinda wrong -- the code above isn't what gives the
warning, because all those functions up there do the right thing, since
file-name-handler-alist is set:

  (let ((file-name-handler-alist
         (cons (cons url-handler-regexp 'url-file-handler)
               file-name-handler-alist)))
    (list (file-exists-p "file:///tmp/a.txt")
	  (file-readable-p "file:///tmp/a.txt")))
=> (t t)

The problem is that insert-file-contents signals a file-error, and the
error string is "Success".  This makes things confused, because it knows
that it has an error, but when it tests for all things that could go
wrong, it doesn't find anything, and ends up here:

(defun after-find-file (&optional error warn noauto
				  _after-find-file-from-revert-buffer
				  nomodes)
[...]
	     ((and error (file-attributes buffer-file-name))
	      (setq buffer-read-only t)
	      (if (and (file-symlink-p buffer-file-name)
		       (not (file-exists-p
			     (file-chase-links buffer-file-name))))
		  "Symbolic link that points to nonexistent file"
		"File exists, but cannot be read"))

Which is where the message itself comes from.

> So one solution would be to convert file:// URLs into local file names
> in the above snippet, before calling insert-file-contents-literally.

It would be, but I think this points to an error in insert-file-contents
itself.  I'll poke around some more...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, michael.albinus <at> gmx.de,
 42431 <at> debbugs.gnu.org
Subject: Re: bug#42431: 28.0.50; browse-url-of-dired-file confusing messaging
Date: Sun, 09 Aug 2020 11:45:05 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> It would be, but I think this points to an error in insert-file-contents
> itself.  I'll poke around some more...

Yup.  The error signalling comes from Finsert_file_contents.  If I make
this change, then the confusing messaging goes away:

diff --git a/src/fileio.c b/src/fileio.c
index 37072d9b6b..05e262b201 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -4826,7 +4826,6 @@ because (1) it preserves some marker positions and (2) it puts less data
   if (!NILP (visit) && current_buffer->modtime.tv_nsec < 0)
     {
       /* Signal an error if visiting a file that could not be opened.  */
-      report_file_errno ("Opening input file", orig_filename, save_errno);
     }
 
   /* We made a lot of deletions and insertions above, so invalidate

This was apparently introduced/changed in 2019 by this patch:

commit 3a1e7624ed234bb434cdafed59515cadd037cafa
Author:     Paul Eggert <eggert <at> cs.ucla.edu>
AuthorDate: Thu Oct 31 23:31:17 2019 -0700
Commit:     Paul Eggert <eggert <at> cs.ucla.edu>
CommitDate: Thu Oct 31 23:32:05 2019 -0700

    Fix insert-file-contents file error regression
    
    Problem reported for dired-view-file (Bug#37950).
    * src/fileio.c (Finsert_file_contents): When visiting,
    signal an error if the file could not be opened for any reason,
    rather than signaling an error only for nonexistent files, fixing
    a bug introduced in 2019-09-16T03:17:43!eggert <at> cs.ucla.edu.

I've Cc'd Paul on this.  Paul, the test case is:

(browse-url-of-file "/tmp/a.txt")

This will open the file correctly (via the url-file-handler file name
handler), but Emacs will then message "File exists, but cannot be read"
because Finsert_file_contents signals an error with the error message
"Success".

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: eggert <at> cs.ucla.edu, michael.albinus <at> gmx.de, 42431 <at> debbugs.gnu.org
Subject: Re: bug#42431: 28.0.50; browse-url-of-dired-file confusing messaging
Date: Sun, 09 Aug 2020 17:05:14 +0300

> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Cc: michael.albinus <at> gmx.de,  42431 <at> debbugs.gnu.org, Paul Eggert
>  <eggert <at> cs.ucla.edu>
> Date: Sun, 09 Aug 2020 11:45:05 +0200
> 
> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> 
> > It would be, but I think this points to an error in insert-file-contents
> > itself.  I'll poke around some more...
> 
> Yup.  The error signalling comes from Finsert_file_contents.  If I make
> this change, then the confusing messaging goes away:
> 
> diff --git a/src/fileio.c b/src/fileio.c
> index 37072d9b6b..05e262b201 100644
> --- a/src/fileio.c
> +++ b/src/fileio.c
> @@ -4826,7 +4826,6 @@ because (1) it preserves some marker positions and (2) it puts less data
>    if (!NILP (visit) && current_buffer->modtime.tv_nsec < 0)
>      {
>        /* Signal an error if visiting a file that could not be opened.  */
> -      report_file_errno ("Opening input file", orig_filename, save_errno);
>      }
>  

Of course.  And that's exactly what I meant when I suggested to
convert the file:// URL to a local file name, before calling
insert-file-contents.  If we do that, the problem should go away.  Or
am I missing something?

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: eggert <at> cs.ucla.edu, michael.albinus <at> gmx.de, 42431 <at> debbugs.gnu.org
Subject: Re: bug#42431: 28.0.50; browse-url-of-dired-file confusing messaging
Date: Sun, 09 Aug 2020 16:08:58 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

> Of course.  And that's exactly what I meant when I suggested to
> convert the file:// URL to a local file name, before calling
> insert-file-contents.  If we do that, the problem should go away.  Or
> am I missing something?

I just thought all of this was supposed to work without doing any
mangling of the "file name" -- in this case it's an URL, but presumably
there are other file handlers for non-file stuff that Emacs treats are
files that have the same problem.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, michael.albinus <at> gmx.de,
 42431 <at> debbugs.gnu.org
Subject: Re: bug#42431: 28.0.50; browse-url-of-dired-file confusing messaging
Date: Thu, 13 Oct 2022 09:00:38 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> @@ -4826,7 +4826,6 @@ because (1) it preserves some marker positions and (2) it puts less data
>    if (!NILP (visit) && current_buffer->modtime.tv_nsec < 0)
>      {
>        /* Signal an error if visiting a file that could not be opened.  */
> -      report_file_errno ("Opening input file", orig_filename, save_errno);
>      }

I've now fixed this in Emacs 29.

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

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>
Cc: michael.albinus <at> gmx.de, 42431 <at> debbugs.gnu.org
Subject: Re: bug#42431: 28.0.50; browse-url-of-dired-file confusing messaging
Date: Thu, 13 Oct 2022 01:51:00 -0700

[Message part 1 (text/plain, inline)]

On 2022-10-13 00:00, Lars Ingebrigtsen wrote:

> I've now fixed this in Emacs 29.

Thanks, I simplified the fileio.c fix by installing the attached further 
patch.

[0001-Simplify-recent-File-exists-fix.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: larsi <at> gnus.org, michael.albinus <at> gmx.de, 42431 <at> debbugs.gnu.org
Subject: Re: bug#42431: 28.0.50; browse-url-of-dired-file confusing messaging
Date: Thu, 13 Oct 2022 13:35:44 +0300

> Date: Thu, 13 Oct 2022 01:51:00 -0700
> Cc: michael.albinus <at> gmx.de, 42431 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> 
> --- a/src/fileio.c
> +++ b/src/fileio.c
> @@ -5000,7 +5000,7 @@ because (1) it preserves some marker positions (in unchanged portions
>        unbind_to (count1, Qnil);
>      }
>  
> -  if (NILP (handler) && !NILP (visit) && current_buffer->modtime.tv_nsec < 0)
> +  if (save_errno != 0)
>      {
>        /* Signal an error if visiting a file that could not be opened.  */
>        report_file_errno ("Opening input file", orig_filename, save_errno);

Why would we want to signal an error when VISIT is nil?  We didn't
before.

Previous Next

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