[PATCH 1/2] gitweb: fix #patchNN anchors when path_info is enabled

[PATCH 1/2] gitweb: fix #patchNN anchors when path_info is enabled

[Kevin Cernekee]

My configuration is as follows:

$feature{'pathinfo'}{'default'} = [1];

<Location /gitweb>
        Options ExecCGI
        SetHandler cgi-script
</Location>

GITWEB_{JS,CSS,LOGO,...} all start with gitweb-static/

gitweb.cgi renamed to /var/www/html/gitweb

This gives me simple, easy-to-read URLs that look like:

http://HOST/gitweb/myproject.git/commitdiff/0faa4a6ef921d8a233f30d66f9a3e1b24e8ec906

The problem is that in this configuration, PATH_INFO is used to set the
base URL:

<base href="http://HOST/gitweb">

This breaks the "patch" anchor links seen on the commitdiff pages,
because they are computed relative to the base URL:

http://HOST/gitweb#patch1

My solution is to add an "anchor" parameter to href(), so that the full
path is included in the patchNN links.

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
---
 gitweb/gitweb.perl |   31 +++++++++++++++++++++++++------
 1 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1b9369d..3b6a90d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1199,6 +1199,7 @@ if (defined caller) {
 # -full => 0|1      - use absolute/full URL ($my_uri/$my_url as base)
 # -replay => 1      - start from a current view (replay with modifications)
 # -path_info => 0|1 - don't use/use path_info URL (if possible)
+# -anchor           - add #ANCHOR to end of URL
 sub href {
 	my %params = @_;
 	# default is to use -absolute url() i.e. $my_uri
@@ -1314,6 +1315,10 @@ sub href {
 	# final transformation: trailing spaces must be escaped (URI-encoded)
 	$href =~ s/(\s+)$/CGI::escape($1)/e;
 
+	if (defined($params{'anchor'})) {
+		$href .= "#".esc_param($params{'anchor'});
+	}
+
 	return $href;
 }
 
@@ -4334,8 +4339,10 @@ sub git_difftree_body {
 			if ($action eq 'commitdiff') {
 				# link to patch
 				$patchno++;
-				print "<td class=\"link\">" .
-				      $cgi->a({-href => "#patch$patchno"}, "patch") .
+				print $cgi->a({-href =>
+				      href(action=>"commitdiff",
+				      hash=>$hash, anchor=>"patch$patchno")},
+				      "patch") .
 				      " | " .
 				      "</td>\n";
 			}
@@ -4432,7 +4439,10 @@ sub git_difftree_body {
 			if ($action eq 'commitdiff') {
 				# link to patch
 				$patchno++;
-				print $cgi->a({-href => "#patch$patchno"}, "patch");
+				print $cgi->a({-href =>
+				      href(action=>"commitdiff",
+				      hash=>$hash, anchor=>"patch$patchno")},
+				      "patch");
 				print " | ";
 			}
 			print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
@@ -4452,7 +4462,10 @@ sub git_difftree_body {
 			if ($action eq 'commitdiff') {
 				# link to patch
 				$patchno++;
-				print $cgi->a({-href => "#patch$patchno"}, "patch");
+				print $cgi->a({-href =>
+				      href(action=>"commitdiff",
+				      hash=>$hash, anchor=>"patch$patchno")},
+				      "patch");
 				print " | ";
 			}
 			print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'from_id'},
@@ -4494,7 +4507,10 @@ sub git_difftree_body {
 			if ($action eq 'commitdiff') {
 				# link to patch
 				$patchno++;
-				print $cgi->a({-href => "#patch$patchno"}, "patch") .
+				print $cgi->a({-href =>
+				      href(action=>"commitdiff",
+				      hash=>$hash, anchor=>"patch$patchno")},
+				      "patch") .
 				      " | ";
 			} elsif ($diff->{'to_id'} ne $diff->{'from_id'}) {
 				# "commit" view and modified file (not onlu mode changed)
@@ -4539,7 +4555,10 @@ sub git_difftree_body {
 			if ($action eq 'commitdiff') {
 				# link to patch
 				$patchno++;
-				print $cgi->a({-href => "#patch$patchno"}, "patch") .
+				print $cgi->a({-href =>
+				      href(action=>"commitdiff",
+				      hash=>$hash, anchor=>"patch$patchno")},
+				      "patch") .
 				      " | ";
 			} elsif ($diff->{'to_id'} ne $diff->{'from_id'}) {
 				# "commit" view and modified file (not only pure rename or copy)
-- 
1.7.4.1

[PATCH 2/2] gitweb: introduce localtime feature

[Kevin Cernekee]

With this feature enabled, all timestamps are shown in the machine's
local timezone instead of GMT.

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
---
 gitweb/gitweb.perl |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3b6a90d..d171ad5 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -504,6 +504,12 @@ our %feature = (
 		'sub' => sub { feature_bool('remote_heads', @_) },
 		'override' => 0,
 		'default' => [0]},
+
+	# Use localtime rather than GMT for all timestamps.  Disabled
+	# by default.  Project specific override is not supported.
+	'localtime' => {
+		'override' => 0,
+		'default' => [0]},
 );
 
 sub gitweb_get_feature {
@@ -2927,6 +2933,12 @@ sub parse_date {
 	$date{'iso-tz'} = sprintf("%04d-%02d-%02d %02d:%02d:%02d %s",
 	                          1900+$year, $mon+1, $mday,
 	                          $hour, $min, $sec, $tz);
+
+	if (gitweb_check_feature('localtime')) {
+		$date{'rfc2822'}   = sprintf "%s, %d %s %4d %02d:%02d:%02d $tz",
+				     $days[$wday], $mday, $months[$mon],
+				     1900+$year, $hour ,$min, $sec;
+	}
 	return %date;
 }
 
@@ -3989,7 +4001,7 @@ sub git_print_authorship_rows {
 		      "</td></tr>\n" .
 		      "<tr>" .
 		      "<td></td><td> $wd{'rfc2822'}";
-		print_local_time(%wd);
+		print_local_time(%wd) if !gitweb_check_feature('localtime');
 		print "</td>" .
 		      "</tr>\n";
 	}
-- 
1.7.4.1

Re: [PATCH 1/2] gitweb: fix #patchNN anchors when path_info is enabled

[Jakub Narebski]

Kevin Cernekee <cernekee@gmail.com> writes:

> My configuration is as follows:

Very minor issue: Documentation/SubmittingPatches states the
following:

    - describe changes in imperative mood, e.g. "make xyzzy do frotz"
      instead of "[This patch] makes xyzzy do frotz" or "[I] changed
      xyzzy to do frotz", as if you are giving orders to the codebase
      to change its behaviour.

I think this also means trying to avoid "My configuration..." and "My
solution..." etc. in commit message.  But this is just a side issue,
not worth worrying over in my opinion; perhaps something to think
about in the future.

> $feature{'pathinfo'}{'default'} = [1];

[...]

> The problem is that in this configuration, PATH_INFO is used to set the
> base URL:
> 
> <base href="http://git.example.com/gitweb.cgi">
> 
> This breaks the "patch" anchor links seen on the commitdiff pages,
> because they are computed relative to the base URL:
> 
> http://git.example.com/gitweb.cgi#patch1

I think that the above configuration is enough to trigger bug /
errorneous behavior that you describe, isn't it?  It is better to try
to find minimal way to reproduce a bug when describing it.

I guess that
 
> <Location /gitweb>
>         Options ExecCGI
>         SetHandler cgi-script
> </Location>
> 
> GITWEB_{JS,CSS,LOGO,...} all start with gitweb-static/
> 
> gitweb.cgi renamed to /var/www/html/gitweb
> 
> This gives me simple, easy-to-read URLs that look like:
> 
> http://HOST/gitweb/myproject.git/commitdiff/0faa4a6ef921d8a233f30d66f9a3e1b24e8ec906
 
is not strictly necessary to trigger a bug.

> 
> My solution is to add an "anchor" parameter to href(), so that the full
> path is included in the patchNN links.

This is a very good idea.  Thank you very much for sending this patch,
and contributing to gitweb.

Its implemetation could be though improved a bit; see below.

> 
> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
> ---
>  gitweb/gitweb.perl |   31 +++++++++++++++++++++++++------
>  1 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 1b9369d..3b6a90d 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1199,6 +1199,7 @@ if (defined caller) {
>  # -full => 0|1      - use absolute/full URL ($my_uri/$my_url as base)
>  # -replay => 1      - start from a current view (replay with modifications)
>  # -path_info => 0|1 - don't use/use path_info URL (if possible)
> +# -anchor           - add #ANCHOR to end of URL

Shouldn't it be:

  +# -anchor => ANCHOR - add #ANCHOR to end of URL

>  sub href {
>  	my %params = @_;
>  	# default is to use -absolute url() i.e. $my_uri
> @@ -1314,6 +1315,10 @@ sub href {
>  	# final transformation: trailing spaces must be escaped (URI-encoded)
>  	$href =~ s/(\s+)$/CGI::escape($1)/e;
>  
> +	if (defined($params{'anchor'})) {
> +		$href .= "#".esc_param($params{'anchor'});
> +	}
> +
>  	return $href;
>  }

Here you have slight mismatch between description, which uses
'-anchor', and code, which uses 'anchor'.

>  
> @@ -4334,8 +4339,10 @@ sub git_difftree_body {
>  			if ($action eq 'commitdiff') {
>  				# link to patch
>  				$patchno++;
> -				print "<td class=\"link\">" .
> -				      $cgi->a({-href => "#patch$patchno"}, "patch") .
> +				print $cgi->a({-href =>
> +				      href(action=>"commitdiff",
> +				      hash=>$hash, anchor=>"patch$patchno")},
> +				      "patch") .

It would be better (less error prone) and easier to use '-replay'
option to href(), i.e. write

> +				print $cgi->a({-href => href(-replay=>1, -anchor=>"patch$patchno")},
> +				              "patch") .

or even make it so 'href(-anchor=>"ANCHOR")' implies '-replay => 1'.
The href() part of patch would then look something like this:

  @@ -1199,6 +1199,7 @@ if (defined caller) {
   # -full => 0|1      - use absolute/full URL ($my_uri/$my_url as base)
   # -replay => 1      - start from a current view (replay with modifications)
   # -path_info => 0|1 - don't use/use path_info URL (if possible)
  +# -anchor => ANCHOR - add #ANCHOR to end of URL, implies -replay if used alone
   sub href {
   	my %params = @_;
   	# default is to use -absolute url() i.e. $my_uri
  @@ -1310,6 +1310,7 @@ sub href {
  
  	$params{'project'} = $project unless exists $params{'project'};
  
 -	if ($params{-replay}) {
 +	if ($params{-replay} ||
 +	    ($params{-anchor} && keys %params == 1)) {
  		while (my ($name, $symbol) = each %cgi_param_mapping) {
  			if (!exists $params{$name}) {
  				$params{$name} = $input_params{$name};
  @@ -1314,6 +1315,10 @@ sub href {
   	# final transformation: trailing spaces must be escaped (URI-encoded)
   	$href =~ s/(\s+)$/CGI::escape($1)/e;
   
  +	if (defined($params{'anchor'})) {
  +		$href .= "#".esc_param($params{'anchor'});
  +	}
  +
   	return $href;
   }

Do you want to resend patch with those corrections yourself, or should
I do this?

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH 2/2] gitweb: introduce localtime feature

[Jakub Narebski]

Kevin Cernekee <cernekee@gmail.com> writes:

> With this feature enabled, all timestamps are shown in the machine's
> local timezone instead of GMT.

This does not describe why would one want such way of displaying
timestamps, and which views would be affected.

BTW. should it be timezone of web server (machine where gitweb is
run), or local time of author / committer / tagger as described in the
timezone part of git timestamp?
 
> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
> ---
>  gitweb/gitweb.perl |   14 +++++++++++++-
>  1 files changed, 13 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 3b6a90d..d171ad5 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -504,6 +504,12 @@ our %feature = (
>  		'sub' => sub { feature_bool('remote_heads', @_) },
>  		'override' => 0,
>  		'default' => [0]},
> +
> +	# Use localtime rather than GMT for all timestamps.  Disabled
> +	# by default.  Project specific override is not supported.
> +	'localtime' => {
> +		'override' => 0,
> +		'default' => [0]},

Why project specific override is not supported?  I think it might make
sense to enable this feature on project-by-project basis; some
projects might be dispersed geographically, some might not.

It is not as if this feature affect only non-project views, or doesn't
make sense on less that site-wide basis, like other nonoverridable
features.

>  );
>  
>  sub gitweb_get_feature {
> @@ -2927,6 +2933,12 @@ sub parse_date {
>  	$date{'iso-tz'} = sprintf("%04d-%02d-%02d %02d:%02d:%02d %s",
>  	                          1900+$year, $mon+1, $mday,
>  	                          $hour, $min, $sec, $tz);
> +
> +	if (gitweb_check_feature('localtime')) {
> +		$date{'rfc2822'}   = sprintf "%s, %d %s %4d %02d:%02d:%02d $tz",
> +				     $days[$wday], $mday, $months[$mon],
> +				     1900+$year, $hour ,$min, $sec;
> +	}

Is it still an RFC 2822 conformant date?  If it is not, then above
change is invalid, and we have to implement this feature in different
way.

>  	return %date;
>  }
>  
> @@ -3989,7 +4001,7 @@ sub git_print_authorship_rows {
>  		      "</td></tr>\n" .
>  		      "<tr>" .
>  		      "<td></td><td> $wd{'rfc2822'}";
> -		print_local_time(%wd);
> +		print_local_time(%wd) if !gitweb_check_feature('localtime');

Hmmm... I wonder if it wouldn't be better to print both times (perhaps
reversed) in this case...

>  		print "</td>" .
>  		      "</tr>\n";
>  	}
> -- 
> 1.7.4.1
> 

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH 2/2] gitweb: introduce localtime feature

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Kevin Cernekee <cernekee@gmail.com> writes:
>
>> With this feature enabled, all timestamps are shown in the machine's
>> local timezone instead of GMT.
>
> This does not describe why would one want such way of displaying
> timestamps, and which views would be affected.
>
> BTW. should it be timezone of web server (machine where gitweb is
> run), or local time of author / committer / tagger as described in the
> timezone part of git timestamp?

Both are sensible choices; another plausible choice that normal people
would want to see is to use the zone of whoever is requesting the page
(note that this would affect the caching layer).

>> +	# Use localtime rather than GMT for all timestamps.  Disabled
>> +	# by default.  Project specific override is not supported.
>> +	'localtime' => {
>> +		'override' => 0,
>> +		'default' => [0]},
>
> Why project specific override is not supported?

Good question.

Re: [PATCH 1/2] gitweb: fix #patchNN anchors when path_info is enabled

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> It would be better (less error prone) and easier to use '-replay'
> option to href(), i.e. write
>
>> +				print $cgi->a({-href => href(-replay=>1, -anchor=>"patch$patchno")},
>> +				              "patch") .
>
> or even make it so 'href(-anchor=>"ANCHOR")' implies '-replay => 1'.

I don't see why "or even" is an improvement, given the following
implementation.

>   @@ -1310,6 +1310,7 @@ sub href {
>   
>   	$params{'project'} = $project unless exists $params{'project'};
>   
>  -	if ($params{-replay}) {
>  +	if ($params{-replay} ||
>  +	    ($params{-anchor} && keys %params == 1)) {
>   		while (my ($name, $symbol) = each %cgi_param_mapping) {
>   			if (!exists $params{$name}) {
>   				$params{$name} = $input_params{$name};

I don't share your intuition that "anchor" is so special that this part
will not grow into a long chain of "(I want an implicit replay too) ||".

Implicitly enabling it in certain obvious cases is perfectly fine, but the
logic to do so should be in a separate place.  Wouldn't it better to have
a separate code that sets 'replay' under this and that condition so that
other people can later add to it at the very beginning of "sub href"?

Unless we do so, if there are other places that need to change the
behaviour based on 'replay', they need to duplicate the "implicit" logic.

Re: [PATCH 1/2] gitweb: fix #patchNN anchors when path_info is enabled

[Jakub Narebski]

On Thu, 17 Mar 2011, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> It would be better (less error prone) and easier to use '-replay'
>> option to href(), i.e. write
>>
>>> +				print $cgi->a({-href => href(-replay=>1, -anchor=>"patch$patchno")},
>>> +				              "patch") .
>>
>> or even make it so 'href(-anchor=>"ANCHOR")' implies '-replay => 1'.
> 
> I don't see why "or even" is an improvement, given the following
> implementation.

Well, 

  -href => href(-anchor=>"patch$patchno")

is closer in spirit to

  -href => "#patch$patchno"

that is currently used, and does not work with path_info.
 
> >   @@ -1310,6 +1310,7 @@ sub href {
> >   
> >   	$params{'project'} = $project unless exists $params{'project'};
> >   
> >  -	if ($params{-replay}) {
> >  +	if ($params{-replay} ||
> >  +	    ($params{-anchor} && keys %params == 1)) {
> >   		while (my ($name, $symbol) = each %cgi_param_mapping) {
> >   			if (!exists $params{$name}) {
> >   				$params{$name} = $input_params{$name};
> 
> I don't share your intuition that "anchor" is so special that this part
> will not grow into a long chain of "(I want an implicit replay too) ||".
> 
> Implicitly enabling it in certain obvious cases is perfectly fine, but the
> logic to do so should be in a separate place.  Wouldn't it better to have
> a separate code that sets 'replay' under this and that condition so that
> other people can later add to it at the very beginning of "sub href"?
> 
> Unless we do so, if there are other places that need to change the
> behaviour based on 'replay', they need to duplicate the "implicit" logic.

So you would prefer to have something like this:

 	# implicit -replay
 	if (keys %params == 1 && $params{-anchor}) {
 		# href(-anchor=>"ANCHOR") works like "#ANCHOR", 
 		# correctly if base href is set (for path_info URLs)
 		$params{-replay} = 1;
 	}

set above 'if ($params{-replay}) {'?

-- 
Jakub Narebski
Poland

Re: [PATCH 2/2] gitweb: introduce localtime feature

[Kevin Cernekee]

On Thu, Mar 17, 2011 at 4:01 AM, Jakub Narebski <jnareb@gmail.com> wrote:

Jakub,

Thanks for all of your constructive feedback.  I have taken your
suggestions into account and posted an updated series of patches.

> This does not describe why would one want such way of displaying
> timestamps, and which views would be affected.
>
> BTW. should it be timezone of web server (machine where gitweb is
> run), or local time of author / committer / tagger as described in the
> timezone part of git timestamp?

The case I am currently trying to improve is the one in which all
developers are at a single site.

In the open source world it is common to have developers scattered all
over the globe, so some of them will inevitably have to perform
timezone conversions.

But Git is becoming a popular tool in the private sector and it is
common to have most/all contributors based in a single office.  In the
latter case, it is helpful to display the local timezone instead of
GMT.  This also helps make the data more readable by program managers
and other non-developers who have an interest in tracking the project.

> Why project specific override is not supported?  I think it might make
> sense to enable this feature on project-by-project basis; some
> projects might be dispersed geographically, some might not.

Mostly ease of testing.  I did not need it for any of my projects.

It turned out to be a simple change, and it is in v2.  The cases I tested were:

default 0
default 1
override 1, project unset
override 1, project 0
override 1, project 1

> Is it still an RFC 2822 conformant date?  If it is not, then above
> change is invalid, and we have to implement this feature in different
> way.

I believe it is still valid.

Original date: Thu, 17 Mar 2011 02:11:05 +0000
New date: Wed, 16 Mar 2011 19:11:05 -0700
Sample date from RFC 2822 Appendix A: Fri, 21 Nov 1997 09:55:06 -0600

> Hmmm... I wonder if it wouldn't be better to print both times (perhaps
> reversed) in this case...

I have submitted a third patch which does this.

Re: [PATCH 1/2] gitweb: fix #patchNN anchors when path_info is enabled

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

>>> or even make it so 'href(-anchor=>"ANCHOR")' implies '-replay => 1'.
>> 
>> I don't see why "or even" is an improvement, given the following
>> implementation.
>
> Well, 
>
>   -href => href(-anchor=>"patch$patchno")
>
> is closer in spirit to
>
>   -href => "#patch$patchno"
>
> that is currently used, and does not work with path_info.

I questioned "implies '-replay => 1'" part, not the use of href(...) part.

Re: [PATCH 2/2] gitweb: introduce localtime feature

[Jakub Narebski]

On Thu, Mar 17, 2011 at 21:12, Kevin Cernekee wrote:
> On Thu, Mar 17, 2011 at 4:01 AM, Jakub Narebski <jnareb@gmail.com> wrote:

> > This does not describe why would one want such way of displaying
> > timestamps, and which views would be affected.
> >
> > BTW. should it be timezone of web server (machine where gitweb is
> > run), or local time of author / committer / tagger as described in the
> > timezone part of git timestamp?
> 
> The case I am currently trying to improve is the one in which all
> developers are at a single site.
> 
> In the open source world it is common to have developers scattered all
> over the globe, so some of them will inevitably have to perform
> timezone conversions.
> 
> But Git is becoming a popular tool in the private sector and it is
> common to have most/all contributors based in a single office.  In the
> latter case, it is helpful to display the local timezone instead of
> GMT.  This also helps make the data more readable by program managers
> and other non-developers who have an interest in tracking the project.

Such explanation should in my opinion be present in the proposed commit
message.  Good commit message should explain not only what the change is
intent to do (to catch when implementation and intent differs), but also
whys behind the change (to find whether commit is worth having).

The above nicely explains why and when such feature would be useful,
 
> > Why project specific override is not supported?  I think it might make
> > sense to enable this feature on project-by-project basis; some
> > projects might be dispersed geographically, some might not.
> 
> Mostly ease of testing.  I did not need it for any of my projects.

You mean that for your instance of gitweb all projects were single
office (not dispersed geographically), so for you enabling it site-wide
was enough, isn't it?
 
> It turned out to be a simple change, and it is in v2.

Thanks.

> The cases I tested were: 
> 
> default 0
> default 1
> override 1, project unset
> override 1, project 0
> override 1, project 1

Well, I think the non-overridden / overridden feature we have tested quite
well.  BTW did you add test for 'localtime' feature to t9500 test?  Perhaps
it is not strictly necessary, though...

> > Is it still an RFC 2822 conformant date?  If it is not, then above
> > change is invalid, and we have to implement this feature in different
> > way.
> 
> I believe it is still valid.
> 
> Original date: Thu, 17 Mar 2011 02:11:05 +0000
> New date: Wed, 16 Mar 2011 19:11:05 -0700
> Sample date from RFC 2822 Appendix A: Fri, 21 Nov 1997 09:55:06 -0600

Thanks.

> > Hmmm... I wonder if it wouldn't be better to print both times (perhaps
> > reversed) in this case...
> 
> I have submitted a third patch which does this.

Note that we print localtime (time in author / committer / tagger timezone)
to be able to mark given time as "atnight", so one can easily see commits
and tags which needs more careful review because they were made 4 AM or
something.

If you reverse the direction you still should make sure that "atnight"
styling applies to localtime.

-- 
Jakub Narebski
Poland