doc/todo/need_global_renamepage_hook.mdwn

   1 As documented in [[plugins/write]], the current `renamepage` hook is
   2 heavily oriented towards updating links in pages' content:  it is run
   3 once per page linking to the renamed page.
   4
   5 That's fine, but it can't be used to trigger more general actions on
   6 page rename. E.g. it won't be run at all if the page being renamed is
   7 an orphan one.
   8
   9 This is a real issue for the [[plugins/contrib/po]] development: what
  10 I'm about to achieve is:
  11
  12 - when a master page is renamed, the plugin takes notice of it (using
  13   the `rename` hook), and later renames the translation pages
  14   accordingly (in the `change` hook)
  15 - when a master page is deleted, the plugin deletes its translations
  16   (using the `delete` hook)
  17
  18 With the current `renamepage` hook behavior, combining these two goals
  19 has an annoying drawback: a plugin can't notice an orphan master page
  20 has been renamed, so instead of renaming (and preserving) its
  21 translations, it considers the oldpage as deleted, and deletes its
  22 translations. Game over.
  23
  24 It may seem like a corner case, but I want to be very careful when
  25 deleting files automatically in `srcdir`, which is not always under
  26 version control.
  27
  28 As a sad workaround, I can still disable any deletion in `srcdir`
  29 when it is not under version control. But I think ikiwiki deserves
  30 a global `renamepage` hook that would be run once per rename
  31 operation.
  32
  33 My proposal is thus:
  34
  35 - keep the documented `renamepage` hook as it is
  36 - use something inspired by the trick `preprocess` uses: when `hook`
  37   is passed an optional "global" parameter, set to a true value, the
  38   declared `renamepage` hook is run once per rename operation, and is
  39   passed named parameters: `src`, `srcfile`, `dest` and `destfile`.
  40
  41 I'm of course volunteering to implement this, or anything related that
  42 would solve my problem. Hmmm? --[[intrigeri]]
  43
  44 > I think it would be better to have a different hook that is called for
  45 > renames, since the two hook actions are very different (unlike the
  46 > preprocess hook, which does a very similar thing in scan mode).
  47 >
  48 > Just calling it `rename` seems like a reasonable name, by analogy with
  49 > the `delete` and `change` hooks.
  50 >
  51 > It might make sense to rename `renamepage` to `renamelink` to make it
  52 > clearer what it does. (I'm not very worried about this breaking things, at
  53 > this point.) --[[Joey]]
  54
  55 >> In my `po` branch, I renamed `renamepage` to `renamelink`, and
  56 >> created a `rename` hook that is passed a reference to `@torename`.
  57 >> --[[intrigeri]]
  58
  59 >>> As Joey highlights it on [[plugins/contrib/po]], it's too late to
  60 >>> merge such a change, as the 3.x plugin API is released and should
  61 >>> not be broken. I will thus keep the existing `renamepage` as it
  62 >>> is, and call `rename` the global hook I need. --[[intrigeri]]
  63
  64 >>>> Done in my `po` branch. --[[intrigeri]]
  65
  66 I think I see a problem in the rename hook. The hook is called
  67 before the plugin adds any subpages to the set of pages to rename.
  68 So, if the user choses to rename subpages, po will not notice
  69 they are moving, and will not move their po files.
  70
  71 Perhaps the hooks should be moved to come after subpages are added.
  72 This would, though, mean that if the hook somehow decides to add
  73 entirely other pages to the list, their subpages would not be
  74 automatically added.
  75
  76 I also have some qualms about the design of the hook. In particular,
  77 passing the mutable array reference probably makes it impossible
  78 to use from external plugins. Instead it could return any additional
  79 rename hashes it wants to add. Or, if the ability to modify existing
  80 hashes is desired, it could return the full set of hashes.
  81
  82 --[[Joey]]
  83
  84 > I fixed the last part, i.e. a rename hook function now returns the
  85 > full set of hashes. As I also converted it to take named parameters,
  86 > such a function still is passed a reference to the original array,
  87 > though, because one can't build a hash containing an array of hashes
  88 > as a value, without passing this array as a reference.
  89 >
  90 >> Sure.
  91 >
  92 > I'm not entirely sure about your first concern. Calling the hook
  93 > before or after the subpages addition both have their own problems.
  94 >
  95 > What about running the hook before *and* after the subpages
  96 > addition, with an additional `when` named parameter, so that
  97 > a given hook function can choose to act only before or after, or both?
  98 >
  99 > --[[intrigeri]]
 100 >>
 101 >> Have you thought about making the hook be run once *per* file that is
 102 >> selected to be renamed? This would even handle the case where two
 103 >> plugins use the hook; plugin A would see when plugin B adds a new file
 104 >> to be renamed. And the subpage renaming stuff could probably be moved
 105 >> into the rename hook too. --[[Joey]]
 106 >>>
 107 >>> I've implemented this nice solution in my po branch, please review.
 108 >>> I'm slowly coming back to do the last bits needed to get my po and
 109 >>> meta branch merged.  --[[intrigeri]]
 110
 111 >>>> It looks good. I made some small changes to it in my own po branch.
 112 >>>> Nothing significant really. If this were not tied up in the po branch,
 113 >>>> I've have merged it to master already. --[[Joey]]
 114
 115 >>>> Thanks, this is great :) --[[intrigeri]]