1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
|
Here is a patch that makes [[ikiwiki-calendar]] almost useless.
It adds some options, the main one being `calendar_autocreate`, which is
similar to the `tag_autocreate` option of the [[tag|plugins/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):
- [[plugin|https://github.com/paternal/ikiwiki/blob/calendar-autocreate/doc/plugins/calendar.mdwn]]
- [[directive|https://github.com/paternal/ikiwiki/blob/calendar-autocreate/doc/ikiwiki/directive/calendar.mdwn]]
[[!tag patch]]
[[!template id=gitbranch branch=spalax/calendar-autocreate browse="https://github.com/paternal/ikiwiki/tree/calendar-autocreate" author="[[Louis|spalax]]"]]
--[[Louis|spalax]]
> 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'd personally do the conditional in gencalendaryear more like:
>
> [[!format perl """
return unless $config{calendar_autocreate};
"""]]
>
> to reduce the indentation depth of the more interesting code.
>
> The recursion to generate missing years:
>
> [[!format perl """
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:
>
> [[!format perl """
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'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.
>
> This piece of ikiwiki-calendar functionality is lost:
>
> [[!format diff """
- ... 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").
>
> [[!format diff """
-\[[!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.
>
> --[[smcv]]
|