>> Yes, we could check it in datestamp(), but ... >> >>> Initially, I thought this would be sufficient to show "-0000" in commit log >>> message. However, I found that the show_date function is used for "decoding"; >>> converting timestamp and tz to more readable format. Then I realize the >>> function won't distinguish between +0 and -0 as it only takes in a tz as >>> argument. As a result,... >> >> ... I would have imagined that you do not have to deal with all >> those complications if you don't hook this to such a low level of >> the call graph. That is why I wondered: >> ... > > Let me answer some of my puzzlement myself; that is, I would have > understood the change well if it were explained to me this way, and > if that explanation matched what the patches did ;-) Yes, I agree. > The topic has two major parts. > > The code that prepares the timestamp to be recorded for the current > user, who wants to record an anonymous timezone "-0000", is one (and > the easier) part. And this part could be done all inside > ident_default_date() without touching anything in date.c; when we > need to call datestamp(), we are getting the current time for the > current user, so we can mask the timezone. So for this part, there is no need to modify datestamp in dates.c. We could modify ident_default_date buf after datestamp to set the last 5 bytes to "-0000" using strcpy. > The other part is that we need to read the timestamp from existing > records, and if we choose to distinguish between timestamp in UTC > and timestamp with anonymous timezone, we'd need to devise a way to > encode the anonymous timezone differently. It is where the extra > bit that says "this bit does not usually mean anything but only when > the offset (which is a signed integer whose valid range is set to > between -2400 to +2400 by date.c::match_tz()) is zero, and this bit > is set, the zone is anonymous" comes in. Yes, that's correct. > Side note. I suspect the damage to the callchain can be > limited much narrower if we didn't add this bit throughout > the API. What if we instead pick a number outside the valid > range of offsets, say -10000, as a sentinel value and passed > that throughout the code when we want an anonymous zone? Good idea. > The functions in the callchain that care about the timezone > must understand how anonymous zone is encoded anyway, so to > them it's a matter of using an int plus one bit or using an > int that can have a special value. But other functions in > the callchain whose sole purpose (with respect to the > timezone information) is to pass it between their caller and > their callee as an opaque piece of data, using just a single > integer is much less error prone---the patch does not have > to touch them at all. That would be easier to follow and requires less changes as well. > Thanks. Thanks for the clarification. Now I think we have a much better understanding. I will try to do a better job describing patches next time.
