code review
[ikiwiki] / doc / todo / mirrorlist_with_per-mirror_usedirs_settings.mdwn
1 I've got a wiki that is built at two places:
2
3 * a static copy, aimed at being viewed without any web server, using
4   a web browser's `file:///` urls => usedirs is disabled to get nice
5   and working links
6 * an online copy, with usedirs enabled in order to benefit from the
7   language negotiation using the po plugin
8
9 I need to use mirrorlist on the static copy, so that one can easily
10 reach the online, possibly updated, pages. But as documented, "pages are
11 assumed to exist in the same location under the specified url on each
12 mirror", so the generated urls are wrong.
13
14 My `mirrorlist` branch contains a patch that allows one to configure usedirs
15 per-mirror. Note: the old configuration format is still supported, so this should
16 not break existing wikis.
17
18 OT: as a bonus, this branch contains a patch to support {hashes,arrays} of
19 {hashes,arrays} in `$config`, which I missed a bit when writing the po plugin,
20 and decided this time it was really needed to implement this feature.
21
22 --[[intrigeri]]
23
24 > Ping. --[[intrigeri]]
25
26 [[!tag patch]]
27
28 >> (I'm not an ikiwiki committer, opinions may vary.)
29 >>
30 >>> In my opinion, you're an ikiwiki committer! --[[Joey]]
31 >>
32 >> This would be easier to review if there weren't a million merges from
33 >> master; perhaps either leave a branch as-is, or rebase it, or merge
34 >> only at "significant" times like after a release?
35 >>
36 >> I believe Joey's main objection to complex $config entries is that
37 >> it's not at all clear what [[plugins/websetup]] would do with them.
38 >> Would something like this make a reasonable alternative?
39 >>
40 >>      $config{mirrorlist} = ["nousedirs|file:///home/intrigeri/wiki",
41 >>          "usedirs|http://example.com/wiki", "http://example.net"];
42 >>
43 >> From how I understand tainting, this:
44 >>
45 >>      $untainted{$_} = possibly_foolish_untaint($tainted->{$_})
46 >>
47 >> probably needs to untaint the key too:
48 >>
49 >>      my $key = possibly_foolish_untaint($_);
50 >>      $untainted{$key} = possibly_foolish_untaint($tainted->{key});
51 >>
52 >> --[[smcv]]
53
54 >>> You are fully right about the complex `$config` entries. I'll
55 >>> convert this to use what you are suggesting, i.e. what we ended up
56 >>> choosing for the `po_slave_languages` setting.
57 >>>
58 >>> About the merges in this branch: Joey told me once he did not care
59 >>> about this; moreover the `--no-merges` git log option makes it
60 >>> easy to filter these out. I'll try merging tagged releases only in
61 >>> the future, though.
62 >>>
63 >>> --[[intrigeri]]
64
65 >>>> FWIW, I don't care about merge commits etc because I review
66 >>>> `git diff ...intrigeri/mirrorlist` -- and if I want to dig deeper
67 >>>> into the why of some code, I'll probably checkout the branch and
68 >>>> use git blame.
69 >>>>
70 >>>> I agree with what smcv said, my other concern though is that
71 >>>> this is such an edge case, that supporting it just adds clutter.
72 >>>> Have to wonder if it wouldn't perhaps be better to do something
73 >>>> using the goto plugin and cgiurl, so that the mirror doesn't have
74 >>>> to know about the configuration of the other mirror. --[[Joey]]
75
76 >>>>> I have implemented something using the cgi + goto in my (history
77 >>>>> rewrite warning) mirrorlist branch. Please review, please pull.
78 >>>>> --[[intrigeri]]
79
80 >>>>>> Ping? I've merged 3.20110321 in my `mirrorlist` branch and
81 >>>>>> checked it still works properly. --[[intrigeri]]
82
83 >>>>> concerning goto/cgiurl, what about having that as the default in
84 >>>>> mirrorlist, but keeping ``nousedirs|file:///home/intrigeri/wiki`` and
85 >>>>> ``usedirs|http://example.com/wiki`` valid for cgi-less cases?
86 >>>>> that would keep typical installation with a clutter-less configuration,
87 >>>>> and support more individual setups too.
88 >>>>> --[[chrysn]]
89
90 >>>>>> I would not mind. On the other hand Joey was concerned about
91 >>>>>> cluttering the code to support edge cases, which I fully
92 >>>>>> understand. The case you (chrysn) are describing being even
93 >>>>>> more specific than the one I was initially talking of, I think
94 >>>>>> this should not block the merge of the branch I have been
95 >>>>>> proposing. Support for the usecase you are suggesting can
96 >>>>>> always be added later if needed. --[[intrigeri]]