[dokuwiki] Re: New changelog system
- From: Ben Coburn <btcoburn@xxxxxxxxxxxxx>
- To: dokuwiki@xxxxxxxxxxxxx
- Date: Sun, 3 Sep 2006 09:42:40 -0700
On Sep 3, 2006, at 2:17 AM, Andreas Gohr wrote:
Hi Ben and interested ones!
I just took a deeper look at the new changelog system and as promised
here are my nitpicks. But first: I absolutely love it. It's really
elegant. Good work!
Here are my few minor points:
set_time_limit should be replaced by @set_time_limit to supress any
errors
Ok.
I like it how the changelog conversion is implemented as action plugin.
One minor thing I'd add: the plugin should try to disable itself after
finishing the conversion. There is no need to instanciate the whole
plugin when it does nothing anymore. This of course only works if the
plugin directory is writable, so in addition the ?do=check command
should check if the import was finished but the plugin is still enabled
and issue an warning about it.
Sounds good, but how do users re-enable it if they need to re-run the
import. I put 'importoldchangelog' in the $plugin_protected list of the
plugin manager. This disables all the plugin manager form elements for
the 'importoldchangelog' plugin. Should the plugin manager be changed
to allow protected plugins that are disabled to be re-enabled?
You use filectime in some places which is probably not what you want:
"In most Unix filesystems, a file is considered changed when its
inode
data is changed"
It should be replaced with filemtime which also works on Windows
systems.
No, this is exactly what I want. I use this when checking if the recent
changelog cache needs to be trimmed. The cache is updated on every page
edit so filemtime is useless (for this purpose). To ensure that the
inode change time reflects the time when the recent changelog cache was
last trimmed, I explicitly unlink the file before saving the newly
trimmed version.
The filectime call in addLogEntry was already in the code elsewhere
(saveMetaData). It is used to get the "creation date" of the newly
created file.
In both cases filectime should return the time the file was created.
This could only be different if a third-party program was messing with
the inodes of the files in Dokuwiki's data directory.... but then there
would be other worse problems to worry about. The only comment I see
about Windows is that on Win32 filectime really is a creation time,
which is not a problem here.
All calls to unlink() should be prefixed with an @ to suppress possible
errors. I usually do the same for file_exists to compensate for a bug
in
one certain PHP version where this prints an error as well.
Ok, I'll adjust those.
You're using a tab as delimiter in the changelog but don't filter the
log entries (summary) and malicious person could possibly break the
changelog by inserting newlines or tabs into the change summary. Or did
I miss something?
Your right. (I think I got caught thinking tab could not be entered
from the browser, but from curl it's another story...) I'll strip tabs
and newlines out of the 'type', 'sum', and 'extra' fields. This will
prevent both users and programmers from shooting themselves in the
foot!
Okay and that's it as you can see, it's just minor things :-) Would be
great if you could send a patch for these things.
In addition we should check if there needs to be anything done on the
commandline tools in the bin directory.
I thought I would have to patch this too, but the addLogEntry call is
in saveWikiText so there is nothing to do.
On Sep 3, 2006, at 3:09 AM, Andreas Gohr wrote:
Just another addition: Maybe we should move all the changelog code
from common.inc to it's own file?
Well, 'inc/common.php' is getting large but these function always need
to be included. I think this is just a project management issue, so
it's your call Andi.
Regards, Ben Coburn
-------------------
silicodon.net
-------------------
--
DokuWiki mailing list - more info at
http://wiki.splitbrain.org/wiki:mailinglist
Other related posts:
