On Sat, Mar 19, 2011 at 2:09 PM, Jakub Narebski <jnareb@xxxxxxxxx> wrote:
>> 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.
Oh OK, I did not notice this or intend to change the formatting. My bad.
>> 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.
I can fix up v4 if Junio thinks this is a better way to go...
It does make the code a little more complicated, though. If we paint
the entire string, it can be done once toward the end of the function,
rather than twice (two different ways) inside the localtime /
no-localtime clauses.
I am largely indifferent to how "atnight" is handled but it would be
good to arrive at a consensus.
>> 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 = @_;
This did not work for me. It interprets "shift" as a variable name.
> Or just use hash reference for $date:
>
> Âsub timestamp_html {
> Â Â Âmy ($date, %opts) = @_;
>
> and use $date->{'sth'}.
That is what I did on the latest 3/3 (change "atnight" highlighting)
patch. Seemed to work OK.
Thanks.
--
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