From 1c7d5eabd7c9e94498af7f89a005311a850d742c Mon Sep 17 00:00:00 2001 From: "http://smcv.pseudorandom.co.uk/" Date: Wed, 24 Mar 2010 03:24:15 +0000 Subject: propsed branch --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 47 ++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 doc/todo/allow_plugins_to_add_sorting_methods.mdwn (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn new file mode 100644 index 000000000..3aa1d94a6 --- /dev/null +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -0,0 +1,47 @@ +[[!template id=gitbranch branch=smcv/sort-hooks author="[[Simon_McVittie|smcv]]"]] +[[!tag patch]] + +The available [[ikiwiki/pagespec/sorting]] methods are currently hard-coded in +IkiWiki.pm, making it difficult to add any extra sorting mechanisms. I've +prepared a branch which adds 'sort' as a hook type and uses it to implement a +new `meta_title` sort type. + +Someone could use this hook to make `\[[!inline sort=title]]` prefer the meta +title over the page name, but for compatibility, I'm not going to (I do wonder +whether it would be worth making sort=name an alias for the current sort=title, +and changing the meaning of sort=title in 4.0, though). + +Gitweb: + + +## Documentation extracted from the branch + +### sort hook (added to [[plugins/write]]) + + hook(type => "sort", id => "foo", call => \&sort_by_foo); + +This hook adds an additional [[ikiwiki/pagespec/sorting]] order or overrides +an existing one. The callback is given two page names as arguments, and +returns negative, zero or positive if the first page should come before, +close to (i.e. undefined order), or after the second page. + +For instance, the built-in `title` sort order could be reimplemented as + + sub sort_by_title { + pagetitle(basename($_[0])) cmp pagetitle(basename($_[1])); + } + +### meta_title sort order (conditionally added to [[ikiwiki/pagespec/sorting]]) + +* `meta_title` - Order according to the `\[[!meta title="foo" sort="bar"]]` + or `\[[!meta title="foo"]]` [[ikiwiki/directive]], or the page name if no + full title was set. + +### meta title sort parameter (added to [[ikiwiki/directive/meta]]) + +An optional `sort` parameter will be used preferentially when +[[ikiwiki/pagespec/sorting]] by `meta_title`: + + \[[!meta title="The Beatles" sort="Beatles, The"]] + + \[[!meta title="David Bowie" sort="Bowie, David"]] -- cgit v1.2.3 From 7fbddb032ee952c6d0b1ee290568ea4f42ef181f Mon Sep 17 00:00:00 2001 From: "http://smcv.pseudorandom.co.uk/" Date: Wed, 24 Mar 2010 03:27:11 +0000 Subject: link to an alternative approach that I decided against --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index 3aa1d94a6..1533b6c44 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -14,6 +14,12 @@ and changing the meaning of sort=title in 4.0, though). Gitweb: +I briefly tried to turn *all* the current sort types into hook functions, and +have some of them pre-registered, but decided that probably wasn't a good idea. +That earlier version of the branch is also available for comparison: + + + ## Documentation extracted from the branch ### sort hook (added to [[plugins/write]]) -- cgit v1.2.3 From 48c09d44637dd724d084b1d06e2277f11e80d489 Mon Sep 17 00:00:00 2001 From: "http://smcv.pseudorandom.co.uk/" Date: Wed, 24 Mar 2010 03:30:06 +0000 Subject: note: old version untested --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index 1533b6c44..99f256fbe 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -20,6 +20,11 @@ That earlier version of the branch is also available for comparison: +(The older version is untested, and probably doesn't really work as-is - I +misunderstood the details of how the built-in function `sort` works when using +`$a` and `$b`. The newer version has been tested, and has a regression test for +its core functionality.) + ## Documentation extracted from the branch ### sort hook (added to [[plugins/write]]) -- cgit v1.2.3 From 9d4bedf760fbbbdba28986c01c3e429f67386217 Mon Sep 17 00:00:00 2001 From: "http://smcv.pseudorandom.co.uk/" Date: Wed, 24 Mar 2010 17:11:17 +0000 Subject: relationship with [[plugins/contrib/report]] --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index 99f256fbe..21800f4de 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -25,6 +25,21 @@ misunderstood the details of how the built-in function `sort` works when using `$a` and `$b`. The newer version has been tested, and has a regression test for its core functionality.) +This hook *isn't* (yet) sufficient to implement [[plugins/contrib/report]]'s +NIH'd sorting mechanisms: + +* `report` can sort by any [[plugins/contrib/field]], whereas this one has a + finite number of hooks: if the `field` plugin's functionality is desirable, + perhaps parameterized sort mechanisms similar to pagespec match functions + would be useful? Then the `field` plugin could register + `hook(type => "sort", id => "field")` and you could have + `\[[!inline ... sort="field(Mood)"]]` or something? + +* `report` can sort by multiple criteria, with independent direction-changing: + if this is desirable, perhaps `pagespec_match_list` could be enhanced to + interpret `sort="x -y z(w)"` as sorting by (pseudocode) + `{ $cmp_x->($a, $b) || (-$cmp_y->($a, $b)) || $cmp_z->($a, $b, "w") }`? + ## Documentation extracted from the branch ### sort hook (added to [[plugins/write]]) -- cgit v1.2.3 From 1a587504e949bf0584acdf7737597c8332467332 Mon Sep 17 00:00:00 2001 From: "http://kerravonsen.dreamwidth.org/" Date: Wed, 24 Mar 2010 22:48:47 +0000 Subject: what about a SortSpec rather than a sort-hook? --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 2 ++ 1 file changed, 2 insertions(+) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index 21800f4de..5bfe102ac 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -40,6 +40,8 @@ NIH'd sorting mechanisms: interpret `sort="x -y z(w)"` as sorting by (pseudocode) `{ $cmp_x->($a, $b) || (-$cmp_y->($a, $b)) || $cmp_z->($a, $b, "w") }`? +>> I wonder if IkiWiki would benefit from the concept of a "sortspec", like a [[ikiwiki/PageSpec]] but dedicated to sorting lists of pages rather than defining lists of pages? Rather than defining a sort-hook, define a SortSpec class, and enable people to add their own sort methods as functions defined inside that class, similarly to the way they can add their own pagespec definitions. --[[KathrynAndersen]] + ## Documentation extracted from the branch ### sort hook (added to [[plugins/write]]) -- cgit v1.2.3 From 81cd30690024db1fc0b300e3a09504f1c613be21 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 25 Mar 2010 00:05:58 +0000 Subject: an updated branch --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 29 +++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index 5bfe102ac..419a73419 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -40,8 +40,13 @@ NIH'd sorting mechanisms: interpret `sort="x -y z(w)"` as sorting by (pseudocode) `{ $cmp_x->($a, $b) || (-$cmp_y->($a, $b)) || $cmp_z->($a, $b, "w") }`? +> I've now added both of these features to the sort-hooks branch. --[[smcv]] + >> I wonder if IkiWiki would benefit from the concept of a "sortspec", like a [[ikiwiki/PageSpec]] but dedicated to sorting lists of pages rather than defining lists of pages? Rather than defining a sort-hook, define a SortSpec class, and enable people to add their own sort methods as functions defined inside that class, similarly to the way they can add their own pagespec definitions. --[[KathrynAndersen]] +>>> I'd be inclined to think that's overkill, but it probably wouldn't be +>>> all that hard to implement... Joey? Any thoughts? --s + ## Documentation extracted from the branch ### sort hook (added to [[plugins/write]]) @@ -49,7 +54,9 @@ NIH'd sorting mechanisms: hook(type => "sort", id => "foo", call => \&sort_by_foo); This hook adds an additional [[ikiwiki/pagespec/sorting]] order or overrides -an existing one. The callback is given two page names as arguments, and +an existing one. + +The callback is given two page names followed by the parameter as arguments, and returns negative, zero or positive if the first page should come before, close to (i.e. undefined order), or after the second page. @@ -59,12 +66,32 @@ For instance, the built-in `title` sort order could be reimplemented as pagetitle(basename($_[0])) cmp pagetitle(basename($_[1])); } +and to sort by an arbitrary `meta` value, you could use: + + # usage: sort="meta(description)" + sub sort_by_meta { + my $param = $_[2]; + error "sort=meta requires a parameter" unless defined $param; + my $left = $pagestate{$_[0]}{meta}{$param}; + $left = "" unless defined $left; + my $right = $pagestate{$_[1]}{meta}{$param}; + $right = "" unless defined $right; + return $left cmp $right; + } + + ### meta_title sort order (conditionally added to [[ikiwiki/pagespec/sorting]]) * `meta_title` - Order according to the `\[[!meta title="foo" sort="bar"]]` or `\[[!meta title="foo"]]` [[ikiwiki/directive]], or the page name if no full title was set. +### Multiple sort orders (added to [[ikiwiki/pagespec/sorting]]) + +In addition, you can combine several sort orders and/or reverse the order of +sorting, with a string like `age -title` (which would sort by age, then by +title in reverse order if two pages have the same age). + ### meta title sort parameter (added to [[ikiwiki/directive/meta]]) An optional `sort` parameter will be used preferentially when -- cgit v1.2.3 From 959d5b197d842e494076034f89d7bca84e531a45 Mon Sep 17 00:00:00 2001 From: "http://smcv.pseudorandom.co.uk/" Date: Thu, 25 Mar 2010 23:39:45 +0000 Subject: an alternative way to do plugins, as rubykat suggested --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 37 ++++++++++++++++++++-- 1 file changed, 34 insertions(+), 3 deletions(-) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index 419a73419..f37a0758e 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -44,10 +44,13 @@ NIH'd sorting mechanisms: >> I wonder if IkiWiki would benefit from the concept of a "sortspec", like a [[ikiwiki/PageSpec]] but dedicated to sorting lists of pages rather than defining lists of pages? Rather than defining a sort-hook, define a SortSpec class, and enable people to add their own sort methods as functions defined inside that class, similarly to the way they can add their own pagespec definitions. --[[KathrynAndersen]] ->>> I'd be inclined to think that's overkill, but it probably wouldn't be ->>> all that hard to implement... Joey? Any thoughts? --s +>>> [[!template id=gitbranch branch=smcv/sort-package author="[[Simon_McVittie|smcv]]"]] +>>> I'd be inclined to think that's overkill, but it wasn't very hard to +>>> implement, and in a way is more elegant. I set it up so sort mechanisms +>>> share the `IkiWiki::PageSpec` package, but with a `cmp_` prefix. Gitweb: +>>> -## Documentation extracted from the branch +## Documentation from sort-hooks branch ### sort hook (added to [[plugins/write]]) @@ -100,3 +103,31 @@ An optional `sort` parameter will be used preferentially when \[[!meta title="The Beatles" sort="Beatles, The"]] \[[!meta title="David Bowie" sort="Bowie, David"]] + +## Documentation from sort-hooks branch + +The changes to [[ikiwiki/pagespec/sorting]] are the same. +The changes to [[plugins/write]] are replaced by: + +### Sorting plugins + +Similarly, it's possible to write plugins that add new functions as +[[ikiwiki/pagespec/sorting]] methods. To achieve this, add a function to +the IkiWiki::PageSpec package named `cmp_foo`, which will be used when sorting +by `foo` or `foo(...)` is requested. + +The function will be passed three or more parameters. The first two are +page names, and the third is `undef` if invoked as `foo`, or the parameter +`"bar"` if invoked as `foo(bar)`. It may also be passed additional, named +parameters. + +It should return the same thing as Perl's `cmp` and `<=>` operators: negative +if the first argument is less than the second, positive if the first argument +is greater, or zero if they are considered equal. It may also raise an +error using `error`, for instance if it needs a parameter but one isn't +provided. + +You can also define a function called `check_cmp_foo` in the same package. +If you do, it will be called while preparing to sort by `foo` or `foo(bar)`, +with argument `undef` or `"bar"` respectively; it may raise an error using +`error`, if sorting like that isn't going to work. -- cgit v1.2.3 From de62ea1cc8111dd96bc84b9c839be99c921168e3 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Fri, 2 Apr 2010 17:03:33 -0400 Subject: fix branch name --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index f37a0758e..8edc95fb9 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -104,7 +104,7 @@ An optional `sort` parameter will be used preferentially when \[[!meta title="David Bowie" sort="Bowie, David"]] -## Documentation from sort-hooks branch +## Documentation from sort-package branch The changes to [[ikiwiki/pagespec/sorting]] are the same. The changes to [[plugins/write]] are replaced by: -- cgit v1.2.3 From 1b0d4e8d885ded59ca0ad1b4d1ca1315585cce06 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Fri, 2 Apr 2010 17:16:12 -0400 Subject: minor comment --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 3 +++ 1 file changed, 3 insertions(+) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index 8edc95fb9..e4e1829dc 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -89,6 +89,9 @@ and to sort by an arbitrary `meta` value, you could use: or `\[[!meta title="foo"]]` [[ikiwiki/directive]], or the page name if no full title was set. + > I feel it sould be clearer to call that "sortas", since "sort=" is used + > to specify a sort method in other directives. --[[Joey]] + ### Multiple sort orders (added to [[ikiwiki/pagespec/sorting]]) In addition, you can combine several sort orders and/or reverse the order of -- cgit v1.2.3 From 9e7dcefd7ed9424de20706f63c7bab5182c5df78 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Fri, 2 Apr 2010 17:26:32 -0400 Subject: comments --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index e4e1829dc..67d85f6f8 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -50,6 +50,21 @@ NIH'd sorting mechanisms: >>> share the `IkiWiki::PageSpec` package, but with a `cmp_` prefix. Gitweb: >>> +>>>> I agree it seems more elegant, so I have focused on it. +>>>> +>>>> I don't know about reusing `IkiWiki::PageSpec` for this. +>>>> +>>>> I would be inclined to drop the `check_` stuff. +>>>> +>>>> Wouldn't it make sense to have `meta(title)` instead +>>>> of `meta_title`? +>>>> +>>>> As I read the regexp in `cmpspec_translate`, the "command" +>>>> is required to have params. They should be optional, +>>>> to match the documentation and because most sort methods +>>>> do not need parameters. +>>>> --[[Joey]] + ## Documentation from sort-hooks branch ### sort hook (added to [[plugins/write]]) -- cgit v1.2.3 From 1faf9b08e16bde9b4b0d72620fcef79c715e64de Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Fri, 2 Apr 2010 17:37:38 -0400 Subject: idea --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index 67d85f6f8..e1e05e81c 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -63,6 +63,14 @@ NIH'd sorting mechanisms: >>>> is required to have params. They should be optional, >>>> to match the documentation and because most sort methods >>>> do not need parameters. +>>>> +>>>> I wonder if it would make sense to add some combining keywords, so +>>>> a sortspec reads like `sort="age then ascending title"` +>>>> In a way, this reduces the amount of syntax that needs to be learned. +>>>> I like the "then" (and it could allow other operations than +>>>> simple combination, if any others make sense). Not so sure about the +>>>> "ascending", which could be "reverse" instead, but "descending age" and +>>>> "ascending age" both seem useful to be able to explicitly specify. >>>> --[[Joey]] ## Documentation from sort-hooks branch -- cgit v1.2.3 From 011fe920d162924876170d167be11dc64cf8be2f Mon Sep 17 00:00:00 2001 From: "http://smcv.pseudorandom.co.uk/" Date: Sat, 3 Apr 2010 00:00:53 +0000 Subject: respond at some length --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 163 ++++++++++++--------- 1 file changed, 94 insertions(+), 69 deletions(-) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index e1e05e81c..156678da7 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -1,4 +1,3 @@ -[[!template id=gitbranch branch=smcv/sort-hooks author="[[Simon_McVittie|smcv]]"]] [[!tag patch]] The available [[ikiwiki/pagespec/sorting]] methods are currently hard-coded in @@ -11,36 +10,13 @@ title over the page name, but for compatibility, I'm not going to (I do wonder whether it would be worth making sort=name an alias for the current sort=title, and changing the meaning of sort=title in 4.0, though). -Gitweb: - +*[sort-hooks branch now withdrawn in favour of sort-package --s]* I briefly tried to turn *all* the current sort types into hook functions, and have some of them pre-registered, but decided that probably wasn't a good idea. That earlier version of the branch is also available for comparison: - - -(The older version is untested, and probably doesn't really work as-is - I -misunderstood the details of how the built-in function `sort` works when using -`$a` and `$b`. The newer version has been tested, and has a regression test for -its core functionality.) - -This hook *isn't* (yet) sufficient to implement [[plugins/contrib/report]]'s -NIH'd sorting mechanisms: - -* `report` can sort by any [[plugins/contrib/field]], whereas this one has a - finite number of hooks: if the `field` plugin's functionality is desirable, - perhaps parameterized sort mechanisms similar to pagespec match functions - would be useful? Then the `field` plugin could register - `hook(type => "sort", id => "field")` and you could have - `\[[!inline ... sort="field(Mood)"]]` or something? - -* `report` can sort by multiple criteria, with independent direction-changing: - if this is desirable, perhaps `pagespec_match_list` could be enhanced to - interpret `sort="x -y z(w)"` as sorting by (pseudocode) - `{ $cmp_x->($a, $b) || (-$cmp_y->($a, $b)) || $cmp_z->($a, $b, "w") }`? - -> I've now added both of these features to the sort-hooks branch. --[[smcv]] +*[also withdrawn in favour of sort-package --s]* >> I wonder if IkiWiki would benefit from the concept of a "sortspec", like a [[ikiwiki/PageSpec]] but dedicated to sorting lists of pages rather than defining lists of pages? Rather than defining a sort-hook, define a SortSpec class, and enable people to add their own sort methods as functions defined inside that class, similarly to the way they can add their own pagespec definitions. --[[KathrynAndersen]] @@ -53,17 +29,65 @@ NIH'd sorting mechanisms: >>>> I agree it seems more elegant, so I have focused on it. >>>> >>>> I don't know about reusing `IkiWiki::PageSpec` for this. ->>>> +>>>> --[[Joey]] + +>>>>> Fair enough, `IkiWiki::SortSpec::cmp_foo` would be just +>>>>> as easy, or `IkiWiki::Sorting::cmp_foo` if you don't like +>>>>> introducing "sort spec" in the API. I took a cue from +>>>>> [[ikiwiki/pagespec/sorting]] being a subpage of +>>>>> [[ikiwiki/pagespec]], and decided that yes, sorting is +>>>>> a bit like a pagespec :-) --s + >>>> I would be inclined to drop the `check_` stuff. ->>>> + +>>>>> It basically exists to support `title_natural`, to avoid +>>>>> firing up the whole import mechanism on every `cmp` +>>>>> (although I suppose that could just be a call to a +>>>>> memoized helper function). It also lets sort specs that +>>>>> *must* have a parameter, like +>>>>> [[field|plugins/contrib/field/discussion]], fail early +>>>>> (again, not so valuable). +>>>>> +>>>>> The former function could be achieved at a small +>>>>> compatibility cost by putting `title_natural` in a new +>>>>> sortnatural plugin (that fails to load if you don't +>>>>> have `title_natural`), if you'd prefer - that's what would +>>>>> have happened if `title_natural` was written after this +>>>>> code had been merged, I suspect. --s + >>>> Wouldn't it make sense to have `meta(title)` instead ->>>> of `meta_title`? ->>>> +>>>> of `meta_title`? --J + +>>>>> Yes, you're right. I added parameters to support `field`, +>>>>> and didn't think about making `meta` use them too. +>>>>> However, `title` does need a special case to make it +>>>>> default to the basename instead of the empty string. +>>>>> +>>>>> Another special case for `title` is to use `titlesort` +>>>>> first (the name `titlesort` is derived from Ogg/FLAC +>>>>> tags, which can have `titlesort` and `artistsort`). +>>>>> I could easily extend that to other metas, though; +>>>>> in fact, for e.g. book lists it would be nice for +>>>>> `field(bookauthor)` to behave similarly, so you can +>>>>> display "Douglas Adams" but sort by "Adams, Douglas". +>>>>> +>>>>> `meta_title` is also meant to be a prototype of how +>>>>> `sort=title` could behave in 4.0 or something - sorting +>>>>> by page name (which usually sorts in approximately the +>>>>> same place as the meta-title, but occasionally not), while +>>>>> displaying meta-titles, does look quite odd. --s + >>>> As I read the regexp in `cmpspec_translate`, the "command" >>>> is required to have params. They should be optional, >>>> to match the documentation and because most sort methods ->>>> do not need parameters. ->>>> +>>>> do not need parameters. --J + +>>>>> No, `$2` is either `\w+\([^\)]*\)` or `[^\s]+` (with the +>>>>> latter causing an error later if it doesn't also match `\w+`). +>>>>> This branch doesn't add any parameterized sort methods, +>>>>> in fact, although I did provide one on +>>>>> [[field's_discussion_page|plugins/contrib/report/discussion]]. --s + >>>> I wonder if it would make sense to add some combining keywords, so >>>> a sortspec reads like `sort="age then ascending title"` >>>> In a way, this reduces the amount of syntax that needs to be learned. @@ -73,38 +97,42 @@ NIH'd sorting mechanisms: >>>> "ascending age" both seem useful to be able to explicitly specify. >>>> --[[Joey]] -## Documentation from sort-hooks branch - -### sort hook (added to [[plugins/write]]) - - hook(type => "sort", id => "foo", call => \&sort_by_foo); - -This hook adds an additional [[ikiwiki/pagespec/sorting]] order or overrides -an existing one. - -The callback is given two page names followed by the parameter as arguments, and -returns negative, zero or positive if the first page should come before, -close to (i.e. undefined order), or after the second page. - -For instance, the built-in `title` sort order could be reimplemented as - - sub sort_by_title { - pagetitle(basename($_[0])) cmp pagetitle(basename($_[1])); - } - -and to sort by an arbitrary `meta` value, you could use: - - # usage: sort="meta(description)" - sub sort_by_meta { - my $param = $_[2]; - error "sort=meta requires a parameter" unless defined $param; - my $left = $pagestate{$_[0]}{meta}{$param}; - $left = "" unless defined $left; - my $right = $pagestate{$_[1]}{meta}{$param}; - $right = "" unless defined $right; - return $left cmp $right; - } +>>>>> Perhaps. I do like the simplicity of [[KathyrnAndersen]]'s syntax +>>>>> from [[plugins/contrib/report]] (which I copied verbatim, except for +>>>>> turning sort-by-`field` into a parameterized spec), and I can't really +>>>>> think of any sensible way to combine sort specs other than "sort by a, +>>>>> break ties by b, ...", possibly with some reversals thrown in. +>>>>> +>>>>> If no other combinations do make sense, is your proposal that "then" +>>>>> is entirely redundant (easy, just make it a predefined sort spec that +>>>>> returns 0!), or that it's mandatory "punctuation" (add an explicit +>>>>> check, or make "then" expand to "||" and let Perl fail to compile +>>>>> the generated code if it's omitted)? +>>>>> +>>>>> It is a little unfortunate that reversal has to move into the sort +>>>>> spec - I prefer `reverse=yes` - but that's necessary for multi-level +>>>>> sorting. I can see your point about ascending/descending being more +>>>>> obvious to look at, but they're also considerably more verbose. +>>>>> +>>>>> Unfortunately, `sort="ascending mtime"` actually sorts by *descending* +>>>>> timestamp (but`sort=age` is fine, because `age` could be defined as +>>>>> now minus `ctime`). `sort=freshness` isn't right either, because +>>>>> "sort by freshness" seems as though it ought to mean freshest first, +>>>>> but "sort by ascending freshness" means put the least fresh first. If +>>>>> we have ascending and descending keywords which are optional, I don't +>>>>> think we really want different sort types to have different default +>>>>> directions - it seems clearer to have `ascending` always be a no-op, +>>>>> and `descending` always negate. +>>>>> +>>>>> Perhaps we could borrow from `meta updated` and use `update_age`? +>>>>> `updateage` would perhaps be a more normal IkiWiki style - but that +>>>>> makes me think that updateage is a quantity analagous to tonnage or +>>>>> voltage, with more or less recently updated pages being said to have +>>>>> more or less updateage. I don't know whether that's good or bad :-) +>>>>> +>>>>> I'm sure there's a much better word, but I can't see it. --s +## Documentation from sort-package branch ### meta_title sort order (conditionally added to [[ikiwiki/pagespec/sorting]]) @@ -115,6 +143,8 @@ and to sort by an arbitrary `meta` value, you could use: > I feel it sould be clearer to call that "sortas", since "sort=" is used > to specify a sort method in other directives. --[[Joey]] + >> Fair enough, that's easy to do. --[[smcv]] + ### Multiple sort orders (added to [[ikiwiki/pagespec/sorting]]) In addition, you can combine several sort orders and/or reverse the order of @@ -130,12 +160,7 @@ An optional `sort` parameter will be used preferentially when \[[!meta title="David Bowie" sort="Bowie, David"]] -## Documentation from sort-package branch - -The changes to [[ikiwiki/pagespec/sorting]] are the same. -The changes to [[plugins/write]] are replaced by: - -### Sorting plugins +### Sorting plugins (added to [[plugins/write]]) Similarly, it's possible to write plugins that add new functions as [[ikiwiki/pagespec/sorting]] methods. To achieve this, add a function to -- cgit v1.2.3 From aec7dec2795aedfe1f13cff9a888bed83ee760df Mon Sep 17 00:00:00 2001 From: "http://smcv.pseudorandom.co.uk/" Date: Sat, 3 Apr 2010 00:12:59 +0000 Subject: make questions to Joey more explicit --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index 156678da7..36c134a59 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -36,9 +36,9 @@ That earlier version of the branch is also available for comparison: >>>>> introducing "sort spec" in the API. I took a cue from >>>>> [[ikiwiki/pagespec/sorting]] being a subpage of >>>>> [[ikiwiki/pagespec]], and decided that yes, sorting is ->>>>> a bit like a pagespec :-) --s +>>>>> a bit like a pagespec :-) Which name would you prefer? --s ->>>> I would be inclined to drop the `check_` stuff. +>>>> I would be inclined to drop the `check_` stuff. --J >>>>> It basically exists to support `title_natural`, to avoid >>>>> firing up the whole import mechanism on every `cmp` @@ -50,10 +50,10 @@ That earlier version of the branch is also available for comparison: >>>>> >>>>> The former function could be achieved at a small >>>>> compatibility cost by putting `title_natural` in a new ->>>>> sortnatural plugin (that fails to load if you don't +>>>>> `sortnatural` plugin (that fails to load if you don't >>>>> have `title_natural`), if you'd prefer - that's what would >>>>> have happened if `title_natural` was written after this ->>>>> code had been merged, I suspect. --s +>>>>> code had been merged, I suspect. Would you prefer this? --s >>>> Wouldn't it make sense to have `meta(title)` instead >>>> of `meta_title`? --J @@ -97,7 +97,7 @@ That earlier version of the branch is also available for comparison: >>>> "ascending age" both seem useful to be able to explicitly specify. >>>> --[[Joey]] ->>>>> Perhaps. I do like the simplicity of [[KathyrnAndersen]]'s syntax +>>>>> Perhaps. I do like the simplicity of [[KathrynAndersen]]'s syntax >>>>> from [[plugins/contrib/report]] (which I copied verbatim, except for >>>>> turning sort-by-`field` into a parameterized spec), and I can't really >>>>> think of any sensible way to combine sort specs other than "sort by a, @@ -130,7 +130,8 @@ That earlier version of the branch is also available for comparison: >>>>> voltage, with more or less recently updated pages being said to have >>>>> more or less updateage. I don't know whether that's good or bad :-) >>>>> ->>>>> I'm sure there's a much better word, but I can't see it. --s +>>>>> I'm sure there's a much better word, but I can't see it. Do you have +>>>>> a better idea? --s ## Documentation from sort-package branch -- cgit v1.2.3 From 5f87d5d242b87ce5cfbd7ac5fcb1efcc62fc5582 Mon Sep 17 00:00:00 2001 From: "http://smcv.pseudorandom.co.uk/" Date: Sat, 3 Apr 2010 00:25:28 +0000 Subject: actually I can see a second use for "nonlinear" syntax - but I don't think it's worth it --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 32 ++++++++++++++-------- 1 file changed, 20 insertions(+), 12 deletions(-) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index 36c134a59..1657ca8e9 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -99,20 +99,28 @@ That earlier version of the branch is also available for comparison: >>>>> Perhaps. I do like the simplicity of [[KathrynAndersen]]'s syntax >>>>> from [[plugins/contrib/report]] (which I copied verbatim, except for ->>>>> turning sort-by-`field` into a parameterized spec), and I can't really ->>>>> think of any sensible way to combine sort specs other than "sort by a, ->>>>> break ties by b, ...", possibly with some reversals thrown in. +>>>>> turning sort-by-`field` into a parameterized spec). >>>>> ->>>>> If no other combinations do make sense, is your proposal that "then" ->>>>> is entirely redundant (easy, just make it a predefined sort spec that ->>>>> returns 0!), or that it's mandatory "punctuation" (add an explicit ->>>>> check, or make "then" expand to "||" and let Perl fail to compile ->>>>> the generated code if it's omitted)? +>>>>> If we're getting into English-like (or at least SQL-like) queries, +>>>>> it might make sense to change the signature of the hook function +>>>>> so it's a function to return a key, e.g. +>>>>> `sub key_age { return -%pagemtime{$_[0]) }`. Then we could sort like +>>>>> this: >>>>> ->>>>> It is a little unfortunate that reversal has to move into the sort ->>>>> spec - I prefer `reverse=yes` - but that's necessary for multi-level ->>>>> sorting. I can see your point about ascending/descending being more ->>>>> obvious to look at, but they're also considerably more verbose. +>>>>> field(artistsort) or field(artist) or constant(Various Artists) then meta(titlesort) or meta(title) or title +>>>>> +>>>>> with "or" binding more closely than "then". Does this seem valuable? +>>>>> I think the implementation would be somewhat more difficult. and +>>>>> it's probably getting too complicated to be worthwhile, though? +>>>>> (The keys that actually benefit from this could just +>>>>> have smarter cmp functions, I think.) +>>>>> +>>>>> If the hooks return keys rather than cmp results, then we could even +>>>>> have "lowercase" as an adjective used like "ascending"... maybe. +>>>>> However, there are two types of adjective here: "lowercase" +>>>>> really applies to the keys, whereas "ascending" applies to the "cmp" +>>>>> result. Again, I think this is getting too complex, and could just +>>>>> be solved with smarter cmp functions. >>>>> >>>>> Unfortunately, `sort="ascending mtime"` actually sorts by *descending* >>>>> timestamp (but`sort=age` is fine, because `age` could be defined as -- cgit v1.2.3 From c2f54ccfb7b401b59f7eda13095e2ba2af69ed7a Mon Sep 17 00:00:00 2001 From: "http://smcv.pseudorandom.co.uk/" Date: Sat, 3 Apr 2010 00:41:31 +0000 Subject: sort-order could usefully be overridden for meta author, too --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 3 +++ 1 file changed, 3 insertions(+) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index 1657ca8e9..e79e52f39 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -169,6 +169,9 @@ An optional `sort` parameter will be used preferentially when \[[!meta title="David Bowie" sort="Bowie, David"]] +> I now realise that `author` should also have this, again for use +> with (Western) names. --s + ### Sorting plugins (added to [[plugins/write]]) Similarly, it's possible to write plugins that add new functions as -- cgit v1.2.3 From 931c7b00ccb47371ee6e1d56baf5c52d725a321f Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Fri, 2 Apr 2010 22:46:31 -0400 Subject: response --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 25 +++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index e79e52f39..c6e18505e 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -38,7 +38,9 @@ That earlier version of the branch is also available for comparison: >>>>> [[ikiwiki/pagespec]], and decided that yes, sorting is >>>>> a bit like a pagespec :-) Which name would you prefer? --s ->>>> I would be inclined to drop the `check_` stuff. --J +>>>>>> `SortSpec` --[[Joey]] + +>>>> I would be inclined to drop the `check_` stuff. --[[Joey]] >>>>> It basically exists to support `title_natural`, to avoid >>>>> firing up the whole import mechanism on every `cmp` @@ -48,6 +50,11 @@ That earlier version of the branch is also available for comparison: >>>>> [[field|plugins/contrib/field/discussion]], fail early >>>>> (again, not so valuable). >>>>> +>>>>>> AFAIK, `use foo` has very low overhead when the module is already +>>>>>> loaded. There could be some evalation overhead in `eval q{use foo}`, +>>>>>> if so it would be worth addressing across the whole codebase. +>>>>>> --[[Joey]] +>>>>> >>>>> The former function could be achieved at a small >>>>> compatibility cost by putting `title_natural` in a new >>>>> `sortnatural` plugin (that fails to load if you don't @@ -55,8 +62,12 @@ That earlier version of the branch is also available for comparison: >>>>> have happened if `title_natural` was written after this >>>>> code had been merged, I suspect. Would you prefer this? --s +>>>>>> Yes! (Assuming it does not make sense to support +>>>>>> natural order sort of other keys than the title, at least..) +>>>>>> --[[Joey]] + >>>> Wouldn't it make sense to have `meta(title)` instead ->>>> of `meta_title`? --J +>>>> of `meta_title`? --[[Joey]] >>>>> Yes, you're right. I added parameters to support `field`, >>>>> and didn't think about making `meta` use them too. @@ -77,10 +88,12 @@ That earlier version of the branch is also available for comparison: >>>>> same place as the meta-title, but occasionally not), while >>>>> displaying meta-titles, does look quite odd. --s +>>>>>> Agreed. --[[Joey]] + >>>> As I read the regexp in `cmpspec_translate`, the "command" >>>> is required to have params. They should be optional, >>>> to match the documentation and because most sort methods ->>>> do not need parameters. --J +>>>> do not need parameters. --[[Joey]] >>>>> No, `$2` is either `\w+\([^\)]*\)` or `[^\s]+` (with the >>>>> latter causing an error later if it doesn't also match `\w+`). @@ -122,6 +135,9 @@ That earlier version of the branch is also available for comparison: >>>>> result. Again, I think this is getting too complex, and could just >>>>> be solved with smarter cmp functions. >>>>> +>>>>>> I agree. (Also, I think returning keys may make it harder to write +>>>>>> smarter cmp functions.) --[[Joey]] +>>>>> >>>>> Unfortunately, `sort="ascending mtime"` actually sorts by *descending* >>>>> timestamp (but`sort=age` is fine, because `age` could be defined as >>>>> now minus `ctime`). `sort=freshness` isn't right either, because @@ -132,6 +148,9 @@ That earlier version of the branch is also available for comparison: >>>>> directions - it seems clearer to have `ascending` always be a no-op, >>>>> and `descending` always negate. >>>>> +>>>>>> I think you've convinced me that ascending/descending impose too +>>>>>> much semantics on it, so "-" is better. --[[Joey]] +>>>>> >>>>> Perhaps we could borrow from `meta updated` and use `update_age`? >>>>> `updateage` would perhaps be a more normal IkiWiki style - but that >>>>> makes me think that updateage is a quantity analagous to tonnage or -- cgit v1.2.3 From 56c64ff1963b33da74677f08a0b8c6579bc2d68b Mon Sep 17 00:00:00 2001 From: "http://smcv.pseudorandom.co.uk/" Date: Sat, 3 Apr 2010 13:39:44 +0000 Subject: updated branch --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 56 +++++++++++++++------- 1 file changed, 38 insertions(+), 18 deletions(-) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index c6e18505e..8c6e1df3b 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -40,6 +40,8 @@ That earlier version of the branch is also available for comparison: >>>>>> `SortSpec` --[[Joey]] +>>>>>>> Done. --s + >>>> I would be inclined to drop the `check_` stuff. --[[Joey]] >>>>> It basically exists to support `title_natural`, to avoid @@ -54,6 +56,8 @@ That earlier version of the branch is also available for comparison: >>>>>> loaded. There could be some evalation overhead in `eval q{use foo}`, >>>>>> if so it would be worth addressing across the whole codebase. >>>>>> --[[Joey]] +>>>>>> +>>>>>>> check_cmp_foo now dropped. --s >>>>> >>>>> The former function could be achieved at a small >>>>> compatibility cost by putting `title_natural` in a new @@ -66,6 +70,8 @@ That earlier version of the branch is also available for comparison: >>>>>> natural order sort of other keys than the title, at least..) >>>>>> --[[Joey]] +>>>>>>> Done. I added some NEWS.Debian for it, too. --s + >>>> Wouldn't it make sense to have `meta(title)` instead >>>> of `meta_title`? --[[Joey]] @@ -90,6 +96,11 @@ That earlier version of the branch is also available for comparison: >>>>>> Agreed. --[[Joey]] +>>>>>>> I've implemented meta(title). meta(author) also has the +>>>>>>> `sortas` special case; meta(updated) and meta(date) +>>>>>>> should also work how you'd expect them to (but they're +>>>>>>> earliest-first, unlike age). --s + >>>> As I read the regexp in `cmpspec_translate`, the "command" >>>> is required to have params. They should be optional, >>>> to match the documentation and because most sort methods @@ -150,6 +161,10 @@ That earlier version of the branch is also available for comparison: >>>>> >>>>>> I think you've convinced me that ascending/descending impose too >>>>>> much semantics on it, so "-" is better. --[[Joey]] + +>>>>>>> I've kept the semantics from `report` as-is, then: +>>>>>>> e.g. `sort="age -title"`. --s + >>>>> >>>>> Perhaps we could borrow from `meta updated` and use `update_age`? >>>>> `updateage` would perhaps be a more normal IkiWiki style - but that @@ -160,18 +175,22 @@ That earlier version of the branch is also available for comparison: >>>>> I'm sure there's a much better word, but I can't see it. Do you have >>>>> a better idea? --s -## Documentation from sort-package branch +[Regarding the `meta title=foo sort=bar` special case] -### meta_title sort order (conditionally added to [[ikiwiki/pagespec/sorting]]) +> I feel it sould be clearer to call that "sortas", since "sort=" is used +> to specify a sort method in other directives. --[[Joey]] +>> Done. --[[smcv]] -* `meta_title` - Order according to the `\[[!meta title="foo" sort="bar"]]` - or `\[[!meta title="foo"]]` [[ikiwiki/directive]], or the page name if no - full title was set. +## Documentation from sort-package branch - > I feel it sould be clearer to call that "sortas", since "sort=" is used - > to specify a sort method in other directives. --[[Joey]] +### advanced sort orders (conditionally added to [[ikiwiki/pagespec/sorting]]) - >> Fair enough, that's easy to do. --[[smcv]] +* `title_natural` - Orders by title, but numbers in the title are treated + as such, ("1 2 9 10 20" instead of "1 10 2 20 9") +* `meta(title)` - Order according to the `\[[!meta title="foo" sortas="bar"]]` + or `\[[!meta title="foo"]]` [[ikiwiki/directive]], or the page name if no + full title was set. `meta(author)`, `meta(date)`, `meta(updated)`, etc. + also work. ### Multiple sort orders (added to [[ikiwiki/pagespec/sorting]]) @@ -179,23 +198,29 @@ In addition, you can combine several sort orders and/or reverse the order of sorting, with a string like `age -title` (which would sort by age, then by title in reverse order if two pages have the same age). -### meta title sort parameter (added to [[ikiwiki/directive/meta]]) +### meta sortas parameter (added to [[ikiwiki/directive/meta]]) + +[in title] An optional `sort` parameter will be used preferentially when -[[ikiwiki/pagespec/sorting]] by `meta_title`: +[[ikiwiki/pagespec/sorting]] by `meta(title)`: \[[!meta title="The Beatles" sort="Beatles, The"]] \[[!meta title="David Bowie" sort="Bowie, David"]] -> I now realise that `author` should also have this, again for use -> with (Western) names. --s +[in author] + + An optional `sortas` parameter will be used preferentially when + [[ikiwiki/pagespec/sorting]] by `meta(author)`: + + \[[!meta author="Joey Hess" sortas="Hess, Joey"]] ### Sorting plugins (added to [[plugins/write]]) Similarly, it's possible to write plugins that add new functions as [[ikiwiki/pagespec/sorting]] methods. To achieve this, add a function to -the IkiWiki::PageSpec package named `cmp_foo`, which will be used when sorting +the IkiWiki::SortSpec package named `cmp_foo`, which will be used when sorting by `foo` or `foo(...)` is requested. The function will be passed three or more parameters. The first two are @@ -208,8 +233,3 @@ if the first argument is less than the second, positive if the first argument is greater, or zero if they are considered equal. It may also raise an error using `error`, for instance if it needs a parameter but one isn't provided. - -You can also define a function called `check_cmp_foo` in the same package. -If you do, it will be called while preparing to sort by `foo` or `foo(bar)`, -with argument `undef` or `"bar"` respectively; it may raise an error using -`error`, if sorting like that isn't going to work. -- cgit v1.2.3 From b51703569d35790f31dccc3dc2921e8bcccd5b49 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Mon, 5 Apr 2010 14:59:29 -0400 Subject: speed --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index 8c6e1df3b..739a3d6b0 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -181,6 +181,20 @@ That earlier version of the branch is also available for comparison: > to specify a sort method in other directives. --[[Joey]] >> Done. --[[smcv]] +## speed + +I notice the implementation does not use the magic `$a` and `$b` globals. +That nasty perl optimisation is still worthwhile: + + perl -e 'use warnings; use strict; use Benchmark; sub a { $a <=> $b } sub b ($$) { $_[0] <=> $_[1] }; my @list=reverse(1..9999); timethese(10000, {a => sub {my @f=sort a @list}, b => sub {my @f=sort b @list}, c => => sub {my @f=sort { b($a,$b) } @list}})' + Benchmark: timing 10000 iterations of a, b, c... + a: 80 wallclock secs (76.74 usr + 0.05 sys = 76.79 CPU) @ 130.23/s (n=10000) + b: 112 wallclock secs (106.14 usr + 0.20 sys = 106.34 CPU) @ 94.04/s (n=10000) + c: 330 wallclock secs (320.25 usr + 0.17 sys = 320.42 CPU) @ 31.21/s (n=10000) + +Unfortunatly, I think that c is closest to the new implementation. +--[[Joey]] + ## Documentation from sort-package branch ### advanced sort orders (conditionally added to [[ikiwiki/pagespec/sorting]]) -- cgit v1.2.3 From 861080b918ef71d82f4a4b9a22093f4a379b5ef8 Mon Sep 17 00:00:00 2001 From: "http://smcv.pseudorandom.co.uk/" Date: Mon, 5 Apr 2010 19:19:00 +0000 Subject: potential performance improvements --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index 739a3d6b0..2ce1de6a4 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -195,6 +195,28 @@ That nasty perl optimisation is still worthwhile: Unfortunatly, I think that c is closest to the new implementation. --[[Joey]] +> Unfortunately, `$a` isn't always `$main::a` - it's `$Package::a` where +> `Package` is the call site of the sort call. This was a showstopper when +> `sort` was a hook implemented in many packages, but now that it's a +> `SortSpec`, I may be able to fix this by putting a `sort` wrapper in the +> `SortSpec` namespace, so it's like this: +> +> sub sort ($@) +> { +> my $cmp = shift; +> return sort $cmp @_; +> } +> +> which would mean that the comparison used `$IkiWiki::SortSpec::a`. +> +> I do notice that `pagespec_match_list` performs the sort before the +> filter by pagespec. Is this a deliberate design choice, or +> coincidence? I can see that when `limit` is used, this could be +> used to only run the pagespec match function until `limit` pages +> have been selected, but the cost is that every page in the wiki +> is sorted. Or, it might be useful to do the filtering first, then +> sort the sub-list thus produced, then finally apply the limit? --s + ## Documentation from sort-package branch ### advanced sort orders (conditionally added to [[ikiwiki/pagespec/sorting]]) -- cgit v1.2.3 From 10f4695abd65db6c009864c5abb7cb5dfa1cf153 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Mon, 5 Apr 2010 15:28:54 -0400 Subject: response --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index 2ce1de6a4..0aca74be2 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -165,7 +165,6 @@ That earlier version of the branch is also available for comparison: >>>>>>> I've kept the semantics from `report` as-is, then: >>>>>>> e.g. `sort="age -title"`. --s ->>>>> >>>>> Perhaps we could borrow from `meta updated` and use `update_age`? >>>>> `updateage` would perhaps be a more normal IkiWiki style - but that >>>>> makes me think that updateage is a quantity analagous to tonnage or @@ -190,7 +189,7 @@ That nasty perl optimisation is still worthwhile: Benchmark: timing 10000 iterations of a, b, c... a: 80 wallclock secs (76.74 usr + 0.05 sys = 76.79 CPU) @ 130.23/s (n=10000) b: 112 wallclock secs (106.14 usr + 0.20 sys = 106.34 CPU) @ 94.04/s (n=10000) - c: 330 wallclock secs (320.25 usr + 0.17 sys = 320.42 CPU) @ 31.21/s (n=10000) + c: 330 wallclock secs (320.25 usr + 0.17 sys = 320.42 CPU) @ 31.21/s (n=10000) Unfortunatly, I think that c is closest to the new implementation. --[[Joey]] @@ -217,6 +216,10 @@ Unfortunatly, I think that c is closest to the new implementation. > is sorted. Or, it might be useful to do the filtering first, then > sort the sub-list thus produced, then finally apply the limit? --s +>> Yes, it was deliberate, pagespec matching can be expensive enough that +>> needing to sort a lot of pages seems likely to be less work. (I don't +>> remember what benchmarking was done though.) --[[Joey]] + ## Documentation from sort-package branch ### advanced sort orders (conditionally added to [[ikiwiki/pagespec/sorting]]) -- cgit v1.2.3 From b186ec1b4cb8145d6a6cb68478e23d7fb0fa1476 Mon Sep 17 00:00:00 2001 From: "http://smcv.pseudorandom.co.uk/" Date: Tue, 6 Apr 2010 00:02:18 +0000 Subject: ready for review, I think --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 51 +++++++++++++++++----- 1 file changed, 39 insertions(+), 12 deletions(-) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index 0aca74be2..d4da13feb 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -20,7 +20,7 @@ That earlier version of the branch is also available for comparison: >> I wonder if IkiWiki would benefit from the concept of a "sortspec", like a [[ikiwiki/PageSpec]] but dedicated to sorting lists of pages rather than defining lists of pages? Rather than defining a sort-hook, define a SortSpec class, and enable people to add their own sort methods as functions defined inside that class, similarly to the way they can add their own pagespec definitions. --[[KathrynAndersen]] ->>> [[!template id=gitbranch branch=smcv/sort-package author="[[Simon_McVittie|smcv]]"]] +>>> [[!template id=gitbranch branch=smcv/ready/sort-package author="[[Simon_McVittie|smcv]]"]] >>> I'd be inclined to think that's overkill, but it wasn't very hard to >>> implement, and in a way is more elegant. I set it up so sort mechanisms >>> share the `IkiWiki::PageSpec` package, but with a `cmp_` prefix. Gitweb: @@ -207,7 +207,26 @@ Unfortunatly, I think that c is closest to the new implementation. > } > > which would mean that the comparison used `$IkiWiki::SortSpec::a`. -> +> --s + +>> I've now done this. On a wiki with many [[plugins/contrib/album]]s +>> (a full rebuild takes half an hour!), I tested a refresh after +>> `touch tags/*.mdwn` (my tag pages contain inlines of the form +>> `tagged(foo)` sorted by date, so they exercise sorting). +>> I also tried removing sorting from `pagespec_match_list` +>> altogether, as an upper bound for how fast we can possibly make it. +>> +>> * `master` at branch point: 63.72user 0.29system +>> * `master` at branch point: 63.91user 0.37system +>> * my branch, with `@_`: 65.28user 0.29system +>> * my branch, with `@_`: 65.21user 0.28system +>> * my branch, with `$a`: 64.09user 0.28system +>> * my branch, with `$a`: 63.83user 0.36system +>> * not sorted at all: 58.99user 0.29system +>> * not sorted at all: 58.92user 0.29system +>> +>> --s + > I do notice that `pagespec_match_list` performs the sort before the > filter by pagespec. Is this a deliberate design choice, or > coincidence? I can see that when `limit` is used, this could be @@ -218,7 +237,15 @@ Unfortunatly, I think that c is closest to the new implementation. >> Yes, it was deliberate, pagespec matching can be expensive enough that >> needing to sort a lot of pages seems likely to be less work. (I don't ->> remember what benchmarking was done though.) --[[Joey]] +>> remember what benchmarking was done though.) --[[Joey]] + +>>> We discussed this on IRC and Joey pointed out that this also affects +>>> dependency calculation, so I'm not going to get into this now... --s + +Joey pointed out on IRC that the `titlesort` feature duplicates all the +meta titles. I did that in order to sort by the unescaped version, but +I've now changed the branch to only store that if it makes a difference. +--s ## Documentation from sort-package branch @@ -262,13 +289,13 @@ Similarly, it's possible to write plugins that add new functions as the IkiWiki::SortSpec package named `cmp_foo`, which will be used when sorting by `foo` or `foo(...)` is requested. -The function will be passed three or more parameters. The first two are -page names, and the third is `undef` if invoked as `foo`, or the parameter -`"bar"` if invoked as `foo(bar)`. It may also be passed additional, named -parameters. +The names of pages to be compared are in the global variables `$a` and `$b` +in the IkiWiki::SortSpec package. The function should return the same thing +as Perl's `cmp` and `<=>` operators: negative if `$a` is less than `$b`, +positive if `$a` is greater, or zero if they are considered equal. It may +also raise an error using `error`, for instance if it needs a parameter but +one isn't provided. -It should return the same thing as Perl's `cmp` and `<=>` operators: negative -if the first argument is less than the second, positive if the first argument -is greater, or zero if they are considered equal. It may also raise an -error using `error`, for instance if it needs a parameter but one isn't -provided. +The function will also be passed one or more parameters. The first is +`undef` if invoked as `foo`, or the parameter `"bar"` if invoked as `foo(bar)`; +it may also be passed additional, named parameters. -- cgit v1.2.3 From dad7ac5a21bc049b9f559c98f4e113c99edb4eb5 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Tue, 6 Apr 2010 23:24:22 -0400 Subject: question --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 3 +++ 1 file changed, 3 insertions(+) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index d4da13feb..d7f10528a 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -10,6 +10,9 @@ title over the page name, but for compatibility, I'm not going to (I do wonder whether it would be worth making sort=name an alias for the current sort=title, and changing the meaning of sort=title in 4.0, though). +> What compatability concerns, exactly, are there that prevent making that +> change now? --[[Joey]] + *[sort-hooks branch now withdrawn in favour of sort-package --s]* I briefly tried to turn *all* the current sort types into hook functions, and -- cgit v1.2.3 From 32ce94f5a30e52da17f06b9b9dce7f3d3112da98 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Tue, 6 Apr 2010 23:30:10 -0400 Subject: close (but one question remains!) --- doc/todo/allow_plugins_to_add_sorting_methods.mdwn | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'doc/todo/allow_plugins_to_add_sorting_methods.mdwn') diff --git a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn index d7f10528a..b523cd19f 100644 --- a/doc/todo/allow_plugins_to_add_sorting_methods.mdwn +++ b/doc/todo/allow_plugins_to_add_sorting_methods.mdwn @@ -43,7 +43,7 @@ That earlier version of the branch is also available for comparison: >>>>>> `SortSpec` --[[Joey]] ->>>>>>> Done. --s +>>>>>>> [[Done]]. --s >>>> I would be inclined to drop the `check_` stuff. --[[Joey]] -- cgit v1.2.3