From: Simon McVittie Date: Sat, 18 Jul 2009 12:08:13 +0000 (+0100) Subject: Some more attempts to review patches X-Git-Url: https://sipb.mit.edu/gitweb.cgi/ikiwiki.git/commitdiff_plain/900d428e5f28f6b7f98afd36f3e261b3b89b8033?hp=d2b1264546fa412db8c1591bc8bb14aba04b960d Some more attempts to review patches --- diff --git a/doc/todo/Add_space_before_slash_in_parent_links.mdwn b/doc/todo/Add_space_before_slash_in_parent_links.mdwn index 40a334032..9eb8e5364 100644 --- a/doc/todo/Add_space_before_slash_in_parent_links.mdwn +++ b/doc/todo/Add_space_before_slash_in_parent_links.mdwn @@ -8,6 +8,11 @@ This [[patch]] adds a space before the forward-slash in the the parent links. Th >>> Yes, please. This seems to be something a lot of people want to customize. (I certainly do -- a forward slash only looks natural to Unix users) --[[sabr]] +>> Joey, would I be right to summarize your position on this as "people who +>> want to change the text of the templates should maintain their own version +>> of the `.tmpl` files"? It's not clear to me how this todo item could be +>> closed in a way acceptable to you, except perhaps as WONTFIX. --[[smcv]] + Before: ikiwiki/ todo/ Add space before slash in parent links diff --git a/doc/todo/allow_site-wide_meta_definitions.mdwn b/doc/todo/allow_site-wide_meta_definitions.mdwn index 893498f2c..492a8d747 100644 --- a/doc/todo/allow_site-wide_meta_definitions.mdwn +++ b/doc/todo/allow_site-wide_meta_definitions.mdwn @@ -54,3 +54,20 @@ my github ikiwiki fork): * title -- [[Jon]] + +> This doesn't support multiple-argument meta directives like +> `link=x rel=y`, or meta directives with special side-effects like +> `updated`. +> +> The first could be solved (if you care) by a syntax like this: +> +> meta_defaults => [ +> { copyright => "© me" }, +> { link => "about:blank", rel => "silly", }, +> ] +> +> The second could perhaps be solved by invoking `meta::preprocess` from within +> `scan` (which might be a simplification anyway), although this is complicated +> by the fact that some (but not all!) meta headers are idempotent. +> +> --[[smcv]] diff --git a/doc/todo/enable-htaccess-files.mdwn b/doc/todo/enable-htaccess-files.mdwn index 968279113..e302a49ed 100644 --- a/doc/todo/enable-htaccess-files.mdwn +++ b/doc/todo/enable-htaccess-files.mdwn @@ -39,6 +39,13 @@ access and such .htaccess files should not be accessible through wiki cgi. Of co > See [[!debbug 447267]] for a patch for this. +>> It looks to me as though this functionality won't be included in ikiwiki +>> unless someone who wants it takes responsibility for updating the patch +>> from that Debian bug to be applicable to current versions, so that there's a +>> setup file parameter for extra filenames to allow, defaulting to none +>> (i.e. a less simplistic patch than the one at the top of this page). +>> Joey, is this an accurate summary? --[[smcv]] + --- bump! I would like to see some form of this functionality included in ikiwiki. I use a patched version, but diff --git a/doc/todo/language_definition_for_the_meta_plugin.mdwn b/doc/todo/language_definition_for_the_meta_plugin.mdwn index 4ac4e2e25..8c4b45141 100644 --- a/doc/todo/language_definition_for_the_meta_plugin.mdwn +++ b/doc/todo/language_definition_for_the_meta_plugin.mdwn @@ -81,4 +81,16 @@ This may be useful for sites with a few pages in different languages, but no ful > Please resolve lang somewhere reusable rather than within meta plugin: It is certainly usable outside > the scope of the meta plugin as well. --[[JonasSmedegaard]] +>> I don't see any problem with having this in meta? meta is on by default, and +>> other plugins are free to use it or even depend on it (e.g. inline does). +>> +>> My only comments on this patch beyond what Joey said are that the page +>> language could usefully go into `$pagestate{$page}{meta}{lang}` for other +>> plugins to be able to see it (is that what you meant?), and that +>> restricting to 2 characters is too restrictive (HTML 4.01 mentions +>> `en`, `en-US` and `i-navajo` as possible language codes). +>> This slightly complicates parsing the locale to get the default language: +>> it'll need `tr/_/-/` after the optional `.encoding` is removed. +>> --[[smcv]] + [[!tag wishlist patch plugins/meta translation]] diff --git a/doc/todo/passwordauth:_sendmail_interface.mdwn b/doc/todo/passwordauth:_sendmail_interface.mdwn index 770c4825a..29f28ca32 100644 --- a/doc/todo/passwordauth:_sendmail_interface.mdwn +++ b/doc/todo/passwordauth:_sendmail_interface.mdwn @@ -52,3 +52,10 @@ Remaining TODOs: > lost it. --[[Joey]] Resent. --[[tschwinge]] + +> Debian now has Mail::Sender, Mail::SendEasy, and Email::Sender +> (which, according to its dpkg description, "replaces the old and sometimes +> problematic Email::Send library, which did a decent job at handling very +> simple email sending tasks, but was not suitable for serious use, for a +> variety of reasons"). Are any of those any better? It's unfortunate that +> there doesn't seem to be a clear "best practice"... --[[smcv]] diff --git a/doc/todo/tmplvars_plugin.mdwn b/doc/todo/tmplvars_plugin.mdwn index 644cf23aa..2fe819682 100644 --- a/doc/todo/tmplvars_plugin.mdwn +++ b/doc/todo/tmplvars_plugin.mdwn @@ -2,6 +2,29 @@ A simple plugin to allow per-page customization of a template by passing paramat [[!tag patch]] +> The implementation looks fine to me (assuming it works with current ikiwiki), +> apart from the "XXX" already noted in the patch. The design could reasonably +> be considered premature generalization, though - how often do you actually +> need to define new tmplvars? +> +> As for the page/destpage/preview thing, it would be good if the preprocess +> hook could distinguish between software-supplied and user-supplied +> parameters (the [[plugins/tag]] plugin could benefit from this too). Perhaps +> the IkiWiki core could be modified so that +> `hook(type => "preprocess", splitparams => 1, ...)` would invoke preprocess +> with { page => "foo", destpage => "bar", ... } as a special first argument, +> and the user-supplied parameters as subsequent arguments? Then plugins like +> tag could use: +> +> my $ikiparams = shift; +> my %params = @_; +> +> add_tags($ikiparams->{page}, keys %params); +> +> --[[smcv]] + +---- + #!/usr/bin/perl package IkiWiki::Plugin::tmplvars;