time values

time values

[Luc Teirlinck]

At Richard's request, I plan to soon change the return values of
`visited-file-modtime' and `calendar-time-from-absolute' to the form
(HIGH LOW) instead of (HIGH . LOW).  Grepping shows that these are the
last two functions that still return time values in this obsolete
form.  The return value of `visited-file-modtime' is also inconsistent
with that of `file-attributes'.

The change to `visited-file-modtime' requires supporting changes in
dired.el and tramp.el.   I do not know how tramp.el has to be handled
since there is also a standalone version.

I do not believe that changes in `lisp/gnus/nnfolder.el' or
`lisp/fast-lock.el' are needed, but I use neither, so I do not feel
100% confident about that.  Anyway, any problems I missed should be
easy to correct by simple changes like replacing cdr by cadr or the
like.

Diffs:
 
===File ~/fileio.c-diff=====================================
*** fileio.c	04 Jul 2004 21:04:39 -0500	1.506
--- fileio.c	10 Jul 2004 09:18:18 -0500	
***************
*** 5628,5640 ****
  DEFUN ("visited-file-modtime", Fvisited_file_modtime,
         Svisited_file_modtime, 0, 0, 0,
         doc: /* Return the current buffer's recorded visited file modification time.
! The value is a list of the form (HIGH . LOW), like the time values
  that `file-attributes' returns.  If the current buffer has no recorded
  file modification time, this function returns 0.
  See Info node `(elisp)Modification Time' for more details.  */)
       ()
  {
!   return long_to_cons ((unsigned long) current_buffer->modtime);
  }
  
  DEFUN ("set-visited-file-modtime", Fset_visited_file_modtime,
--- 5628,5644 ----
  DEFUN ("visited-file-modtime", Fvisited_file_modtime,
         Svisited_file_modtime, 0, 0, 0,
         doc: /* Return the current buffer's recorded visited file modification time.
! The value is a list of the form (HIGH LOW), like the time values
  that `file-attributes' returns.  If the current buffer has no recorded
  file modification time, this function returns 0.
  See Info node `(elisp)Modification Time' for more details.  */)
       ()
  {
!   Lisp_Object tcons;
!   tcons = long_to_cons ((unsigned long) current_buffer->modtime);
!   if (CONSP (tcons))
!     return list2 (XCAR (tcons), XCDR (tcons));
!   return tcons;
  }
  
  DEFUN ("set-visited-file-modtime", Fset_visited_file_modtime,
============================================================

===File ~/cal-dst.el-diff===================================
*** cal-dst.el	02 Sep 2003 07:40:04 -0500	1.20
--- cal-dst.el	09 Jul 2004 20:37:09 -0500	
***************
*** 70,83 ****
  (defun calendar-time-from-absolute (abs-date s)
    "Time of absolute date ABS-DATE, S seconds after midnight.
  
! Returns the pair (HIGH . LOW) where HIGH and LOW are the high and low
  16 bits, respectively, of the number of seconds 1970-01-01 00:00:00 UTC,
  ignoring leap seconds, that is the equivalent moment to S seconds after
  midnight UTC on absolute date ABS-DATE."
    (let* ((a (- abs-date calendar-system-time-basis))
           (u (+ (* 163 (mod a 512)) (floor s 128))))
      ;; Overflow is a terrible thing!
!     (cons
       ;; floor((60*60*24*a + s) / 2^16)
       (+ a (* 163 (floor a 512)) (floor u 512))
       ;; (60*60*24*a + s) mod 2^16
--- 70,83 ----
  (defun calendar-time-from-absolute (abs-date s)
    "Time of absolute date ABS-DATE, S seconds after midnight.
  
! Returns the list (HIGH LOW) where HIGH and LOW are the high and low
  16 bits, respectively, of the number of seconds 1970-01-01 00:00:00 UTC,
  ignoring leap seconds, that is the equivalent moment to S seconds after
  midnight UTC on absolute date ABS-DATE."
    (let* ((a (- abs-date calendar-system-time-basis))
           (u (+ (* 163 (mod a 512)) (floor s 128))))
      ;; Overflow is a terrible thing!
!     (list
       ;; floor((60*60*24*a + s) / 2^16)
       (+ a (* 163 (floor a 512)) (floor u 512))
       ;; (60*60*24*a + s) mod 2^16
============================================================

===File ~/dired.el-diff=====================================
*** dired.el	11 Jun 2004 16:28:02 -0500	1.291
--- dired.el	09 Jul 2004 22:00:40 -0500	
***************
*** 620,627 ****
  	     (modtime (visited-file-modtime)))
  	 (or (eq modtime 0)
  	     (not (eq (car attributes) t))
! 	     (and (= (car (nth 5 attributes)) (car modtime))
! 		  (= (nth 1 (nth 5 attributes)) (cdr modtime)))))))
  
  (defun dired-buffer-stale-p (&optional noconfirm)
    "Return non-nil if current dired buffer needs updating.
--- 620,626 ----
  	     (modtime (visited-file-modtime)))
  	 (or (eq modtime 0)
  	     (not (eq (car attributes) t))
! 	     (equal (nth 5 attributes) modtime)))))
  
  (defun dired-buffer-stale-p (&optional noconfirm)
    "Return non-nil if current dired buffer needs updating.
============================================================

===File ~/tramp.el-diff=====================================
*** tramp.el	26 Jun 2004 16:46:18 -0500	1.46
--- tramp.el	10 Jul 2004 15:10:30 -0500	
***************
*** 2338,2349 ****
  	(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
--- 2338,2346 ----
  	(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 mt)) 2)))
  		(attr
  		 (save-excursion
  		   (tramp-send-command
============================================================

===File ~/buffers.texi-diff=================================
*** buffers.texi	23 Jun 2004 10:21:06 -0500	1.37
--- buffers.texi	10 Jul 2004 18:48:30 -0500	
***************
*** 625,633 ****
  @c Emacs 19 feature
  @defun visited-file-modtime
  This function returns the current buffer's recorded last file
! modification time, as a list of the form @code{(@var{high} .
! @var{low})}.  (This is the same format that @code{file-attributes}
! uses to return time values; see @ref{File Attributes}.)
  
  The function returns zero if the buffer has no recorded last
  modification time, which can happen, for instance, if the record has
--- 625,633 ----
  @c Emacs 19 feature
  @defun visited-file-modtime
  This function returns the current buffer's recorded last file
! modification time, as a list of the form @code{(@var{high} @var{low})}.
! (This is the same format that @code{file-attributes} uses to return
! time values; see @ref{File Attributes}.)
  
  The function returns zero if the buffer has no recorded last
  modification time, which can happen, for instance, if the record has
============================================================

Re: time values

[Kai Grossjohann]

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

> The change to `visited-file-modtime' requires supporting changes in
> dired.el and tramp.el.   I do not know how tramp.el has to be handled
> since there is also a standalone version.

It would be great if there was a way for Tramp to find out whether
the new-style or the old-style value should be returned.  Then a
single code base could be used to support Emacs 21.3 as well as 21.4.

Kai

Re: time values

[Andreas Schwab]

Kai Grossjohann <kai@emptydomain.de> writes:

> It would be great if there was a way for Tramp to find out whether
> the new-style or the old-style value should be returned.

Does tramp actually have to contruct any such value?  From a cursory look
it only has to process the return value of visited-file-modtime, which is
easy to handle both ways.  But probing the correct style is not difficult
either, you just have to call visited-file-modtime once in a dummy buffer
visiting any random file (which does not need to exist) and examine the
returned value.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: time values

[Alex Schroeder]

Kai Grossjohann <kai@emptydomain.de> writes:

> It would be great if there was a way for Tramp to find out whether
> the new-style or the old-style value should be returned.  Then a
> single code base could be used to support Emacs 21.3 as well as 21.4.

Maybe you can use (and (not (equal (visited-file-modtime) 0))
                            (consp (cdr (visited-file-modtime)))) ?

It returns nil when (visited-file-modtime) is 0 or a list of at least
two elements.

Alex.
-- 
.O.  http://www.emacswiki.org/alex/
..O  Schroeder's fourth law:
OOO  None of your friends and coworkers share your taste in music.

Re: time values

[Luc Teirlinck]

   It would be great if there was a way for Tramp to find out whether
   the new-style or the old-style value should be returned.  Then a
   single code base could be used to support Emacs 21.3 as well as 21.4.

Like Andreas already pointed out, it depends on whether you actually
have to construct time values that other functions are going to _believe
to be return values of `visited-file-modtime'_.  In as far as I know,
all functions that accept time value arguments already can handle
both conses and lists.  If you have to construct such values, what
about just using `emacs-version'?

Otherwise, I believe the following patch should do.

By the way, your current code seems to assume that
`visited-file-modtime' is not 0.  Are you sure of that?  Of course,
you know that (nth 5 attr) is not (0 0), but still.  My patch still
probably assumes that: can tramp-time-diff handle 0 as an argument?
Otherwise, it is easy to rewrite the patch to transform 0 into (0 0),
or maybe an integer N < 2**16 into (0 N), because, theoretically,
`visited-file-modtime' can return such integers.  (Even though probably
not in practice).  But maybe `tramp-time-diff' is the place to
actually do that.

===File ~/tramp-diff-2======================================
*** tramp.el	26 Jun 2004 16:46:18 -0500	1.46
--- tramp.el	11 Jul 2004 09:35:00 -0500	
***************
*** 2338,2349 ****
  	(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
--- 2338,2353 ----
  	(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 (and (consp mt) (atom (cdr mt)))
! 				(list (car mt) (cdr mt))
! 			      mt)))
! 			   2)))
  		(attr
  		 (save-excursion
  		   (tramp-send-command
============================================================

Re: time values

[Luc Teirlinck]

>From my earlier message:

   My patch still probably assumes that: can tramp-time-diff handle 0
   as an argument?

I should have checked this.  No, it does not.

Sincerely,

Luc.

Re: time values

[Luc Teirlinck]

>From my previous message:

   By the way, your current code seems to assume that
   `visited-file-modtime' is not 0.  Are you sure of that?  Of course,
   you know that (nth 5 attr) is not (0 0), but still.  My patch still
   probably assumes that: can tramp-time-diff handle 0 as an argument?
   Otherwise, it is easy to rewrite the patch to transform 0 into (0 0),
   or maybe an integer N < 2**16 into (0 N), because, theoretically,
   `visited-file-modtime' can return such integers.  (Even though probably
   not in practice).  But maybe `tramp-time-diff' is the place to
   actually do that.

I was confused there for a moment.  If `visited-file-modtime' returns
0, `verify-visited-file-modtime' and hence
`tramp-handle-verify-visited-file-modtime' should return t.
Hence, it has to be handled specially by 
`tramp-handle-verify-visited-file-modtime'.
So the only question is: 
Are you sure that the value can not be 0 at that point?
Your current code assumes it is not.

There probably is no need to worry about positive return values <
2**16, as these are not going to occur in practice.

Sincerely,

Luc.

Re: time values

[Luc Teirlinck]

I do not see why `visited-file-modtime could not be 0 at that point.
What about the following:

===File ~/tramp-diff-3======================================
*** tramp.el	26 Jun 2004 16:46:18 -0500	1.46
--- tramp.el	11 Jul 2004 10:43:12 -0500	
***************
*** 2337,2349 ****
        (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
--- 2337,2354 ----
        (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
============================================================

Re: time values

[Richard Stallman]

Thanks for working on this.  I see you came across an error
in the manual too.

Re: time values

[Richard Stallman]

    It would be great if there was a way for Tramp to find out whether
    the new-style or the old-style value should be returned.

How about (consp (cdr VALUE))?

Re: time values

[Luc Teirlinck]

>From my previous message:

   By the way, your current code seems to assume that
   `visited-file-modtime' is not 0.  Are you sure of that?

Yes.  I should have read the code of `verify-visited-file-modtime'
more carefully.  If `visited-file-modtime' returns 0, then
`verify-visited-file-modtime' does not even call the handler.

   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?

Yes, for the same reason.  So there was no reason to worry about that
either.

Well, at least there is no reason to worry about those two things
_unless_ `tramp-handle-verify-visited-file-modtime' could be called
from some place else than `verify-visited-file-modtime', which would
however surprise me.

There are still problems with the last `t' clause.  I will submit a
revised patch tomorrow.

Sincerely,

Luc.

Re: time values

[Luc Teirlinck]

I plan to commit the following minimal patch to tramp.el.  Problems
remain, which I will discuss later, but they are unrelated to the
`time valus' stuff.

===File ~/tramp-diff========================================
*** tramp.el	26 Jun 2004 16:46:18 -0500	1.46
--- tramp.el	12 Jul 2004 10:16:09 -0500	
***************
*** 2343,2349 ****
  		 ;; (HIGH . LOW)?
  		 (let ((mt (visited-file-modtime)))
  		   (< (abs (tramp-time-diff
! 			    modtime (list (car mt) (cdr mt)))) 2)))
  		(attr
  		 (save-excursion
  		   (tramp-send-command
--- 2343,2356 ----
  		 ;; (HIGH . LOW)?
  		 (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)))
  		(attr
  		 (save-excursion
  		   (tramp-send-command
============================================================

Re: time values

[Kai Grossjohann]

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

> I plan to commit the following minimal patch to tramp.el.  Problems
> remain, which I will discuss later, but they are unrelated to the
> `time valus' stuff.

Thanks, Luc.  It's a pity that I got distracted by heaps of work in
the middle of the discussion.  I've now merged your change with the
Tramp CVS.

Now on to grokking the other issues you mention in later messages.

Kai