last: please review -F and --time-format

last: please review -F and --time-format

[Ruediger Meier]

Hi,

IMO -F and --time-format=xxx need some review/cleanup regarding
logout times "still running" and "gone - no logout".

Is there any reason why -F and --time-format=full
look different? --time-format=notime looks ugly too.

See the all the cases here: 
------------
for opt in -F --time-format=full --time-format=iso --time-format=short --time-format=notime; do
    echo "### $opt ###"
    ./last  -f ../tests/ts/last/wtmp.LE  $opt  | grep "running\| no logout"
done

### -F ###
torvalds linux        hobby            Mon Aug 26 02:57:08 1991   gone - no logout
reboot   system boot  system-name      Wed Aug 28 20:00:00 2013   still running
### --time-format=full ###
torvalds linux        hobby            Mon Aug 26 02:57:08 1991    gone                    - no logout
reboot   system boot  system-name      Wed Aug 28 20:00:00 2013   still                    running
### --time-format=iso ###
torvalds linux        hobby            1991-08-26T02:57:08+0200   gone - no logout
reboot   system boot  system-name      2013-08-28T20:00:00+0200   still running
### --time-format=short ###
torvalds linux        hobby            Mon Aug 26 02:57    gone - no logout
reboot   system boot  system-name      Wed Aug 28 20:00   still running
### --time-format=notime ###
torvalds linux        hobby              - no logout
reboot   system boot  system-name        running
------------


cu,
Rudi

Re: last: please review -F and --time-format

[Sami Kerola]

On 7 May 2014 15:31, Ruediger Meier <sweet_f_a@gmx.de> wrote:
> IMO -F and --time-format=xxx need some review/cleanup regarding
> logout times "still running" and "gone - no logout".
>
> Is there any reason why -F and --time-format=full
> look different? --time-format=notime looks ugly too.
>
> See the all the cases here:
> ------------
> for opt in -F --time-format=full --time-format=iso --time-format=short --time-format=notime; do
>     echo "### $opt ###"
>     ./last  -f ../tests/ts/last/wtmp.LE  $opt  | grep "running\| no logout"
> done
>
> ### -F ###
> torvalds linux        hobby            Mon Aug 26 02:57:08 1991   gone - no logout
> reboot   system boot  system-name      Wed Aug 28 20:00:00 2013   still running
> ### --time-format=full ###
> torvalds linux        hobby            Mon Aug 26 02:57:08 1991    gone                    - no logout
> reboot   system boot  system-name      Wed Aug 28 20:00:00 2013   still                    running
> ### --time-format=iso ###
> torvalds linux        hobby            1991-08-26T02:57:08+0200   gone - no logout
> reboot   system boot  system-name      2013-08-28T20:00:00+0200   still running
> ### --time-format=short ###
> torvalds linux        hobby            Mon Aug 26 02:57    gone - no logout
> reboot   system boot  system-name      Wed Aug 28 20:00   still running
> ### --time-format=notime ###
> torvalds linux        hobby              - no logout
> reboot   system boot  system-name        running

Hi Ruediger,

The alignment makes sense when you look the printout where there are
sessions  that are still present, no logout, and logged out. In
--time-format=full the hyphen in front of 'no logout' does look a bit
silly. If it really bothers you send a patch.

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

Re: last: please review -F and --time-format

[Ruediger Meier]

On Thursday 08 May 2014, Sami Kerola wrote:
> On 7 May 2014 15:31, Ruediger Meier <sweet_f_a@gmx.de> wrote:
> > IMO -F and --time-format=xxx need some review/cleanup regarding
> > logout times "still running" and "gone - no logout".
> >
> > Is there any reason why -F and --time-format=full
> > look different? --time-format=notime looks ugly too.
> >
> > See the all the cases here:
> > ------------
> > for opt in -F --time-format=full --time-format=iso
> > --time-format=short --time-format=notime; do echo "### $opt ###"
> >     ./last  -f ../tests/ts/last/wtmp.LE  $opt  | grep "running\| no
> > logout" done
> >
> > ### -F ###
> > torvalds linux        hobby            Mon Aug 26 02:57:08 1991  
> > gone - no logout reboot   system boot  system-name      Wed Aug 28
> > 20:00:00 2013   still running ### --time-format=full ###
> > torvalds linux        hobby            Mon Aug 26 02:57:08 1991   
> > gone                    - no logout reboot   system boot 
> > system-name      Wed Aug 28 20:00:00 2013   still                  
> >  running ### --time-format=iso ###
> > torvalds linux        hobby            1991-08-26T02:57:08+0200  
> > gone - no logout reboot   system boot  system-name     
> > 2013-08-28T20:00:00+0200   still running ### --time-format=short
> > ###
> > torvalds linux        hobby            Mon Aug 26 02:57    gone -
> > no logout reboot   system boot  system-name      Wed Aug 28 20:00  
> > still running ### --time-format=notime ###
> > torvalds linux        hobby              - no logout
> > reboot   system boot  system-name        running
>
> Hi Ruediger,
>
> The alignment makes sense when you look the printout where there are
> sessions  that are still present, no logout, and logged out.

Yes but I wondered why is there a difference between "full" and -F 
while "iso" behaves equally to -F.

> In 
> --time-format=full the hyphen in front of 'no logout' does look a bit
> silly. If it really bothers you send a patch.

That's a bit ugly to fix without more refactoring. I've tried here to 
address both issues:
https://github.com/karelzak/util-linux/pull/73

cu,
Rudi

Re: last: please review -F and --time-format

[Karel Zak]

On Thu, May 08, 2014 at 10:26:36PM +0200, Ruediger Meier wrote:
> That's a bit ugly to fix without more refactoring. I've tried here to 
> address both issues:
> https://github.com/karelzak/util-linux/pull/73

 Merged, but note that code like:

   if (ctl->time_fmt > LAST_TIMEFTM_SHORT_CTIME)

 is fragile (as someone can modify LAST_TIMEFTM_* enum), it would be
 better to introduce any macro and keep it near to LAST_TIMEFTM_*
 definitions or fix the problem in a better way.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: last: please review -F and --time-format

[Ruediger Meier]

On Monday 12 May 2014, Karel Zak wrote:
> On Thu, May 08, 2014 at 10:26:36PM +0200, Ruediger Meier wrote:
> > That's a bit ugly to fix without more refactoring. I've tried here
> > to address both issues:
> > https://github.com/karelzak/util-linux/pull/73
>
>  Merged, but note that code like:
>
>    if (ctl->time_fmt > LAST_TIMEFTM_SHORT_CTIME)
>
>  is fragile (as someone can modify LAST_TIMEFTM_* enum), it would be
>  better to introduce any macro and keep it near to LAST_TIMEFTM_*
>  definitions or fix the problem in a better way.

Yes, my first version of that patch had hardcoded

   if (timefmts[ctl->time_fmt].out > 7)

because the particular strings in these if statements really require "7" 
and nothing else. But then I've notived that the whole thing explodes 
anyway if somebody would change fmt values.

cu,
Rudi