X-Git-Url: http://git.vanrenterghem.biz/git.ikiwiki.info.git/blobdiff_plain/1a4c6b5fcf0fa36d0368c8294c7629c850dab676..473071c4be84858d0a72c57622db3fe73e9c7ae8:/doc/bugs/pagetitle_function_does_not_respect_meta_titles.mdwn diff --git a/doc/bugs/pagetitle_function_does_not_respect_meta_titles.mdwn b/doc/bugs/pagetitle_function_does_not_respect_meta_titles.mdwn index 77c86eba1..15d28f989 100644 --- a/doc/bugs/pagetitle_function_does_not_respect_meta_titles.mdwn +++ b/doc/bugs/pagetitle_function_does_not_respect_meta_titles.mdwn @@ -1,12 +1,289 @@ +[[!tag patch plugins/inline patch/core]] + The `IkiWiki::pagetitle` function does not respect title changes via `meta.title`. It really should, so that links rendered with `htmllink` get the proper title in the link text. --[[madduck]] -> Agreed. [[todo/using_meta_titles_for_parentlinks]] contains a beginning of -> solution. A few quick notes about it: +---- + +It is possible to set a Page-Title in the meta-plugin, but that one isn't +reused in parentlinks. This patch may fix it. + +
+diff -c /usr/share/perl5/IkiWiki/Render.pm.distrib /usr/share/perl5/IkiWiki/Render.pm +*** /usr/share/perl5/IkiWiki/Render.pm.distrib Wed Aug 6 07:34:55 2008 +--- /usr/share/perl5/IkiWiki/Render.pm Tue Aug 26 23:29:32 2008 +*************** +*** 102,108 **** + $template->param( + title => $page eq 'index' + ? $config{wikiname} +! : pagetitle(basename($page)), + wikiname => $config{wikiname}, + content => $content, + backlinks => $backlinks, +--- 102,108 ---- + $template->param( + title => $page eq 'index' + ? $config{wikiname} +! : pagetitle($page), + wikiname => $config{wikiname}, + content => $content, + backlinks => $backlinks, + +diff -c /usr/share/perl5/IkiWiki/Plugin/parentlinks.pm.distrib /usr/share/perl5/IkiWiki/Plugin/parentlinks.pm +*** /usr/share/perl5/IkiWiki/Plugin/parentlinks.pm.distrib Wed Aug 6 07:34:55 2008 +--- /usr/share/perl5/IkiWiki/Plugin/parentlinks.pm Tue Aug 26 23:19:43 2008 +*************** +*** 44,50 **** + "height_$height" => 1, + }; + $path.="/".$dir; +! $title=IkiWiki::pagetitle($dir); + $i++; + } + return @ret; +--- 44,50 ---- + "height_$height" => 1, + }; + $path.="/".$dir; +! $title=IkiWiki::pagetitle($path); + $i++; + } + return @ret; + +diff -c /usr/share/perl5/IkiWiki.pm.distrib /usr/share/perl5/IkiWiki.pm +*** /usr/share/perl5/IkiWiki.pm.distrib Wed Aug 6 07:48:34 2008 +--- /usr/share/perl5/IkiWiki.pm Tue Aug 26 23:47:30 2008 +*************** +*** 792,797 **** +--- 792,799 ---- + my $page=shift; + my $unescaped=shift; + ++ $page=basename($page); ++ + if ($unescaped) { + $page=~s/(__(\d+)__|_)/$1 eq '_' ? ' ' : chr($2)/eg; + } + +diff -c /usr/share/perl5/IkiWiki/Plugin/meta.pm.distrib /usr/share/perl5/IkiWiki/Plugin/meta.pm +*** /usr/share/perl5/IkiWiki/Plugin/meta.pm.distrib Wed Aug 6 07:34:55 2008 +--- /usr/share/perl5/IkiWiki/Plugin/meta.pm Tue Aug 26 23:30:58 2008 +*************** +*** 3,8 **** +--- 3,9 ---- + package IkiWiki::Plugin::meta; + + use warnings; ++ no warnings 'redefine'; + use strict; + use IkiWiki 2.00; + +*************** +*** 289,294 **** +--- 290,319 ---- + } + } + ++ sub IkiWiki::pagetitle ($;$) { ++ my $page=shift; ++ my $unescaped=shift; ++ ++ if ($page =~ m#/#) { ++ $page =~ s#^/##; ++ $page =~ s#/index$##; ++ if ($pagestate{"$page/index"}{meta}{title}) { ++ $page = $pagestate{"$page/index"}{meta}{title}; ++ } else { ++ $page = IkiWiki::basename($page); ++ } ++ } ++ ++ if ($unescaped) { ++ $page=~s/(__(\d+)__|_)/$1 eq '_' ? ' ' : chr($2)/eg; ++ } ++ else { ++ $page=~s/(__(\d+)__|_)/$1 eq '_' ? ' ' : "$2;"/eg; ++ } ++ ++ return $page; ++ } ++ + package IkiWiki::PageSpec; + + sub match_title ($$;@) { + ++ +---- + +> A few quick notes about it: > - Using
inline
would avoid the redefinition + code duplication.
> - A few plugins would need to be upgraded.
> - It may be necessary to adapt the testsuite in `t/pagetitle.t`, as well.
-
+>
> --[[intrigeri]]
+>
+>> It was actually more complicated than expected. A working prototype is
+>> now in my `meta` branch, see my userpage for the up-to-date url.
+>> Thus tagging patch. --[[intrigeri]]
+>>
+>>> Joey, please consider merging my `meta` branch. --[[intrigeri]]
+
+So, looking at your meta branch: --[[Joey]]
+
+* Inter-page dependencies. If page A links to page B, and page B currently
+ has no title, then A will display the link as "B". Now page B is modified
+ and a title is added. Nothing updates "A".
+ The added overhead of rebuilding every page that links to B when B is
+ changed (as the `indexhtml` hook of the po plugin does) is IMHO a killer.
+ That could be hundreds or thousands of pages, making interactive editing
+ way slow. This is probably the main reason I had not attempted this whole
+ thing myself. IMHO this calls for some kind of intellegent dependency
+ handler that can detect when B's title has changed and only rebuild pages
+ that link to B in that case.
+* Looks like some plugins that use `pagetitle` to format it for display
+ were not changed to use `nicepagetitle` (for example, rename).
+ But most of those callers intend to display the page name
+ as a title, but including the parent directories in the path. (Ie,
+ "renaming foo/page title to bar/page title" --
+ you want to know it's moved from foo to bar.) `nicepagetitle` does not
+ allow doing that since it always takes the `basename`.
+* I don't like the name `nicepagetitle`. It's not very descriptive, is it?
+ And it seems very confusing to choose whether to use the "nice" or original
+ version. My hope is that adding a second function is unnecessary.
+ As I understand it, you added a new function for two reasons:
+ 1) It needs the full page name, not basename.
+ 2) `titlepage(pagetitle($page))` reversability.
+
+ 1) If you look at all the callers
+ Of `pagetitle` most of them pass a complete page name, not just the
+ basename. In most cases `pagetitle` is used to display the full name
+ of the page, including any subdirectory it's in. So why not just make
+ it consitently be given the full name of the page, with another argument
+ specifying if we want to get back just the base name.
+
+ 2) I can't find any code that actually uses the reversability like that.
+ The value passed to `titlepage` always comes from some external
+ source. Unless I missed one.
+* The use of `File::Spec->rel2abs` is a bit scary.
+* Does it really make sense to call `pagetitle` on the meta title
+ in meta's `nicepagetitle`? What if the meta title is something like
+ "foo_bar" -- that would be changed to "foo bar".
+* parentlinks is changed to use `nicepagetitle(bestlink($page, $path))`.
+ Won't `bestlink` return "" if the parent page in question does not exist?
+* `backlinks()` is changed to add an additional `title` field
+ to the hash returned, but AFAICS this is not used in the template.
+* Shouldn't `Render.pm` use nicepagetitle when getting the title for the
+ page template? Then meta would not need to override the title in the
+ `pagetemplate` hook. (Although this would eliminate handling of
+ `title_overridden` -- but that is little used and would not catch
+ all the other ways titles can be overridden with this patch anyway.)
+
+> I'm not a reviewer or anything, but can I chime in on changes to pagetitle?
+> I don't think having meta-titles in wikilinks and the parentlinks path by
+> default is necessarily a good thing. I don't consider the meta-title of a page
+> as used in `