On Sat, 19 Mar 2011, Kevin Cernekee wrote:
> On Sat, Mar 19, 2011 at 10:56 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> > Looks like we used to only paint HH:MM part but...
> > ... we now paint the whole line, which I personally think is a friendly
> > move for color challenged people (me included---a larger span of text
> > painted in different colors tends to help you still notice it better using
> > value/brightness difference, even if your hue perception is weaker than
> > other people). But it is a change from the old behaviour and might be
> > worth stating in the log message.
>
> For the $feature{'localtime'} disabled case, the coloring is the same as before.
No, it is not the same. It used to be
Wed, 16 Mar 2011 07:02:42 +0000 (02:02 -0500)
^^^^^
and now it is
Wed, 16 Mar 2011 07:02:42 +0000 (02:02 -0500)
^^^^^^^^^^^^^
which is I also think improvement,... but should be mentioned in the
commit message.
> I will paint the whole line in the next spin, and mention it in the
> commit message.
I think current solution of using
Wed, 16 Mar 2011 02:02:42 -0500 (07:02:42 +0000)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
when 'localtime' feature is enabled is better than painting the whole:
you are now painting the _local_ part, i.e the part responsible for
"atnight" warning.
[...]
> $use_localtime indicates whether or not to add the " (hh:mm -zzzz)" at
> the end. This also enables the atnight coloring.
>
> This argument name was suggested in an earlier post and I guess I took
> it a little too literally...
>
> Do you think it would be a good idea to take two separate options:
> -atnight for the variable coloring, and -alt_time (or some other name)
> to show " (hh:mm -zzzz)" after the RFC 2822 string?
Perhaps name this parameter -long. Or simply always use the long form.
> Or maybe take one option, named something like "-commitpage", to
> indicate that it is a format specific to that view? If it is not
> specified, the caller gets back an uncolored RFC 2822 date.
>
> Also, is there a cleaner way of writing this?
>
> sub timestamp_html {
> my %date = %{$_[0]};
> shift;
> my %opts = @_;
sub timestamp_html {
my %date = %{ shift };
my %opts = @_;
>
> Or should I pass in the options as a hash reference, more like $cgi->a():
>
> sub timestamp_html {
> my %date = %{$_[0]};
> my %opts = %{$_[1]};
Or just use hash reference for $date:
sub timestamp_html {
my ($date, %opts) = @_;
and use $date->{'sth'}.
--
Jakub Narebski
Poland
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html