[dokuwiki] Re: Task 3 finished, patch attached
- From: Matthias Grimm <matthiasgrimm@xxxxxxxxxxxxxxxxxxxxx>
- To: dokuwiki@xxxxxxxxxxxxx
- Date: Tue, 24 May 2005 16:42:35 +0200
On Mon, 23 May 2005 22:23:50 +0200
Andreas Gohr <andi@xxxxxxxxxxxxxx> wrote:
> Hi!
>
> > > 3. Allow to show more than 20 changes in history
> >
> > Job done. The attached patch adds a 'next' and a 'previous' button
> > to the history page which allows to see more than one page of history
> > data.
>
> I added it, but I have some questions/ideas.
>
> If I understood correctly from skimming the code you call getRecents with -1
> to get all changes. This is IMHO a bad idea because it means to create a
> possibly very large array even if only 20 entries are needed. I think it
> would be better to give an additional optional $first parameter to
> getRecents, add it to $num, and after the loop remove the first $first
> entries from the array again before returning it.
Yes, you are right. I did it this way because this has less impact on
the rest of the code. But if you go along with this changes I will
rework it.
> Maybe the button texts should be changed to "older" and "newer"?
I have no favorite names. We could name them as you like. How about
"<< more recent" and "less recent >>"?
> The $first parameter isn't sanitized! It runs amok if I add some bogus data.
> Try ?do=recent&first=foo to see what I mean. Some is_int or alike should do.
That's a point. I only tested values that are too high. I get into it again.
Best Regards
Matthias
--
DokuWiki mailing list - more info at
http://wiki.splitbrain.org/wiki:mailinglist
Other related posts:
