Here is a patch that makes ikiwiki-calendar almost useless.

merged, thanks! --smcv

It adds some options, the main one being calendar_autocreate, which is similar to the tag_autocreate option of the tag: it create archive pages when needed.

The documentation is updated as well (but as a non-native English speaker, I won't be offended if you correct stuff you consider awkward):

Available in a git repository branch.
Branch: spalax/calendar-autocreate
Author: Louis

--Louis

An attempt at a review (although note that I don't have commit access, so my opinion is not final):

Should calendar_autocreate_commit really default to 1? I would personally expect that any new features that synthesize new pages should not commit them by default - I'd prefer to avoid cluttering git history with generated pages. (Indeed, should the option even exist?)

I copied those options from the tag plugin: the tag_autocreate_commit option exists and default to 1.

It should definitely exists: suppose a calendar page is created and not commited, and later, someone tries to push some changes where a page with the same name has been created. This would result in a conflict. The calendar_autocreate_commit prevents this.

tag_autocreate_commit exists because when tag autocreation was introduced, they were always in the $srcdir and committed. I changed it so that it was possible to put them in the transient underlay and not commit them. It defaults to 1 to preserve existing functionality.

When automatic tag pages (or autoindex pages) are not committed, they go in the transient underlay, which means they can't cause conflicts: independent page creation will simply mask them (a page in the $srcdir hides a page of the same name in an underlay). I thought this implementation did the same when not committing? --smcv

I did not realize how easy it was to use the transient plugin! I took it into account. -- Louis

I'd personally do the conditional in gencalendaryear more like:

return unless $config{calendar_autocreate};

to reduce the indentation depth of the more interesting code.

I agree

The recursion to generate missing years:

if (not exists $wikistate{calendar}{minyear}) {
        $wikistate{calendar}{minyear} = $year;
} elsif ($wikistate{calendar}{minyear} > $year) {
        gencalendaryear($year + 1);
        $wikistate{calendar}{minyear} -= 1;
}

does seem to be correct on closer examination, but it took me a while to work out that it would actually do the right thing by recursing:

  • generate 2005
    • recurse to generate 2006
      • recurse to generate 2007
        • recurse to generate 2008
          • recurse to generate 2009
            • recurse to try to generate 2010 (no effect)
          • minyear = minyear - 1 = 2010 - 1 = 2009
        • minyear = minyear - 1 = 2009 - 1 = 2008
      • minyear = minyear - 1 = 2008 - 1 = 2007
    • minyear = minyear - 1 = 2007 - 1 = 2006
  • minyear = minyear - 1 = 2006 - 1 = 2005

I think it might be clearer (as well as less recursion-happy) to use iteration:

  • generate 2005
    • recurse to generate 2006
    • ...
    • recurse to generate 2009
  • minyear = 2005

something like this:

sub gencalendaryear {
        my $year = shift;
        my %params = @_;
        ...
        # generate this year
        ...
        # Filling potential gaps in years [...] years 2006 to 2009.
        return if $params{norecurse};
        if (not exists $wikistate{calendar}{minyear}) {
                $wikistate{calendar}{minyear} = $year;
        } elsif ($wikistate{calendar}{minyear} > $year) {
                foreach my $other ($year + 1 .. $wikistate{calendar}{minyear} - 1) {
                        gencalendar($year, norecurse => 1);
                }
                $wikistate{calendar}{minyear} = $year;
        }
        # ... and the opposite for maxyear
}

I agree

I'm not sure about generating missing years at all, though: if the generation is entirely dynamic, and there were no posts at all during a particular year (or month for that matter), shouldn't we just skip the year/month? That seems to be what e.g. Wordpress does.

Done. I added an option calendar_fill_gaps to chose between the two alternatives (since skipping empty months and years would change the default behaviour of this plugin).

I think the code is a bit ugly at some places. Perl is not one the the programming languages I am fluent into. Sorry.

PS: Good idea, thought. I now have to implement a similar thing for jscalendar.

This piece of ikiwiki-calendar functionality is lost:

- ... It also refreshes the wiki, updating the calendars to
-highlight the current day. This command is typically run at midnight from
-cron.

If I understand correctly, the highlight will be on the day at which the wiki was last refreshed, which seems arbitrary and confusing. If ikiwiki-calendar is not used, I'd say there should just not be a highlight for today (although I'm not sure how best to implement that - perhaps a config option representing "I am going to use ikiwiki-calendar").

This is not lost. What ikiwiki-calendar do is simply: build the missing archive/year/month pages, and run ikiwiki -refresh. With my patch, the ikiwiki -refresh includes:

  • the build of missing archive/year/month pages;
  • highlighting the current day (this was already the case).

So one can simply drop the ikiwiki-calendar ... for ikiwiki --refresh ... in cron to get the same result.

I tried to make the documentation clearer.

-[[!template  id=plugin name=calendar author="[[ManojSrivastava]]"]]
-[[!tag  type/widget]]

Why did you remove that? It's useful information about the plugin which I think ought to stay.

Oops! It was a mistake. Corrected.

--smcv

Thank you for this review. -- Louis

smcv, can you please go on reviewing this?

I don't think I'm really the reviewer you want, since I don't have commit access (as you might be able to tell from the number of pending branches I have)... but nobody with commit access seems to be available to do reviews at the moment, so I'm probably the best you're going to get.

+    0 0 * * * ikiwiki ~/ikiwiki.setup --refresh

I think that should be ikiwiki --setup ~/ikiwiki.setup --refresh

Corrected

The indentation of some of the new code in IkiWiki/Plugin/calendar.pm is weird. Please use one hard tab (U+0009) per indent step: you seem to have used a mixture of one hard tab per indent or two spaces per indent, which looks bizarre for anyone whose tab size is not 2 spaces.

Corrected

+ return unless $config{calendar_autocreate};

This is checked in gencalendaryear but not in gencalendarmonth. Shouldn't gencalendarmonth do it too? Alternatively, do the check in scan, which calls gencalendarmonth directly.

Once again, you are right

+     my $year  = $date[5] + 1900;

You calculate this, but you don't seem to do anything with it?

Corrected

+  if (not exists $changed{$params{year}}) {
+    $changed{$params{year}} = ();
+  }
+  $changed{$params{year}}{$params{month}} = 1;

$changed{$params{year}} is a scalar (you can tell because it starts with the $ sigil) but () is a list. I think you want {} (a scalar that is a reference to an empty anonymous hash).

However, that whole if block can be omitted, and you can just use $changed{$params{year}}{$params{month}} = 1;, because Perl will automatically create $changed{$params{year}} as a reference to an empty hash if necessary, in order to put the pair $params{month} => 1 in it (the term to look up if you're curious is "autovivification").

Corrected

--smcv

Thank you for your review. --Louis