(tramp-handleverify-visited-file-modtime

(tramp-handleverify-visited-file-modtime

[Luc Teirlinck]

There is is another apparent problem with
`tramp-handleverify-visited-file-modtime' that has nothing to do with
the proposed change in `visited-file-modtime':

	     ;; If file does not exist, say it is not modified.
		(t nil)))))))

If the file is not modified,
`tramp-handle-verify-visited-file-modtime' should return t, not nil.
Moreover, replacing (t nil) with (t t) is not sufficient, because an
error is going to be thrown earlier if the buffer is not visiting a
file;

*** Welcome to IELM ***  Type (describe-mode) for help.
ELISP> (verify-visited-file-modtime (get-buffer "*scratch*"))
t
ELISP> (load "tramp")
t
ELISP> (tramp-handle-verify-visited-file-modtime (get-buffer "*scratch*"))
*** Eval error ***  Wrong type argument: stringp, nil
ELISP> 

Because `tramp-handle-verify-visited-file-modtime' is supposed to
emulate `verify-visited-file-modtime' I believe it should return t,
like `visited-visited-file-modtime' does.

So what about the following patch?

===File ~/tramp-diff-4======================================
*** tramp.el	26 Jun 2004 16:46:18 -0500	1.46
--- tramp.el	11 Jul 2004 11:22:58 -0500	
***************
*** 2334,2363 ****
    "Like `verify-visited-file-modtime' for tramp files."
    (with-current-buffer buf
      (let ((f (buffer-file-name)))
!       (with-parsed-tramp-file-name f nil
! 	(let* ((attr (file-attributes f))
! 	       (modtime (nth 5 attr)))
! 	  (cond ((and attr (not (equal modtime '(0 0))))
! 		 ;; Why does `file-attributes' return a list (HIGH
! 		 ;; LOW), but `visited-file-modtime' returns a cons
! 		 ;; (HIGH . LOW)?
! 		 (let ((mt (visited-file-modtime)))
! 		   (< (abs (tramp-time-diff
! 			    modtime (list (car mt) (cdr mt)))) 2)))
! 		(attr
! 		 (save-excursion
! 		   (tramp-send-command
! 		    multi-method method user host
! 		    (format "%s -ild %s"
! 			    (tramp-get-ls-command multi-method method
! 						  user host)
! 			    (tramp-shell-quote-argument localname)))
! 		   (tramp-wait-for-output)
! 		   (setq attr (buffer-substring
! 			       (point) (progn (end-of-line) (point)))))
! 		 (equal tramp-buffer-file-attributes attr))
! 		;; If file does not exist, say it is not modified.
! 		(t nil)))))))
  
  (defadvice clear-visited-file-modtime (after tramp activate)
    "Set `tramp-buffer-file-attributes' back to nil.
--- 2334,2370 ----
    "Like `verify-visited-file-modtime' for tramp files."
    (with-current-buffer buf
      (let ((f (buffer-file-name)))
!       (if (not f)
! 	  t
! 	(with-parsed-tramp-file-name f nil
! 	  (let* ((attr (file-attributes f))
! 		 (modtime (nth 5 attr)))
! 	    (cond ((eq (visited-file-modtime) 0))
! 		  ((and attr (not (equal modtime '(0 0))))
! 		   (let ((mt (visited-file-modtime)))
! 		     (< (abs (tramp-time-diff
! 			      modtime
! 			      ;; For compatibility, deal with both the old
! 			      ;; (HIGH . LOW) and the new (HIGH LOW)
! 			      ;; return values of `visited-file-modtime'.
! 			      (if (and (consp mt) (atom (cdr mt)))
! 				  (list (car mt) (cdr mt))
! 				mt)))
! 			2)))
! 		  (attr
! 		   (save-excursion
! 		     (tramp-send-command
! 		      multi-method method user host
! 		      (format "%s -ild %s"
! 			      (tramp-get-ls-command multi-method method
! 						    user host)
! 			      (tramp-shell-quote-argument localname)))
! 		     (tramp-wait-for-output)
! 		     (setq attr (buffer-substring
! 				 (point) (progn (end-of-line) (point)))))
! 		   (equal tramp-buffer-file-attributes attr))
! 		  ;; If file does not exist, say it is not modified.
! 		  (t t))))))))
  
  (defadvice clear-visited-file-modtime (after tramp activate)
    "Set `tramp-buffer-file-attributes' back to nil.
============================================================

Re: (tramp-handleverify-visited-file-modtime

[Luc Teirlinck]

>From my earlier message:

   Because `tramp-handle-verify-visited-file-modtime' is supposed to
   emulate `verify-visited-file-modtime' I believe it should return t,
   like `visited-visited-file-modtime' does.

like `verify-visited-file-modtime' does.

   ! 		  (t t))))))))

can be simplified to (t)))))))).

Sincerely,

Luc.

Re: (tramp-handleverify-visited-file-modtime

[Luc Teirlinck]

>From my previous message:

   Moreover, replacing (t nil) with (t t) is not sufficient, because an
   error is going to be thrown earlier if the buffer is not visiting a
   file;

Actually, it might be sufficient, because
`tramp-handle-verify-visited-file-modtime' probably never will get
called if the buffer is not visiting a file.  Are we sure of this?  If
so, what about my earlier patch with just the added change of
replacing the last clause (t nil) with (t)?

Sincerely,

Luc.

Re: (tramp-handleverify-visited-file-modtime

[Luc Teirlinck]

>From my earlier message:

   Actually, it might be sufficient, because
   `tramp-handle-verify-visited-file-modtime' probably never will get
   called if the buffer is not visiting a file.  Are we sure of this?

Apparently not, because `tramp-handle-set-visited-file-modtime'
worries about buffers not visiting files.  So what about staying with
my latest patch (tramp-diff-4)?

Sincerely,

Luc.

Re: (tramp-handleverify-visited-file-modtime

[Michael Albinus]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Because `tramp-handle-verify-visited-file-modtime' is supposed to
> emulate `verify-visited-file-modtime' I believe it should return t,
> like `visited-visited-file-modtime' does.
>
> So what about the following patch?

I've committed a looks-like-this patch to Tramp CVS.

Best regards, Michael.

Re: (tramp-handleverify-visited-file-modtime

[Luc Teirlinck]

Michael Albinus wrote:

   Luc Teirlinck <teirllm@dms.auburn.edu> writes:

   > Because `tramp-handle-verify-visited-file-modtime' is supposed to
   > emulate `verify-visited-file-modtime' I believe it should return t,
   > like `visited-visited-file-modtime' does.
   >
   > So what about the following patch?

   I've committed a looks-like-this patch to Tramp CVS.

   Best regards, Michael.

Actually, I recommend the following patch to the present Tramp CVS
version (not the Emacs CVS version) of tramp.el.  The reason is that
we want to return t for a file that does not exist according to
`file-attributes' _and_ for which (visited-file-modtime) returns -1,
but nil if `file-attributes' returns nil and `visited-file-modtime'
returns a "real" modtime (neither 0 nor -1), meaning the file _did_
exist the last time we checked, but has been deleted in the meantime.

To be completely consistent with `verify-visited-file-modtime' we
should also return nil if the file system could not handle the file
name because it was too long (never happens on the GNU system) or
because there were too many symbolic links to follow.  I do not know
whether this case is important enough to worry about.  (The patch
below does not.)  How were we even able to visit the file in the first
place in such a situation?


===File ~/tramp-CVS-diff====================================
*** tramp.el	24 Jul 2004 14:12:22 -0500	2.381
--- tramp.el	24 Jul 2004 19:34:27 -0500	
***************
*** 2342,2360 ****
        (let ((f (buffer-file-name)))
  	(with-parsed-tramp-file-name f nil
  	  (let* ((attr (file-attributes f))
! 		 (modtime (nth 5 attr)))
  	    
   	    (cond ((and attr (not (equal modtime '(0 0))))
! 		   (let ((mt (visited-file-modtime)))
! 		     (< (abs (tramp-time-diff
! 			      modtime
! 			      ;; For compatibility, deal with both the old
! 			      ;; (HIGH . LOW) and the new (HIGH LOW)
! 			      ;; return values of `visited-file-modtime'.
! 			      (if (atom (cdr mt))
! 				  (list (car mt) (cdr mt))
! 				mt)))
! 			2)))
  		  ;; modtime has the don't know value.
  		  (attr
  		   (save-excursion
--- 2342,2360 ----
        (let ((f (buffer-file-name)))
  	(with-parsed-tramp-file-name f nil
  	  (let* ((attr (file-attributes f))
! 		 (modtime (nth 5 attr))
! 		 (mt (visited-file-modtime)))
  	    
   	    (cond ((and attr (not (equal modtime '(0 0))))
! 		   (< (abs (tramp-time-diff
! 			    modtime
! 			    ;; For compatibility, deal with both the old
! 			    ;; (HIGH . LOW) and the new (HIGH LOW)
! 			    ;; return values of `visited-file-modtime'.
! 			    (if (atom (cdr mt))
! 				(list (car mt) (cdr mt))
! 			      mt)))
! 		      2))
  		  ;; modtime has the don't know value.
  		  (attr
  		   (save-excursion
***************
*** 2368,2375 ****
  		     (setq attr (buffer-substring
  				 (point) (progn (end-of-line) (point)))))
  		   (equal tramp-buffer-file-attributes attr))
! 		  ;; If file does not exist, say it is not modified.
! 		  (t t))))))))
  
  (defadvice clear-visited-file-modtime (after tramp activate)
    "Set `tramp-buffer-file-attributes' back to nil.
--- 2368,2376 ----
  		     (setq attr (buffer-substring
  				 (point) (progn (end-of-line) (point)))))
  		   (equal tramp-buffer-file-attributes attr))
! 		  ;; If file does not exist, say it is not modified
! 		  ;; if and only if that agrees with the buffer's record.
! 		  (t (equal mt '(-1 65535))))))))))
  
  (defadvice clear-visited-file-modtime (after tramp activate)
    "Set `tramp-buffer-file-attributes' back to nil.
============================================================

Re: (tramp-handleverify-visited-file-modtime

[Michael Albinus]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Actually, I recommend the following patch to the present Tramp CVS
> version (not the Emacs CVS version) of tramp.el.  The reason is that
> we want to return t for a file that does not exist according to
> `file-attributes' _and_ for which (visited-file-modtime) returns -1,
> but nil if `file-attributes' returns nil and `visited-file-modtime'
> returns a "real" modtime (neither 0 nor -1), meaning the file _did_
> exist the last time we checked, but has been deleted in the meantime.

Sounds OK to me. I've committed the patch to Tramp CVS. Please note
that it is visible in the branch "branch-2-0-stable" only, because
I'll commit later a bigger update to the trunk which is not intended for
Emacs 21.4 (but your patch will be included there, too).

> To be completely consistent with `verify-visited-file-modtime' we
> should also return nil if the file system could not handle the file
> name because it was too long (never happens on the GNU system) or
> because there were too many symbolic links to follow.  I do not know
> whether this case is important enough to worry about.  (The patch
> below does not.)  How were we even able to visit the file in the first
> place in such a situation?

Maybe we can ignore this situation for the time being. Tramp is based
on external programs, which should report an error these cases. Tramp
won't distinguish the reason of such errors.

Best regards, Michael.