diff options
Diffstat (limited to 'doc/todo')
11 files changed, 492 insertions, 5 deletions
diff --git a/doc/todo/Add_nicer_math_formatting.mdwn b/doc/todo/Add_nicer_math_formatting.mdwn index 6e082c102..3a5e94a14 100644 --- a/doc/todo/Add_nicer_math_formatting.mdwn +++ b/doc/todo/Add_nicer_math_formatting.mdwn @@ -21,4 +21,6 @@ It would be nice to add nicer math formatting. I currently use the > I've posted a question about this to its forum: --[[Joey]] > <https://sourceforge.net/projects/jsmath/forums/forum/592273/topic/3831574> +I think [mathjax](http://www.mathjax.org/) would be the best option. This is the math rendering engine used in mathoverflow. + [[!tag wishlist]] diff --git a/doc/todo/Improving_the_efficiency_of_match__95__glob.mdwn b/doc/todo/Improving_the_efficiency_of_match__95__glob.mdwn new file mode 100644 index 000000000..4e1df3381 --- /dev/null +++ b/doc/todo/Improving_the_efficiency_of_match__95__glob.mdwn @@ -0,0 +1,228 @@ +[[!template id=gitbranch branch=smcv/ready/glob-cache + author="[[KathrynAndersen]], [[smcv]]"]] +[[!tag patch]] + +I've been profiling my IkiWiki to try to improve speed (with many pages makes speed even more important) and I've written a patch to improve the speed of match_glob. This matcher is a good one to improve the speed of, because it gets called so many times. + +Here's my patch - please consider it! -- [[KathrynAndersen]] + +> It seems to me as though changing `glob2re` to return qr/$re/, and calling +> `memoize(glob2re)` next to the other memoize calls, would be a less +> verbose way to do this? --[[smcv]] + +>> I think so, yeah. Anyway, do you have any benchmark results handy, +>> Kathryn? --[[Joey]] + +>>> See below. +>>> Also, would it make more sense for glob2re to return qr/^$re$/i rather than qr/$re/? Everything that uses glob2re seems to use + $foo =~ /^$re$/i +>>> rather than /$re/ so I think that would make sense. +>>> -- [[KathrynAndersen]] + +>>>> Git branch `smcv/ka-glob-cache` has Kathryn's patch. Git +>>>> branch `smcv/memoize-glob2re` does as I suggested, which +>>>> is less verbose than Kathryn's patch but also not as +>>>> fast; I'm not sure why, tbh. --[[smcv]] + +>>>>> I think it's because my patch focuses on match_glob while the memoize patch focuses on `glob2re`, and `glob2re` is called in `filecheck`, `meta` and `po` as well as in `match_glob` and `match_user`; thus the memoized `glob2re` is dealing with a bigger set of globs to look up, and thus could be just that little bit slower. -- [[KathrynAndersen]] + +>>>>>> What may be going on is that glob2re is already a fairly fast +>>>>>> function, so the overhead of memoizing it with the very generic +>>>>>> `_memoizer` (see its source) swamps the memoization gain. Note +>>>>>> that the few functions memoized with the Memoizer before were much +>>>>>> more expensive, so that little overhead was acceptable then. +>>>>>> +>>>>>> It also may be that Kathryn's patch is slightly faster due to using +>>>>>> the construct `$foo =~ $regexp` rather than `$foo =~ /$regexp/` +>>>>>> (probably avoids a copy or something like that internally) -- +>>>>>> this despite checking both `exists` and `defined` on the hash, which +>>>>>> should be reundant AFAICS. +>>>>>> +>>>>>> My guess is that the best of both worlds would be to move +>>>>>> the byhand memoization to glob2re and have it return a compiled +>>>>>> `/^/i` regexp that can be used without further modifiction in most +>>>>>> cases. --[[Joey]] + +>>>>>>> Done, see `smcv/ready/glob-cache` and `smcv/glob-cache-too-far`. +>>>>>>> +>>>>>>> Kathryn's patch is a significant improvement; my first patch on top of +>>>>>>> that is a trivial cleanup that speeds it up a little, and the next two +>>>>>>> patches (using precompiled regexes) have surprisingly little effect +>>>>>>> (they don't slow it down either though, so either omit them or merge +>>>>>>> them, whichever). Detailed benchmark results below. +>>>>>>> +>>>>>>> Moving the memoization to `glob2re` actually seems to slow things down +>>>>>>> again - I suspect the docwiki has few enough mentions of `user()` etc. +>>>>>>> that caching them is a waste of time, but perhaps it's not the most +>>>>>>> representative. +>>>>>>> --[[smcv]] + +[[done]] --[[Joey]] + +-------------------------------------------------------------- + +[[!toggle id="smcv-benchmark" text="current benchmarks"]] + +[[!toggleable id="smcv-benchmark" text=""" +master at time of branch: + + time elapsed (wall): 29.6348 + time running program: 24.9212 (84.09%) + time profiling (est.): 4.7136 (15.91%) + number of calls: 1360181 + number of exceptions: 13 + + %Time Sec. #calls sec/call F name + 13.24 3.2986 3408 0.000968 Text::Balanced::_match_tagged + 10.94 2.7253 79514 0.000034 IkiWiki::PageSpec::match_glob + 3.19 0.7952 59454 0.000013 <anon>:IkiWiki/Plugin/inline.pm:223 + +`Improve the speed of match_glob`: + + time elapsed (wall): 27.9755 + time running program: 23.5293 (84.11%) + time profiling (est.): 4.4461 (15.89%) + number of calls: 1280875 + number of exceptions: 13 + + %Time Sec. #calls sec/call F name + 14.56 3.4257 3408 0.001005 Text::Balanced::_match_tagged + 7.82 1.8403 79514 0.000023 IkiWiki::PageSpec::match_glob + 3.27 0.7698 59454 0.000013 <anon>:IkiWiki/Plugin/inline.pm:223 + +`match_glob: streamline glob cache slightly`: + + time elapsed (wall): 27.5753 + time running program: 23.1714 (84.03%) + time profiling (est.): 4.4039 (15.97%) + number of calls: 1280875 + number of exceptions: 13 + + %Time Sec. #calls sec/call F name + 14.09 3.2637 3408 0.000958 Text::Balanced::_match_tagged + 7.74 1.7926 79514 0.000023 IkiWiki::PageSpec::match_glob + 3.30 0.7646 59454 0.000013 <anon>:IkiWiki/Plugin/inline.pm:223 + +`glob2re: return a precompiled, anchored case-insensitiv...`: + + time elapsed (wall): 27.5656 + time running program: 23.1464 (83.97%) + time profiling (est.): 4.4192 (16.03%) + number of calls: 1282189 + number of exceptions: 13 + + %Time Sec. #calls sec/call F name + 14.21 3.2891 3408 0.000965 Text::Balanced::_match_tagged + 7.72 1.7872 79514 0.000022 IkiWiki::PageSpec::match_glob + 3.32 0.7678 59454 0.000013 <anon>:IkiWiki/Plugin/inline.pm:223 + +`make use of precompiled regex objects`: + + time elapsed (wall): 27.5357 + time running program: 23.1289 (84.00%) + time profiling (est.): 4.4068 (16.00%) + number of calls: 1281981 + number of exceptions: 13 + + %Time Sec. #calls sec/call F name + 14.17 3.2776 3408 0.000962 Text::Balanced::_match_tagged + 7.70 1.7814 79514 0.000022 IkiWiki::PageSpec::match_glob + 3.35 0.7756 59454 0.000013 <anon>:IkiWiki/Plugin/inline.pm:223 + +`move memoization from match_glob to glob2re`: + + time elapsed (wall): 28.7677 + time running program: 23.9473 (83.24%) + time profiling (est.): 4.8205 (16.76%) + number of calls: 1360181 + number of exceptions: 13 + + %Time Sec. #calls sec/call F name + 13.98 3.3469 3408 0.000982 Text::Balanced::_match_tagged + 8.85 2.1194 79514 0.000027 IkiWiki::PageSpec::match_glob + 3.24 0.7750 59454 0.000013 <anon>:IkiWiki/Plugin/inline.pm:223 + +--[[smcv]] +"""]] + +-------------------------------------------------------------- + +[[!toggle id="ka-benchmarks" text="Kathryn's benchmarks"]] + +[[!toggleable id="ka-benchmarks" text=""" +Benchmarks done with Devel::Profile on the same testbed IkiWiki setup. I'm just showing the start of the profile output, since that's what's relevant. + +Before: +<pre> +time elapsed (wall): 27.4173 +time running program: 22.5909 (82.40%) +time profiling (est.): 4.8264 (17.60%) +number of calls: 1314729 +number of exceptions: 65 + +%Time Sec. #calls sec/call F name +11.05 2.4969 62333 0.000040 IkiWiki::PageSpec::match_glob + 4.10 0.9261 679 0.001364 Text::Balanced::_match_tagged + 2.72 0.6139 59812 0.000010 IkiWiki::SuccessReason::merge_influences +</pre> + +After: +<pre> +time elapsed (wall): 26.1843 +time running program: 21.5673 (82.37%) +time profiling (est.): 4.6170 (17.63%) +number of calls: 1252433 +number of exceptions: 65 + +%Time Sec. #calls sec/call F name + 7.66 1.6521 62333 0.000027 IkiWiki::PageSpec::match_glob + 4.33 0.9336 679 0.001375 Text::Balanced::_match_tagged + 2.81 0.6057 59812 0.000010 IkiWiki::SuccessReason::merge_influences +</pre> + +Note that the seconds per call for match_glob in the "after" case has gone down by about a third. + +K.A. +"""]] + +-------------------------------------------------------------- + +[[!toggle id="ka-patch" text="Kathryn's original patch"]] + +[[!toggleable id="ka-patch" text=""" + +<pre> +diff --git a/IkiWiki.pm b/IkiWiki.pm +index 08a3d78..c187b98 100644 +--- a/IkiWiki.pm ++++ b/IkiWiki.pm +@@ -2482,6 +2482,8 @@ sub derel ($$) { + return $path; + } + ++my %glob_cache; ++ + sub match_glob ($$;@) { + my $page=shift; + my $glob=shift; +@@ -2489,8 +2491,15 @@ sub match_glob ($$;@) { + + $glob=derel($glob, $params{location}); + +- my $regexp=IkiWiki::glob2re($glob); +- if ($page=~/^$regexp$/i) { ++ # Instead of converting the glob to a regex every time, ++ # cache the compiled regex to save time. ++ if (!exists $glob_cache{$glob} ++ or !defined $glob_cache{$glob}) ++ { ++ my $re=IkiWiki::glob2re($glob); ++ $glob_cache{$glob} = qr/^$re$/i; ++ } ++ if ($page =~ $glob_cache{$glob}) { + if (! IkiWiki::isinternal($page) || $params{internal}) { + return IkiWiki::SuccessReason->new("$glob matches $page"); + } +</pre> +"""]] +-------------------------------------------------------------- diff --git a/doc/todo/__42__forward__42__ing_functionality_for_the_meta_plugin.mdwn b/doc/todo/__42__forward__42__ing_functionality_for_the_meta_plugin.mdwn index 61b19d302..b3804d652 100644 --- a/doc/todo/__42__forward__42__ing_functionality_for_the_meta_plugin.mdwn +++ b/doc/todo/__42__forward__42__ing_functionality_for_the_meta_plugin.mdwn @@ -4,7 +4,7 @@ to the [[`meta`_plugin|plugins/meta]]. > [[done]], with some changes --[[Joey]] Find the most recent version at -<http://www.schwinge.homeip.net/~thomas/tmp/meta_forward.patch>. +<http://schwinge.homeip.net/~thomas/tmp/meta_forward.patch>. I can't use `scrub(...)`, as that will strip out the forwarding HTML command. How to deal with that? diff --git a/doc/todo/auto-create_tag_pages_according_to_a_template.mdwn b/doc/todo/auto-create_tag_pages_according_to_a_template.mdwn index 92c3b7695..16dc78fb2 100644 --- a/doc/todo/auto-create_tag_pages_according_to_a_template.mdwn +++ b/doc/todo/auto-create_tag_pages_according_to_a_template.mdwn @@ -260,5 +260,8 @@ required to implement [[todo/alias directive]], which couldn't be easily done by writing to the RCS as the page's contents can change depending on which other pages claim it as an alias. --[[chrysn]] +I agree with [[chrysn]]. In fact, is there any good reason that the core tag plugin doesn't do this? The current usability is horrible, to the point that I have gone 2.5 years with Ikiwiki and haven't yet started using tags. -- + +> See [[todo/transient_pages]] for progress on this. --[[smcv]] [[!tag done]] diff --git a/doc/todo/autoindex_should_use_add__95__autofile.mdwn b/doc/todo/autoindex_should_use_add__95__autofile.mdwn new file mode 100644 index 000000000..19c5004f8 --- /dev/null +++ b/doc/todo/autoindex_should_use_add__95__autofile.mdwn @@ -0,0 +1,4 @@ +`add_autofile` is a generic version of [[plugins/autoindex]]'s code, +so the latter should probably use the former. --[[smcv]] + +> See [[todo/transient_pages]] for progress on this. --[[smcv]] diff --git a/doc/todo/passwordauth:_sendmail_interface.mdwn b/doc/todo/passwordauth:_sendmail_interface.mdwn index 29f28ca32..556240964 100644 --- a/doc/todo/passwordauth:_sendmail_interface.mdwn +++ b/doc/todo/passwordauth:_sendmail_interface.mdwn @@ -35,7 +35,7 @@ in the ikiwiki source code, where emailing is done. OK, so I'll have a look at replacing all email handling with *Email::Send*. [[!tag patch]] -*<http://www.thomas.schwinge.homeip.net/tmp/ikiwiki-sendmail.patch>* +*<http://schwinge.homeip.net/~thomas/tmp/ikiwiki-sendmail.patch>* Remaining TODOs: diff --git a/doc/todo/replace_HTML::Template_with_Template_Toolkit.mdwn b/doc/todo/replace_HTML::Template_with_Template_Toolkit.mdwn index c4e78ca0b..d55fc0aa8 100644 --- a/doc/todo/replace_HTML::Template_with_Template_Toolkit.mdwn +++ b/doc/todo/replace_HTML::Template_with_Template_Toolkit.mdwn @@ -5,9 +5,14 @@ features and thus makes it rather hard to give an ikiwiki site a consistent look. If you browse the templates provided in the tarball, you'll notice that more than one of them contain the `<html>` tag, which is unnecessary. +> Note that is no longer true, and I didn't have to do such an intrusive +> change to fix it either. --[[Joey]] + Maybe it's just me, I also find HTML::Template cumbersome to use, due in part to its use of capital letters. +> Its entirely optional use of capital letters? --[[Joey]] + Finally, the software seems unmaintained: the mailing list and searchable archives linked from <http://html-template.sourceforge.net/html_template.html#frequently%20asked%20questions> @@ -58,3 +63,25 @@ Yes, Template::Toolkit is very powerful. But I think it's somehow overkill for a I'd have to agree that Template::Toolkit is overkill and personally I'm not a fan, but it is very popular (there is even a book) and the new version (3) is alleged to be much more nimble than current version. --[[ajt]] HTML::Template's HTML-like markup prevents me from editing templates in KompoZer or other WYSIWYG HTML editors. The editor tries to render the template markup rather than display it verbatim, and large parts of the template become invisible. A markup syntax that doesn't confuse editors (such as Template::Toolkit's "[% FOO %]") may promote template customization. The ability to replace the template engine would be within the spirit of ikiwiki's extensibility. --Rocco + + +I agree that being able to replace the template toolkit would be a great piece of modularity, and one I would use. If I could use the slot-based filling and the conditional logic from Template::Toolkit, we could build much more flexible inline and archivepage templates that would look different depending on where in the wiki we use them. Some of this can currently be accomplished with separate templates for each use case and a manual call to the right template in the !inline directive, but this is limited, cumbersome, and makes it difficult to reuse bits of formatting by trapping all of that information in multiple template files. -Ian + +> I don't wish HTML::Template to be *replaced* by Template::Toolkit - as +> others have said above, it's overkill for my needs. However, I also +> agree that HTML::Template has its own problems too. The idea of making +> the template system modular, with a choice of which backend to use - I +> really like that idea. It would enable me to use some other template +> system I like better, such as Text::Template or Text::NeatTemplate. But I +> think it would be a lot of work to implement, though perhaps no more work +> than making the revision-control backend modular, I guess. One would +> need to write an IkiWiki template interface that didn't care what the +> backend was, and yet is somehow still flexible enough to take advantage +> of special features of different backends. There are an *awful lot* of +> things that use templates - not just the `pagetemplate` and `template` +> plugins, but a number of others which have specialized templates of their +> own. -- [[KathrynAndersen]]a + +>> A modular template system in ikiwiki is unlikely, as template objects +>> are part of the API, notably the `pagetemplate` hook. Unless the other +>> system has a compatible template object. --[[Joey]] diff --git a/doc/todo/selective_more_directive.mdwn b/doc/todo/selective_more_directive.mdwn new file mode 100644 index 000000000..2a9998205 --- /dev/null +++ b/doc/todo/selective_more_directive.mdwn @@ -0,0 +1,28 @@ +I'm setting up a blog for NaNoWriMo and other story-writing, which means long posts every day. I want to have excerpts on the front page, which link to the full length story posts. I also want a dedicated page for each story which inlines the story in full and in chronological order. I can use the "more" directive to achieve this effect on the front page but then it spoils the story page. My solution was to add a pages= parameter to the more directive to make it more selective. + + --- /usr/share/perl5/IkiWiki/Plugin/more.pm 2010-10-09 00:09:24.000000000 +0000 + +++ .ikiwiki/IkiWiki/Plugin/more.pm 2010-11-01 20:24:59.000000000 +0000 + @@ -26,7 +26,10 @@ + + $params{linktext} = $linktext unless defined $params{linktext}; + + - if ($params{page} ne $params{destpage}) { + + if ($params{page} ne $params{destpage} && + + (! exists $params{pages} || + + pagespec_match($params{destpage}, $params{pages}, + + location => $params{page}))) { + return "\n". + htmllink($params{page}, $params{destpage}, $params{page}, + linktext => $params{linktext}, + +I can now call it as + + \[[!more pages="index" linktext="Chapter 1" text=""" + etc + """]] + +I'm not entirely happy with the design, since I would rather put this information in the inline directive instead of in every story post. Unfortunately I found no way to pass parameters from the inline directive to the inlined page. + +-- [[dark]] + +> Me neither, but nor do I see a better way, so [[applied|done]]. --[[Joey]] diff --git a/doc/todo/transient_pages.mdwn b/doc/todo/transient_pages.mdwn new file mode 100644 index 000000000..9c1be7362 --- /dev/null +++ b/doc/todo/transient_pages.mdwn @@ -0,0 +1,74 @@ +On [[todo/auto-create_tag_pages_according_to_a_template]], [[chrysn]] +suggests: + +> Instead of creating a file that gets checked in into the RCS, the +> source files could be left out and the output files be written as +> long as there is no physical source file (think of a virtual underlay). +> Something similar would be required to implement alias directive, +> which couldn't be easily done by writing to the RCS as the page's +> contents can change depending on which other pages claim it as an alias. + +`add_autofile` could be adapted to do this, or a similar API could be +added. + +This would also be useful for autoindex, as suggested on +[[plugins/autoindex/discussion]]. I'd also like to use it for +[[plugins/contrib/album]]. + +One refinement I'd suggest is that if the transient page is edited, +its transient contents are evaluated and used as the initial +content for the edit box; after that, it'd become a static page. --[[smcv]] + +-------------------------- + +[[!template id=gitbranch branch=smcv/transient author="[[smcv]]"]] +[[!tag patch]] + +I had a look at implementing this. It turns out to be harder than I thought +to have purely in-memory pages (several plugins want to be able to access the +source file as a file), but I did get this proof-of-concept branch +to write tag and autoindex pages into an underlay. + +This loses the ability to delete the auto-created pages (although they don't +clutter up git this way, at least), and a lot of the code in autoindex is +probably now redundant, so this is probably not quite ready for merge, but +I'd welcome opinions. + +Usage: set `tag_underlay` and/or `autoindex_underlay` to an absolute path, +which you must create beforehand. I suggest *srcdir* + `/.ikiwiki/transient`. + +Refinements that could be made if this approach seems reasonable: + +* make these options boolean, and have the path always be `.ikiwiki/transient` +* improve the `remove` plugin so it also deletes from this special underlay + +>> Perhaps it should be something more generic, so that other plugins could use it (such as "album" mentioned above). +>> The `.ikiwiki/transient` would suit this, but instead of saying "tag_underlay" or "autoindex_underlay" have "use_transient_underlay" or something like that? +>> Or to make it more flexible, have just one option "transient_underlay" which is set to an absolute path, and if it is set, then one is using a transient-underlay. +>> --[[KathrynAndersen]] + +>>> What I had in mind was more like `tag_autocreate_transient => 1` or +>>> `autoindex_transient => 1`; you might conceivably want tags to be +>>> checked in but autoindices to be transient, and it's fine for each +>>> plugin to make its own decision. Going from that to one boolean +>>> (or just always-transient if people don't think that's too +>>> astonishing) would be trivial, though. +>>> +>>> I don't think relocating the transient underlay really makes sense, +>>> except for prototyping: you only want one, and `.ikiwiki` is as good +>>> a place as any (ikiwiki already needs to be able to write there). +>>> +>>> For [[plugins/contrib/album]] I think I'd just make the photo viewer +>>> pages always-transient - you can always make a transient page +>>> permanent by editing it, after all. +>>> +>>> Do you think this approach has enough potential that I should +>>> continue to hack on it? Any thoughts on the implementation? --[[smcv]] + +>>>> Ah, now I understand what you're getting at. Yes, it makes sense to put transient pages under `.ikiwiki`. +>>>> I haven't looked at the code, but I'd be interested in seeing whether it's generic enough to be used by other plugins (such as `album`) without too much fuss. +>>>> The idea of a transient underlay gives us a desirable feature for free: that if someone edits the transient page, it is made permanent and added to the repository. +>>>> +>>>> I think the tricky thing with removing these transient underlay pages is the question of how to prevent whatever auto-generated the pages in the first place from generating them again - or, conversely, how to force whatever auto-generated those pages to regenerate them if you've changed your mind. +>>>> I think you'd need something similar to `will_render` so that transient pages would be automatically removed if whatever auto-generated them is no longer around. +>>>> -- [[KathrynAndersen]] diff --git a/doc/todo/use_secure_cookies_for_ssl_logins.mdwn b/doc/todo/use_secure_cookies_for_ssl_logins.mdwn new file mode 100644 index 000000000..a91a15b98 --- /dev/null +++ b/doc/todo/use_secure_cookies_for_ssl_logins.mdwn @@ -0,0 +1,12 @@ +[[!template id=gitbranch branch=smcv/ready/sslcookie-auto author="[[smcv]]"]] +[[!tag patch]] + +At the moment `sslcookie => 0` never creates secure cookies, so if you log in +with SSL, your browser will send the session cookie even over plain HTTP. +Meanwhile `sslcookie => 1` always creates secure cookies, so you can't +usefully log in over plain http. + +This branch adds `sslcookie => 0, sslcookie_auto => 1` as an option; this +uses the `HTTPS` environment variable, so if you log in over SSL you'll +get a secure session cookie, but if you log in over HTTP, you won't. +(The syntax for the setup file is pretty rubbish - any other suggestions?) diff --git a/doc/todo/want_to_avoid_ikiwiki_using_http_or_https_in_urls_to_allow_serving_both.mdwn b/doc/todo/want_to_avoid_ikiwiki_using_http_or_https_in_urls_to_allow_serving_both.mdwn index 20d22b9ab..264eb9688 100644 --- a/doc/todo/want_to_avoid_ikiwiki_using_http_or_https_in_urls_to_allow_serving_both.mdwn +++ b/doc/todo/want_to_avoid_ikiwiki_using_http_or_https_in_urls_to_allow_serving_both.mdwn @@ -165,14 +165,17 @@ whether `url` and `cgiurl` are on the same server with the the same URL scheme. In theory you could use things like `//static.example.com/wiki/` and `//dynamic.example.com/ikiwiki.cgi` to preserve choice of http/https while switching server, but I don't know how consistently browsers -suppot that. +support that. "local" here is short for "locally valid", because these URLs are neither fully relative nor fully absolute, and there doesn't seem to be a good name for them... -I've tested this on a demo website with the CGI enabled, and it seems to +I've tested this on a demo website with the CGI enabled, and it seemed to work nicely (there might be bugs in some plugins, I didn't try all of them). +The branch at [[todo/use secure cookies for SSL logins]] goes well with +this one. + The `$config{url}` and `$config{cgiurl}` are both HTTP, but if I enable `httpauth`, set `cgiauthurl` to a HTTPS version of the same site and log in via that, links all end up in the HTTPS version. @@ -181,15 +184,93 @@ New API added by this branch: * `urlto(x, y, 'local')` uses `$local_url` instead of `$config{url}` + > Yikes. I see why you wanted to keep it to 3 parameters (4 is too many, + > and po overrides it), but I dislike overloading the third parameter + > like that. + > + > There are fairly few calls to `urlto($foo, $bar)`, so why not + > make that always return the semi-local url form, and leave the third + > parameter for the cases that need a true fully-qualified url. + > The new form for local urls will typically be only a little bit longer, + > except in the unusual case where the cgiurl is elsewhere. --[[Joey]] + + >> So, have urlto(x, y) use `$local_url`? There are few calls, but IMO + >> they're for the most important things - wikilinks, img, map and + >> other ordinary hyperlinks. Using `$local_url` would be fine for + >> webserver-based use, but it does stop you browsing your wiki's + >> HTML over `file:///` (unless you set that as the base URL, but + >> then you can't move it around), and stops you moving simple + >> outputs (like the docwiki!) around. + >> + >> I personally think breaking the docwiki is enough to block that. + >> + >>> Well, the docwiki doesn't have an url configured at all, so I assumed + >>> it would need to fall back to current behavior in that case. I had + >>> not thought about browsing wiki's html files though, good point. + >> + >> How about this? + >> + >> * `urlto($link, $page)` with `$page` defined: relative + >> * `urlto($link, undef)`: local, starts with `/` + >> * `urlto($link)`: also local, as a side-effect + >> * `urlto($link, $anything, 1)` (but idiomatically, `$anything` is + >> normally undef): absolute, starts with `http[s]://` + >> + >> --[[smcv]] + >> + >>> That makes a great deal of sense, bravo for actually removing + >>> parameters in the common case while maintaining backwards + >>> compatability! --[[Joey]] + >>> + >>>> Done in my `localurl` branch; not tested in a whole-wiki way + >>>> yet, but I did add a regression test. I've used + >>>> `urlto(x, undef)` rather than `urlto(x)` so far, but I could + >>>> go back through the codebase using the short form if you'd + >>>> prefer. --[[smcv]] + >>> + >>> It does highlight that it would be better to have a + >>> `absolute_urlto($link)` (or maybe `absolute(urlto($link))` ) + >>> rather than the 3 parameter form. --[[Joey]] + >>> + >>> Possibly. I haven't added this. + * `IkiWiki::baseurl` has a new second argument which works like the third argument of `urlto` + > I assume you have no objection to this --[[smcv]] + + >> It's so little used that I don't really care if it's a bit ugly. + >> (But I assume changes to `urlto` will follow through here anyway.) + >> --[[Joey]] + + >>> I had to use it a bit more, as a replacement for `$config{url}` + >>> when doing things like referencing stylesheets or redirecting to + >>> the top of the wiki. + >>> + >>> I ended up redoing this without the extra parameter. Previously, + >>> `baseurl(undef)` was the absolute URL; now, `baseurl(undef)` is + >>> the local path. I know you objected to me using `baseurl()` in + >>> an earlier branch, because `baseurl().$x` looks confusingly + >>> similar to `baseurl($x)` but has totally different semantics; + >>> I've generally written it `baseurl(undef)` now, to be more + >>> explicit. --[[smcv]] + * `IkiWiki::cgiurl` uses `$local_cgiurl` if passed `local_cgiurl => 1` + > Now changed to always use the `$local_cgiurl`. --[[smcv]] + * `IkiWiki::cgiurl` omits the trailing `?` if given no named parameters except `cgiurl` and/or `local_cgiurl` -Bugs: + > I assume you have no objection to this --[[smcv]] + > + >> Nod, although I don't know of a use case. --[[Joey]] + + >>> The use-case is that I can replace `$config{cgiurl}` with + >>> `IkiWiki::cgiurl()` for things like the action attribute of + >>> forms. --[[smcv]] + +Fixed bugs: * I don't think anything except `openid` calls `cgiurl` without also passing in `local_cgiurl => 1`, so perhaps that should be the default; @@ -199,9 +280,29 @@ Bugs: `cgiurl(cgiurl => $config{cgiurl}, ...)`, although that does look a bit strange + > I agree that makes sense. --[[Joey]] + + >> I'm not completely sure whether you're agreeing with "perhaps do this" + >> or "that looks too strange", so please disambiguate: + >> would you accept a patch that makes `cgiurl` default to a local + >> (starts-with-`/`) result? If you would, that'd reduce the diff. --[[smcv]] + + >>> Yes, I absolutely think it should default to local. (Note that + >>> if `absolute()` were implemented as suggested above, it could also + >>> be used with cgiurl if necessary.) --[[Joey]] + + >>>> Done (minus `absolute()`). --[[smcv]] + +Potential future things: + * It occurs to me that `IkiWiki::cgiurl` could probably benefit from being exported? Perhaps also `IkiWiki::baseurl`? + > Possibly, see [[firm_up_plugin_interface]]. --[[Joey]] + + >> Not really part of this branch, though, so wontfix (unless you ask me + >> to do so). --[[smcv]] + * Or, to reduce use of the unexported `baseurl` function, it might make sense to give `urlto` a special case that references the root of the wiki, with a trailing slash ready to append stuff: perhaps `urlto('/')`, @@ -210,3 +311,11 @@ Bugs: do_something(baseurl => urlto('/', undef, local)`); do_something_else(urlto('/').'style.css'); IkiWiki::redirect(urlto('/', undef, 1)); + + > AFACIS, `baseurl` is only called in 3 places so I don't think that's + > needed. --[[Joey]] + + >> OK, wontfix. For what it's worth, my branch has 6 uses in IkiWiki + >> core code (IkiWiki, CGI, Render and the pseudo-core part of editpage) + >> and 5 in plugins, since I used it for things like redirection back + >> to the top of the wiki --[[smcv]] |