tramp+recentf: persistent errors due to expand-file-name

tramp+recentf: persistent errors due to expand-file-name

[David Reitter]

[-- Attachment #1: Type: text/plain, Size: 4587 bytes --]

I've been playing around with tramp, which does not recognize a  
location such as /dr@localhost:/tmp/123123//bar  as a local /bar file.

Worse than that, the error that ensues (in combination with recentf- 
mode) prevents users from saving or openeing other files in the future:

Debugger entered--Lisp error: (error "Not a tramp file name: /qweqwe")
   signal(error ("Not a tramp file name: /qweqwe"))
   ad-Orig-error("Not a tramp file name: %s" "/qweqwe")
   apply(ad-Orig-error ("Not a tramp file name: %s" "/qweqwe"))
   error("Not a tramp file name: %s" "/qweqwe")
   tramp-dissect-file-name("/qweqwe")
   tramp-file-name-handler(expand-file-name "/dr@localhost:/tmp/ 
123123//qweqwe" nil)
   expand-file-name("/dr@localhost:/tmp/123123//qweqwe")
   recentf-expand-file-name("/dr@localhost:/tmp/123123//qweqwe")
   recentf-track-opened-file()
   run-hook-with-args-until-success(recentf-track-opened-file)
   basic-save-buffer()
   save-buffer()
   mac-key-save-file()
   call-interactively(mac-key-save-file)

when opening:

Debugger entered--Lisp error: (error "Not a tramp file name: /qweqwe")
   signal(error ("Not a tramp file name: /qweqwe"))
   ad-Orig-error("Not a tramp file name: %s" "/qweqwe")
   apply(ad-Orig-error ("Not a tramp file name: %s" "/qweqwe"))
   error("Not a tramp file name: %s" "/qweqwe")
   tramp-dissect-file-name("/qweqwe")
   tramp-file-name-handler(expand-file-name "/dr@localhost:/tmp/ 
123123//qweqwe" nil)
   expand-file-name("/dr@localhost:/tmp/123123//qweqwe")
   recentf-expand-file-name("/dr@localhost:/tmp/123123//qweqwe")
   recentf-track-opened-file()
   run-hooks(find-file-hook)
   after-find-file(nil t)
   find-file-noselect-1(#<buffer recentf.el.gz> "/Applications/ 
Aquamacs Emacs.app/Contents/Resources/lisp/recentf.el.gz" nil nil "/ 
Applications/Aquamacs Emacs.app/Contents/Resources/lisp/ 
recentf.el.gz" (21798801 234881026))
   find-file-noselect("/Applications/Aquamacs Emacs.app/Contents/ 
Resources/lisp/recentf.el.gz")
   find-function-search-for-symbol(recentf-track-opened-file nil  
"recentf.el")
   #[(fun file) "⇄蠀ࠀ씀=ƒ\x11\0ই℀섀\"\x10ৈ준\b#\x1a峊渀䀀℀蠀 
尀渀䄀茀✀\0尀渀䄀戀舀⨀\0찀!)‡" [file fun location require  
find-func C-source help-C-file-name indirect-function find-function- 
search-for-symbol nil pop-to-buffer message "Unable to find location  
in file"] 4](recentf-track-opened-file "recentf.el")
   apply(#[(fun file) "⇄蠀ࠀ씀=ƒ\x11\0ই℀섀\"\x10ৈ준\b#\x1a峊渀䀀 
℀蠀尀渀䄀茀✀\0尀渀䄀戀舀⨀\0찀!)‡" [file fun location  
require find-func C-source help-C-file-name indirect-function find- 
function-search-for-symbol nil pop-to-buffer message "Unable to find  
location in file"] 4] (recentf-track-opened-file "recentf.el"))
   help-do-xref(59 #[(fun file) "⇄蠀ࠀ씀=ƒ\x11\0ই℀섀\"\x10ৈ준\b#\x1a 
峊渀䀀℀蠀尀渀䄀茀✀\0尀渀䄀戀舀⨀\0찀!)‡" [file fun  
location require find-func C-source help-C-file-name indirect-function  
find-function-search-for-symbol nil pop-to-buffer message "Unable to  
find location in file"] 4] (recentf-track-opened-file "recentf.el"))
   help-button-action(#<marker (moves after insertion) at 63 in *Help*>)
   push-button(63 t)
   push-button((mouse-2 (#<window 15 on *Help*> 63 (439 . 10) 71778152  
nil 63 (62 . 0) nil (5 . 10) (7 . 15))))
   call-interactively(push-button)


The patch below fixes this - comments appreciated.

The alternative would be to fix tramp so it doesn't throw an error  
when expand-file-name is called, but this might prevent it from giving  
proper feedback to the user.



*** lisp/recentf.el	06 Apr 2008 13:51:37 +0200	1.56.2.2
--- lisp/recentf.el	10 Aug 2008 19:04:41 +0200	
***************
*** 394,400 ****
     "Convert file NAME to absolute, and canonicalize it.
   NAME is first passed to the function `expand-file-name', then to
   `recentf-filename-handlers' to post process it."
!   (recentf-apply-filename-handlers (expand-file-name name)))

   (defun recentf-include-p (filename)
     "Return non-nil if FILENAME should be included in the recent list.
--- 394,402 ----
     "Convert file NAME to absolute, and canonicalize it.
   NAME is first passed to the function `expand-file-name', then to
   `recentf-filename-handlers' to post process it."
!   (condition-case nil
!       (recentf-apply-filename-handlers (expand-file-name name))
!     (error name)))

   (defun recentf-include-p (filename)
     "Return non-nil if FILENAME should be included in the recent list.


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

Re: tramp+recentf: persistent errors due to expand-file-name

[Michael Albinus]

David Reitter <david.reitter@gmail.com> writes:

> I've been playing around with tramp, which does not recognize a
> location such as /dr@localhost:/tmp/123123//bar  as a local /bar file.
>
> Worse than that, the error that ensues (in combination with recentf-
> mode) prevents users from saving or openeing other files in the future:
>
> Debugger entered--Lisp error: (error "Not a tramp file name: /qweqwe")
>   signal(error ("Not a tramp file name: /qweqwe"))
>   ad-Orig-error("Not a tramp file name: %s" "/qweqwe")
>   apply(ad-Orig-error ("Not a tramp file name: %s" "/qweqwe"))
>   error("Not a tramp file name: %s" "/qweqwe")
>   tramp-dissect-file-name("/qweqwe")
>   tramp-file-name-handler(expand-file-name "/dr@localhost:/tmp/
> 123123//qweqwe" nil)
>   expand-file-name("/dr@localhost:/tmp/123123//qweqwe")
>   recentf-expand-file-name("/dr@localhost:/tmp/123123//qweqwe")
>   recentf-track-opened-file()
>   run-hook-with-args-until-success(recentf-track-opened-file)
>   basic-save-buffer()
>   save-buffer()
>   mac-key-save-file()
>   call-interactively(mac-key-save-file)

I cannot reproduce it here:

(recentf-expand-file-name "/dr@localhost:/tmp/123123//qweqwe")
  => "/scp:dr@localhost:/tmp/123123/qweqwe"

(expand-file-name "/dr@localhost:/tmp/123123//qweqwe")
  => "/scp:dr@localhost:/tmp/123123/qweqwe"

(expand-file-name "/tmp/123123//qweqwe")
  => "/tmp/123123/qweqwe"

Btw, where does the "//" comes from?

> The patch below fixes this - comments appreciated.
>
> The alternative would be to fix tramp so it doesn't throw an error
> when expand-file-name is called, but this might prevent it from giving
> proper feedback to the user.
>
> *** lisp/recentf.el	06 Apr 2008 13:51:37 +0200	1.56.2.2
> --- lisp/recentf.el	10 Aug 2008 19:04:41 +0200	
> ***************
> *** 394,400 ****
>     "Convert file NAME to absolute, and canonicalize it.
>   NAME is first passed to the function `expand-file-name', then to
>   `recentf-filename-handlers' to post process it."
> !   (recentf-apply-filename-handlers (expand-file-name name)))
>
>   (defun recentf-include-p (filename)
>     "Return non-nil if FILENAME should be included in the recent list.
> --- 394,402 ----
>     "Convert file NAME to absolute, and canonicalize it.
>   NAME is first passed to the function `expand-file-name', then to
>   `recentf-filename-handlers' to post process it."
> !   (condition-case nil
> !       (recentf-apply-filename-handlers (expand-file-name name))
> !     (error name)))
>
>   (defun recentf-include-p (filename)
>     "Return non-nil if FILENAME should be included in the recent list.

This leaves `name' in an unexpanded state, which might be not
desirable. I would prefer to find the reason, why Tramp returns it the
way you have shown (and I cannot reproduce).

Could you, please, provide me a recipe for reproducing, w/o mac
specifics?

Best regards, Michael.

Re: tramp+recentf: persistent errors due to expand-file-name

[David Reitter]

[-- Attachment #1: Type: text/plain, Size: 1255 bytes --]

On 11 Aug 2008, at 21:18, Michael Albinus wrote:
>
> I cannot reproduce it here:

Me neither, not with an Emacs -Q (22 or 23) and such a minimal  
example.  Sorry!

>
> This leaves `name' in an unexpanded state, which might be not
> desirable.

Better than throwing a signal that is not handled, preventing the user  
from saving a file.

> I would prefer to find the reason, why Tramp returns it the
> way you have shown (and I cannot reproduce).

Me to.
I looked into this some more and I now think that there is something  
more complex going on that leads to the malformed URL ending up in the  
recentf-list.  This could be because I have code that synchronizes  
`file-name-history' and `recentf-list', and in my Emacs 22, `file-name- 
history' contains malformed tramp locations, while this doesn't seem  
to be the case in 23.  Also, it appears that the // is handled  
correctly in 23 (but wasn't in 22).  So, the problem might not occur  
in 23 anyways.  The particular chain of events may not be worth our  
time.

Generally, I do think it is prudent to guard against errors being  
signaled when running code from hooks so that users don't end up in a  
(to them) non-recoverable situation.  You'll know best where the right  
place is...

- D

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

Re: tramp+recentf: persistent errors due to expand-file-name

[Michael Albinus]

David Reitter <david.reitter@gmail.com> writes:

> Generally, I do think it is prudent to guard against errors being
> signaled when running code from hooks so that users don't end up in a
> (to them) non-recoverable situation.  You'll know best where the right
> place is...

Generally: yes.

But Tramp does not know that it runs from a hook, so it might be
better to protect the code which evaluates the hook.

> - D

Best regards, Michael.

Re: tramp+recentf: persistent errors due to expand-file-name

[Lennart Borgman]

[-- Attachment #1: Type: text/plain, Size: 761 bytes --]

On Tue, Aug 12, 2008 at 2:57 PM, Michael Albinus <michael.albinus@gmx.de>wrote:

> David Reitter <david.reitter@gmail.com> writes:
>
> > Generally, I do think it is prudent to guard against errors being
> > signaled when running code from hooks so that users don't end up in a
> > (to them) non-recoverable situation.  You'll know best where the right
> > place is...
>
> Generally: yes.
>
> But Tramp does not know that it runs from a hook, so it might be
> better to protect the code which evaluates the hook.
>
> > - D
>
> Best regards, Michael.
>
>
I agree, but there is a problem with this in Emas. If you protect the code
with condition-case it makes it very hard to find out where the problem was.
It would be good if someone find a way to enhance this.

[-- Attachment #2: Type: text/html, Size: 1220 bytes --]

Re: tramp+recentf: persistent errors due to expand-file-name

[David Reitter]

[-- Attachment #1: Type: text/plain, Size: 615 bytes --]

On 12 Aug 2008, at 15:04, Lennart Borgman wrote:
>
> I agree, but there is a problem with this in Emas. If you protect  
> the code with condition-case it makes it very hard to find out where  
> the problem was. It would be good if someone find a way to enhance  
> this.

A condition-case in `run-hooks' could output a warning to *Messages*,  
but catch the error.
One can still run the debugger when such an error occurs.

While it's important for us to find bugs, users obviously don't want  
to be prevented from running a command just because an extension (and  
that's what the hooks mostly do) fails to run.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

Re: tramp+recentf: persistent errors due to expand-file-name

[Lennart Borgman (gmail)]

David Reitter wrote:
> On 12 Aug 2008, at 15:04, Lennart Borgman wrote:
>>
>> I agree, but there is a problem with this in Emas. If you protect the 
>> code with condition-case it makes it very hard to find out where the 
>> problem was. It would be good if someone find a way to enhance this.
> 
> A condition-case in `run-hooks' could output a warning to *Messages*, 
> but catch the error.
> One can still run the debugger when such an error occurs.


I use that approach, but it is not very simple.

Re: tramp+recentf: persistent errors due to expand-file-name

[Stefan Monnier]

>>> I agree, but there is a problem with this in Emas. If you protect the
>>> code with condition-case it makes it very hard to find out where the
>>> problem was. It would be good if someone find a way to enhance this.
>> 
>> A condition-case in `run-hooks' could output a warning to *Messages*, but
>> catch the error.
>> One can still run the debugger when such an error occurs.


> I use that approach, but it is not very simple.

`with-demoted-errors'


        Stefan

Re: tramp+recentf: persistent errors due to expand-file-name

[Lennart Borgman (gmail)]

Stefan Monnier wrote:
>>>> I agree, but there is a problem with this in Emas. If you protect the
>>>> code with condition-case it makes it very hard to find out where the
>>>> problem was. It would be good if someone find a way to enhance this.
>>> A condition-case in `run-hooks' could output a warning to *Messages*, but
>>> catch the error.
>>> One can still run the debugger when such an error occurs.
> 
> 
>> I use that approach, but it is not very simple.
> 
> `with-demoted-errors'


condition-case-no-debug?