Re: Patch for timezone handling in cvs log

[Bart Robinson]

On 2004-4-30 Derek Robert Price <derek@ximbiot.com> wrote:
 > Bart Robinson wrote:
 > 
 > >Here is the new patch, which uses the MT thing and does all
 > >conversion on the client side.
 > 
 > 
 > This is a step in the right direction, but I still have a few problems
 > with it:
 > 
 >    1. I don't like the -z option or the new TZ variables.  Why not use
 >       the standard interface for these functions and avoid confusing
 >       the users.  Leave timzone handling to the localtime() & mktime()
 >       functions.  They know what to look for already.

I think I know what you're getting at.  How about no -z option,
and we always display in localtime.  If the user wants GMT
output, or any other timezone supported on their system, they
have to put it in their TZ env var, like

        $ TZ=GMT cvs log foo
or
        $ TZ=GMT-5:30 cvs log foo

I think that would be superior and simplify the code.

However, I would prefer we still print the offset so they know
that conversion has been done and they can save the output and
read it in another locale (like in a mail message) and know what
the time means.

 >    2. You should be able to mix cvs_output() lines with
 >       cvs_output_tagged() lines.  The extensive conversion you
 >       performed on log.c was unecessary.  You should have only needed
 >       to convert the lines containing the dates.

I agree I can get rid of some of that conversion but the bulk of
it is necessary.  All the stuff dealing with the printing of a
line like:

  date: 2004/04/05 10:52:16-0700;  author: lomew;  state: Exp;  lines: +2 -1

must use MT since the date is tagged.  The "version [locked by]"
and "branches" lines don't need it.  (So the diffs won't shrink
by more than about 10 lines, but I agree it is worth fixing.)

In more detail, you can't do something simple like

        cvs_output ("date: ")
        cvs_output_tagged ("date", datestr)
        cvs_output (";  author: ")
        cvs_output (the_author)
        cvs_output ("stuff...\n")
        
Because the M responses are line-based.  This turns into:

        MT date [datestr]
        M date: ;  author: [the_author] stuff...

since cvs_output buffers up to a newline.  More fundamentally,
if any part of a line, save for the beginning, is tagged, the
rest of the line must be tagged as well (usually with "text").

I could do something like

        MT date [datestr]
        M date: %s;  author...

But that would make the code *much* more complicated and make
backward compatibility more difficult.

Another option would be to do:

        MT date date: [datestr]
        M ;  author: [the_author] stuff...

But that means the code that parses the date MT tag would have
to account for the leading "date: ", which seems sloppy and
overly specific -- it would limit its use for this particular
case only.  If there were other contexts we wanted date
conversion, like history or stat, it would either get printed as
"date: [converted-date]" or we would have to add another MT tag,
like "baredate", both of which would require extensive
cvs_output_tagification to the respective code (since neither
use a date at the beginning of a line).

-- bart