X-Git-Url: https://sipb.mit.edu/gitweb.cgi/ikiwiki.git/blobdiff_plain/60ed6c9d16c4dad3cd54dff539edc43efa4a012f..03881f6b1af9c9ec8393ed5842b87778c346dae2:/doc/bugs/removal_of_transient_pages.mdwn diff --git a/doc/bugs/removal_of_transient_pages.mdwn b/doc/bugs/removal_of_transient_pages.mdwn index 67fa886b9..4843b5900 100644 --- a/doc/bugs/removal_of_transient_pages.mdwn +++ b/doc/bugs/removal_of_transient_pages.mdwn @@ -17,3 +17,57 @@ pages, until this is fixed. --[[Joey]] >> --[[Joey]] >>> Yes. It's not entirely clear to me why that's there... --s + +>>>> Yeah, 2461ce0de6231bfeea4d98c86806cdbb85683297 doesn't really +>>>> say, and I tend to assume that when I've written paranoid code +>>>> it's there for a reason. I think that here the concern was that +>>>> the file might be in some underlay that the user should not be able +>>>> to affect by web edits. The `-f` check seems rather redundant, +>>>> surely if it's in `%pagesources` ikiwiki has already verified it's +>>>> safe. --[[Joey]] + +---- + +[[!template id=gitbranch branch=smcv/ready/transient-rm author="[[Simon McVittie|smcv]]"]] + +Here's a branch. It special-cases the `$transientdir`, but in such a way +that the special case could easily be extended to other locations where +deletion should be allowed. + +It also changes `IkiWiki::prune()` to optionally stop pruning empty +parent directories at the point where you'd expect it to (for instance, +previously it would remove the `$transientdir` itself, if it turns out +to be empty), and updates callers. + +The new `prune` API looks like this: + + IkiWiki::prune("$config{srcdir}/$file", $config{srcdir}); + +with the second argument optional. I wonder whether it ought to look +more like `writefile`: + + IkiWiki::prune($config{srcdir}, $file); + +although that would be either an incompatible change to internal API +(forcing all callers to update to 2-argument), or being a bit +inconsistent between the one-and two-argument forms. Thoughts? + +--[[smcv]] + +> I've applied the branch as-is, so this bug is [[done]]. +> `prune` is not an exported API so changing it would be ok.. +> I think required 2-argument would be better, but have not checked +> all the call sites to see if the `$file` is available split out +> as that would need. --[[Joey]] + +[[!template id=gitbranch branch=smcv/ready/prune author="[[Simon McVittie|smcv]]"]] + +>> Try this, then? I had to make some changes to `attachment` +>> to make the split versions available. I suggest reviewing +>> patch-by-patch. +>> +>> I also tried to fix a related bug which I found while testing it: +>> the special case for renaming held attachments didn't seem to work. +>> (`smcv/wip/rename-held`.) Unfortunately, it seems that with that +>> change, the held attachment is committed to the `srcdir` when you +>> rename it, which doesn't seem to be the intention either? --[[smcv]]