[RFC/PATCH] week numbers for cal

[RFC/PATCH] week numbers for cal

[Tommi Kyntola]

I keep trying -w with cal from time to time and I finally decided it's
about time to do something about it. I'm not sure if this is something
you'd be interested in incorporating into util-linux cal, but it's
something I wanted, so here it is. There's gcal of course, but I only
wanted a toothpick not the whole forest.

I added -w|--week for showing week numbers. It defaults to North
American numbering, because cal itself defaults to Sundays being the
first day of the week. With Monday first it uses the ISO-8601 week
numbers.

I didn't worry too much about the implementation being as clean as
possible just yet, but should this be deemed worth a jack, I or anyone
else can take it to any direction.

The three relevant patches are in my github:
https://github.com/Kynde/util-linux.git
https://github.com/Kynde/util-linux/commits

Tommi Kyntola (4):
      cal: added two missing tests
      cal: added -w|--week for showing week numbers
      cal: tests for week number modes added
      cal: added more week number tests to cover various special cases

The first that's on top of Karel's tree is a minor test fix for
bigyear and sep1752 which seemed to lack final their last tests.

 misc-utils/cal.c            |  140 +++-
 tests/expected/cal/1mw      |   37 ++
 tests/expected/cal/3mw      |   37 ++
 tests/expected/cal/bigyear  |   51 ++
 tests/expected/cal/bigyearw |  257 ++++++++
 tests/expected/cal/colorw   |   55 ++
 tests/expected/cal/sep1752  |   51 ++
 tests/expected/cal/sep1752w |  249 ++++++++
 tests/expected/cal/weeknum  | 1485 +++++++++++++++++++++++++++++++++++++++++++
 tests/expected/cal/yearw    |  177 ++++++
 tests/ts/cal/1mw            |   59 ++
 tests/ts/cal/3mw            |   58 ++
 tests/ts/cal/bigyear        |    1 +
 tests/ts/cal/bigyearw       |   96 +++
 tests/ts/cal/colorw         |   71 +++
 tests/ts/cal/sep1752        |    1 +
 tests/ts/cal/sep1752w       |   96 +++
 tests/ts/cal/weeknum        |   87 +++
 tests/ts/cal/yearw          |   58 ++
 19 files changed, 3037 insertions(+), 29 deletions(-)


Please cc me as I'm not on the list.

cheers,
Tommi Kyntola

Re: [RFC/PATCH] week numbers for cal

[Sami Kerola]

On 5 October 2013 21:59, Tommi Kyntola <tommi.kyntola@gmail.com> wrote:
> I keep trying -w with cal from time to time and I finally decided it's
> about time to do something about it. I'm not sure if this is something
> you'd be interested in incorporating into util-linux cal, but it's
> something I wanted, so here it is. There's gcal of course, but I only
> wanted a toothpick not the whole forest.
>
> I added -w|--week for showing week numbers. It defaults to North
> American numbering, because cal itself defaults to Sundays being the
> first day of the week. With Monday first it uses the ISO-8601 week
> numbers.
>
> I didn't worry too much about the implementation being as clean as
> possible just yet, but should this be deemed worth a jack, I or anyone
> else can take it to any direction.
>
> The three relevant patches are in my github:
> https://github.com/Kynde/util-linux.git
> https://github.com/Kynde/util-linux/commits
>
> Tommi Kyntola (4):
>       cal: added two missing tests
>       cal: added -w|--week for showing week numbers
>       cal: tests for week number modes added
>       cal: added more week number tests to cover various special cases
>
> The first that's on top of Karel's tree is a minor test fix for
> bigyear and sep1752 which seemed to lack final their last tests.
>
>  misc-utils/cal.c            |  140 +++-
>  tests/expected/cal/1mw      |   37 ++
>  tests/expected/cal/3mw      |   37 ++
>  tests/expected/cal/bigyear  |   51 ++
>  tests/expected/cal/bigyearw |  257 ++++++++
>  tests/expected/cal/colorw   |   55 ++
>  tests/expected/cal/sep1752  |   51 ++
>  tests/expected/cal/sep1752w |  249 ++++++++
>  tests/expected/cal/weeknum  | 1485 +++++++++++++++++++++++++++++++++++++++++++
>  tests/expected/cal/yearw    |  177 ++++++
>  tests/ts/cal/1mw            |   59 ++
>  tests/ts/cal/3mw            |   58 ++
>  tests/ts/cal/bigyear        |    1 +
>  tests/ts/cal/bigyearw       |   96 +++
>  tests/ts/cal/colorw         |   71 +++
>  tests/ts/cal/sep1752        |    1 +
>  tests/ts/cal/sep1752w       |   96 +++
>  tests/ts/cal/weeknum        |   87 +++
>  tests/ts/cal/yearw          |   58 ++
>  19 files changed, 3037 insertions(+), 29 deletions(-)
>
> Please cc me as I'm not on the list.

Hello Tommi,

Thank you for the weekday contribution. Earlier the year I though I'll
make this to work, but you were first.

I had look of your patch set and found out peculiarity. If the first
day of the week is highlighted the week number is calculated wrong.

$ cal -w 6 10 2013
      October 2013
   Su Mo Tu We Th Fr Sa
39        1  2  3  4  5
147  6  7  8  9 10 11 12
41 13 14 15 16 17 18 19
42 20 21 22 23 24 25 26
43 27 28 29 30 31

When day is not highlighted error does not occur.

$ cal --color=never -w 13 10 2013

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

Re: [RFC/PATCH] week numbers for cal

[Tommi Kyntola]

Oh, forgot the today flag. It has to be masked out before passing the
day to week_number().
Good catch and thanks for taking the time to look at it!

I fixed the flaw and redid the commits, this time with additional
changes to the man page, bash-completion etc.
(I fixed some cal related faulty translations from the fi.po while at
it, although I doubt anyone uses those here)

I can split the changes into smaller chunks if appropriate, but I'll
hold that until I get more feedback.

cheers,
Tommi Kyntola


On Sun, Oct 6, 2013 at 6:29 PM, Sami Kerola <kerolasa@iki.fi> wrote:
> On 5 October 2013 21:59, Tommi Kyntola <tommi.kyntola@gmail.com> wrote:
>> I keep trying -w with cal from time to time and I finally decided it's
>> about time to do something about it. I'm not sure if this is something
>> you'd be interested in incorporating into util-linux cal, but it's
>> something I wanted, so here it is. There's gcal of course, but I only
>> wanted a toothpick not the whole forest.
>>
>> I added -w|--week for showing week numbers. It defaults to North
>> American numbering, because cal itself defaults to Sundays being the
>> first day of the week. With Monday first it uses the ISO-8601 week
>> numbers.
>>
>> I didn't worry too much about the implementation being as clean as
>> possible just yet, but should this be deemed worth a jack, I or anyone
>> else can take it to any direction.
>>
>> The three relevant patches are in my github:
>> https://github.com/Kynde/util-linux.git
>> https://github.com/Kynde/util-linux/commits
>>
>> Tommi Kyntola (4):
>>       cal: added two missing tests
>>       cal: added -w|--week for showing week numbers
>>       cal: tests for week number modes added
>>       cal: added more week number tests to cover various special cases
>>
>> The first that's on top of Karel's tree is a minor test fix for
>> bigyear and sep1752 which seemed to lack final their last tests.
>>
>>  misc-utils/cal.c            |  140 +++-
>>  tests/expected/cal/1mw      |   37 ++
>>  tests/expected/cal/3mw      |   37 ++
>>  tests/expected/cal/bigyear  |   51 ++
>>  tests/expected/cal/bigyearw |  257 ++++++++
>>  tests/expected/cal/colorw   |   55 ++
>>  tests/expected/cal/sep1752  |   51 ++
>>  tests/expected/cal/sep1752w |  249 ++++++++
>>  tests/expected/cal/weeknum  | 1485 +++++++++++++++++++++++++++++++++++++++++++
>>  tests/expected/cal/yearw    |  177 ++++++
>>  tests/ts/cal/1mw            |   59 ++
>>  tests/ts/cal/3mw            |   58 ++
>>  tests/ts/cal/bigyear        |    1 +
>>  tests/ts/cal/bigyearw       |   96 +++
>>  tests/ts/cal/colorw         |   71 +++
>>  tests/ts/cal/sep1752        |    1 +
>>  tests/ts/cal/sep1752w       |   96 +++
>>  tests/ts/cal/weeknum        |   87 +++
>>  tests/ts/cal/yearw          |   58 ++
>>  19 files changed, 3037 insertions(+), 29 deletions(-)
>>
>> Please cc me as I'm not on the list.
>
> Hello Tommi,
>
> Thank you for the weekday contribution. Earlier the year I though I'll
> make this to work, but you were first.
>
> I had look of your patch set and found out peculiarity. If the first
> day of the week is highlighted the week number is calculated wrong.
>
> $ cal -w 6 10 2013
>       October 2013
>    Su Mo Tu We Th Fr Sa
> 39        1  2  3  4  5
> 147  6  7  8  9 10 11 12
> 41 13 14 15 16 17 18 19
> 42 20 21 22 23 24 25 26
> 43 27 28 29 30 31
>
> When day is not highlighted error does not occur.
>
> $ cal --color=never -w 13 10 2013
>
> --
> Sami Kerola
> http://www.iki.fi/kerolasa/

Re: [RFC/PATCH] week numbers for cal

[Benno Schulenberg]

On Sun, Oct 6, 2013, at 19:23, Tommi Kyntola wrote:
> (I fixed some cal related faulty translations from the fi.po while at
> it, although I doubt anyone uses those here)

Most of the things you changed were to "fuzzy" messages, which don't
get used because they were automatically filled in by msgmerge, so they
were not really faulty (in the sense that the translator made a mistake).
I've forwarded your changes to the Finnish transator; with a little luck
the PO file at the translation project will get updated.

One tiny thing: please wrap lines in the man page at around column 70.

Regards,

Benno

-- 
http://www.fastmail.fm - The professional email service

Re: [RFC/PATCH] week numbers for cal

[Pádraig Brady]

Looks good.
I don't suppose it would be useful to allow
specifying a week number as input, like:

cal --week=44

cheers,
Pádraig.

Re: [RFC/PATCH] week numbers for cal

[Tommi Kyntola]

Why not. I wasn't sure what exactly should be done with 52 when many
years have days belonging to two different 52 weeks, but I came up
with a spec that says it points to the week that starts during the
relevant year.

A patch in guthub:
https://github.com/Kynde/util-linux/commit/3a53abddc14f46eccb4264668b24875054f5ba62

cheers,
Tommi Kyntola
---
commit 3a53abddc14f46eccb4264668b24875054f5ba62
Author: Tommi Kyntola <kynde@iki.fi>
Date:   Mon Oct 7 14:32:23 2013 +0300

    cal: added argument option for -w|--week to display that week

    Because many years have two sections of week 52 or 53, the week that
    this argument points to is the one that starts during that year (when availa
    The week number in argument is also highlighted in addition to possibly visi
    current/defined date highlighting.

    Signed-off-by: Tommi Kyntola <kynde@iki.fi>

 misc-utils/cal.1           |   2 +-
 misc-utils/cal.c           | 115 +++++++++++++++++++++++++++++++++------
 tests/expected/cal/weekarg | 127 +++++++++++++++++++++++++++++++++++++++++++
 tests/ts/cal/weekarg       | 131 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 358 insertions(+), 17 deletions(-)

Re: [RFC/PATCH] week numbers for cal

[Pádraig Brady]

On 10/07/2013 12:39 PM, Tommi Kyntola wrote:
> Why not. I wasn't sure what exactly should be done with 52 when many
> years have days belonging to two different 52 weeks, but I came up
> with a spec that says it points to the week that starts during the
> relevant year.
> 
> A patch in guthub:
> https://github.com/Kynde/util-linux/commit/3a53abddc14f46eccb4264668b24875054f5ba62

It's generally best to avoid optional args on short args,
as in this case -w3 is ambiguous and -wj is problematic.
So I'd instead support an interface like -w,--week[=NUM]

thanks,
Pádraig.

Re: [RFC/PATCH] week numbers for cal

[Tommi Kyntola]

> It's generally best to avoid optional args on short args,
> as in this case -w3 is ambiguous and -wj is problematic.
> So I'd instead support an interface like -w,--week[=NUM]

Agreed. I dropped the :: from the short arg.

cheers,
Tommi Kyntola

Re: [RFC/PATCH] week numbers for cal

[Pádraig Brady]

On 10/07/2013 06:59 PM, Tommi Kyntola wrote:
>> It's generally best to avoid optional args on short args,
>> as in this case -w3 is ambiguous and -wj is problematic.
>> So I'd instead support an interface like -w,--week[=NUM]
> 
> Agreed. I dropped the :: from the short arg.

lgtm

thanks,
Pádraig.

Re: [RFC/PATCH] week numbers for cal

[Karel Zak]

On Sat, Oct 05, 2013 at 11:59:54PM +0300, Tommi Kyntola wrote:
> I added -w|--week for showing week numbers.

 Note that I'm going to merge the patches after v2.24. Now it's too late.

    Karel

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

Re: [RFC/PATCH] week numbers for cal

[Karel Zak]

On Sat, Oct 05, 2013 at 11:59:54PM +0300, Tommi Kyntola wrote:
> I keep trying -w with cal from time to time and I finally decided it's
> about time to do something about it. I'm not sure if this is something
> you'd be interested in incorporating into util-linux cal, but it's
> something I wanted, so here it is. There's gcal of course, but I only
> wanted a toothpick not the whole forest.

Applied, thanks.

Comments:

* please, next time always use space around operators and after
  commas, things like
  
            week_number(xd&~TODAY_FLAG,month+which_cal+1,year,wflag);

  are horrible.

* I don't understand this feature:

           /* In some years (e.g. 2010 in ISO mode) it's possible to
            * have a remnant of week 53 starting the year yet the year
            * in question ends during 52, in this case we're assuming
            * that early remnant is being referred to if 53 is given
            * as argument.  */

 For example:

        $ LANG=de_DE.UTF8 ./cal --week=53 2009
             Dezember 2009     
           Mo Di Mi Do Fr Sa So
        49     1  2  3  4  5  6 
        50  7  8  9 10 11 12 13 
        51 14 15 16 17 18 19 20 
        52 21 22 23 24 25 26 27 
        53 28 29 30 31          

  it's correct, let's try the next year:
                        
        $ LANG=de_DE.UTF8 ./cal --week=53 2010
              Januar 2010      
           Mo Di Mi Do Fr Sa So
        53              1  2  3 
         1  4  5  6  7  8  9 10 
         2 11 12 13 14 15 16 17 
         3 18 19 20 21 22 23 24 
         4 25 26 27 28 29 30 31 
                        
  oh... week 53 of the year 2010 in January 2010, but week 52 is December:

        $ LANG=de_DE.UTF8 ./cal --week=52 2010
             Dezember 2010     
           Mo Di Mi Do Fr Sa So
        48        1  2  3  4  5 
        49  6  7  8  9 10 11 12 
        50 13 14 15 16 17 18 19 
        51 20 21 22 23 24 25 26 
        52 27 28 29 30 31       


 It seems pretty strange to interpret --week=53 2010 as last week
 of the year 2009. I think it would be more intuitive to interpret

      cal --week=53 2010

 as invalid week number (or for another years expect this week after
 week 52).

 Note that I'm talking about command line --week=53 interpretations, the
 week numbers in the output are correct of course.

    Karel
    

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

Re: [RFC/PATCH] week numbers for cal

[Sami Kerola]

On 22 October 2013 11:32, Karel Zak <kzak@redhat.com> wrote:
> On Sat, Oct 05, 2013 at 11:59:54PM +0300, Tommi Kyntola wrote:
>> I keep trying -w with cal from time to time and I finally decided it's
>> about time to do something about it. I'm not sure if this is something
>> you'd be interested in incorporating into util-linux cal, but it's
>> something I wanted, so here it is. There's gcal of course, but I only
>> wanted a toothpick not the whole forest.
>
> Applied, thanks.

FYI

I started think, and play with the cal code, in order to make the source a
little bit more maintainable.  My plan is to add run time configuration
and parameters structures. With bit of luck changing data structures
allows less strange functions.

Broken peak preview:
https://github.com/kerolasa/lelux-utiliteetit/tree/cal-ng

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

Re: [RFC/PATCH] week numbers for cal

[Karel Zak]

On Tue, Oct 22, 2013 at 04:23:22PM +0100, Sami Kerola wrote:
> On 22 October 2013 11:32, Karel Zak <kzak@redhat.com> wrote:
> > On Sat, Oct 05, 2013 at 11:59:54PM +0300, Tommi Kyntola wrote:
> >> I keep trying -w with cal from time to time and I finally decided it's
> >> about time to do something about it. I'm not sure if this is something
> >> you'd be interested in incorporating into util-linux cal, but it's
> >> something I wanted, so here it is. There's gcal of course, but I only
> >> wanted a toothpick not the whole forest.
> >
> > Applied, thanks.
> 
> FYI
> 
> I started think, and play with the cal code, in order to make the source a
> little bit more maintainable.  My plan is to add run time configuration
> and parameters structures. With bit of luck changing data structures
> allows less strange functions.

 That's also my wish. The current source is based on code rather data
 structures. That's alway bad thing, the result is mess of for/if()
 machinery.
 
 It would be nice to describe the problem by structs and split code 
 to two sets of the functions 

        1/ generate calendar(s), 
        2/ print calendar. 
        
  Something like:

  struct cal_week {
        int week_number;
        int days[7];
        ...
  };

  struct cal_month {
        struct cal_week weeks[5];
        ...
  }

  struct cal_control {
        int week_start;
        ...
        struct cal_month cals[12];
  }
 
  cal_new_month();
  cal_add_month();
  cal_get_month();
  cal_month_get_week();
  ...
  cal_print_calendar()

  ... or better.

    Karel

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

Re: [RFC/PATCH] week numbers for cal

[Tommi Kyntola]

On 10/22/13 13:32, Karel Zak wrote:
> On Sat, Oct 05, 2013 at 11:59:54PM +0300, Tommi Kyntola wrote:
>> I keep trying -w with cal from time to time and I finally decided it's
>> about time to do something about it. I'm not sure if this is something
>> you'd be interested in incorporating into util-linux cal, but it's
>> something I wanted, so here it is. There's gcal of course, but I only
>> wanted a toothpick not the whole forest.
>
> Applied, thanks.
 >
> Comments:
>
> * please, next time always use space around operators and after
>    commas, things like
>
>              week_number(xd&~TODAY_FLAG,month+which_cal+1,year,wflag);
>
>    are horrible.


Oh, sorry, I thought we might have gone one more iteration cleaning it up
a bit once you were done with the earlier release.

Not that code cleanliness was a design goal at any point and the code seemed
a little stale to begin with. Frankly, I was expecting responses more along
the lines of "that's hardly needed".

Next time, I'll keep it nice even before the RFC.

> * I don't understand this feature:
>
>             /* In some years (e.g. 2010 in ISO mode) it's possible to
>              * have a remnant of week 53 starting the year yet the year
>              * in question ends during 52, in this case we're assuming
>              * that early remnant is being referred to if 53 is given
>              * as argument.  */
>
>   For example:
>
>          $ LANG=de_DE.UTF8 ./cal --week=53 2009
>               Dezember 2009
>             Mo Di Mi Do Fr Sa So
>          49     1  2  3  4  5  6
>          50  7  8  9 10 11 12 13
>          51 14 15 16 17 18 19 20
>          52 21 22 23 24 25 26 27
>          53 28 29 30 31
>
>    it's correct, let's try the next year:
>
>          $ LANG=de_DE.UTF8 ./cal --week=53 2010
>                Januar 2010
>             Mo Di Mi Do Fr Sa So
>          53              1  2  3
>           1  4  5  6  7  8  9 10
>           2 11 12 13 14 15 16 17
>           3 18 19 20 21 22 23 24
>           4 25 26 27 28 29 30 31
>
>    oh... week 53 of the year 2010 in January 2010, but week 52 is December:
>
>          $ LANG=de_DE.UTF8 ./cal --week=52 2010
>               Dezember 2010
>             Mo Di Mi Do Fr Sa So
>          48        1  2  3  4  5
>          49  6  7  8  9 10 11 12
>          50 13 14 15 16 17 18 19
>          51 20 21 22 23 24 25 26
>          52 27 28 29 30 31
>
>
>   It seems pretty strange to interpret --week=53 2010 as last week
>   of the year 2009. I think it would be more intuitive to interpret
>
>        cal --week=53 2010
>
>   as invalid week number (or for another years expect this week after
>   week 52).
>
>   Note that I'm talking about command line --week=53 interpretations, the
>   week numbers in the output are correct of course.

That was a tricky one. It did initially fail with invalid argument,
as it does with years that don't have that remnant in January (e.g. 2006),
but to me it seemed stranger to fail with --week=53 when it still shows up
in -y output. These two only differ in highlighting:
cal -my --week 2010
cal -my --week=53 2010

But I certainly understand your point and I think you're right, it probably
makes more sense to follow the numbering definitions and make it only show
up weeks with Thursdays in it (in ISO mode).

One can drop the extra check (when month > 12) and errx straight up to make it
error with 53 when the only part of 53 with present is in January.

cheers,
Tommi Kyntola