Here is a preliminary patch. It works only with the
'new.tmpl' template, where I find it the most usefull, but
it can be easily added to other pages too.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Anonymous
-
2001-11-28
Logged In: YES
user_id=104776
Pascal: I'm not sure you should have commited that. It's a
development not patch, not finished, and that doesn't follow
daCode's rules (there are hardcoded URIs and .php3 files and
so on).
Moreover, I don't think the patch is actually useable: the
extra_header() function is misplaced (you need to do one for
EACH type of page if you want it to each page on a site,
which is pretty illogical). This feature is not _so_
important, we have some time to implement it. So instead of
commiting the patch and closing this feature, I think we
should better ponder about how to _cleverly_ and _properly_
add this feature.
The best whould be to remove the patch from the CVS and to
wait for a new one, I think.
'cause, well, if it _works_ we don't need to test it, right
? So we can wait for the final one (or make one by ourselves
if Gal doesn't have the time for this).
And why close the feature request ? the patch isn't finished :)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Anonymous
-
2001-11-28
priority: 5 --> 3
assigned_to: nobody --> netsabes
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I also don't think it should have been commited right away.
You should wait more people with write access to the CVS
approve the patch before commiting.
As far as I did read it, I didn't understand what it does,
and I don't get what's <link> in Mozilla. Is it W3C compliant ?
I would prefer the patch to be removed, and we'll think
about it later. As said on dacode-dev list, I would prefer
to not have anything added in daCode so we could go in a
"freeze" state as soon as possible, to release daCode 1.4
(maybe) for december, 31st.
Maybe is it time to create a 1.4 branch, to go into a freeze
state ?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I commited it because it works and it's easier to improve it
if it is in the CVS.
I agree some things need to be changed and it may not be
finished before the 1.4 release. So the best solution would
be to remove it and reapply it in the dev branch as soon as
a 1.4 branch is created.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Some few answers:
* It is not w3c compliant due to the '/>' ending of tags,
but it'll be if the header in changed to XHTML (or if all />
are replaced by >).
* I find this feature really usefull, it greatly enhance the
usability of sites with a lot of comments like DLFP.
* The onyl hardcoded URI is htp://www.dacode.org, I don't
know if it's avaible in $config, but I find it hardcoded in
other places (such as sidebox.php3). For the hardcoded
'search.php3', you should replace with the correct URI from
sidebox.php3.
* I know 'news.php3' is not the best place to add it, but
the problem is that the nature of the links depends of the
current page. In user list it should be links to next or
previous page of users, in news to the next or previous
news, ... But it's really needed, IMHO, it's in news.php3.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Anonymous
-
2001-11-28
Logged In: YES
user_id=104776
Removed it.
We'll see that later.
Basically, I see the thing like that:
- $html->link_header() (or whatever) in phplib/html.php[3],
which contains <link>'s to search, home (and perhaps
bookmarks, but I hardly see the usage of that).
- $news->link_header() (or whatever again) in news.php[3],
which contains a call to html->link_header(), + the
get_prev_next_news stuff.
- Same thing for tips
- Same thing for _all_ (and I mean _all_, *all*, _*all*_) pages.
And always use $config->php for PHP Extensions.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Here is a second patch. It should be cleaner than the
previous one.
The default link toolbar is here for every page and every
theme (in theory...); and additional links are provided for:
* news
* tips
* messages
I'm open to all suggestions/bug reports.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Use the patch labelled "Second patch" not "Deuxime patch".
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Anonymous
-
2001-12-02
milestone: --> v1.5
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Anonymous
-
2001-12-02
Logged In: YES
user_id=104776
Perhaps you should explain a bit more what your patch does,
since it seems to me that there isn't only changes for the
<link> bar (especially for tips).
I didn't investigate it a lot, but you definetely need to
explain all this stuff.
By the way, AUTHORS is for AUTHORS, CREDITS is for contributors.
Also don't forget to make a proper, comprehensive and clear
ChangeLog entry, not a crappy one like the one in your last
patch.
Admin> deleted the two first patch
Admin> Group -> v1.5
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Sorry for AUTHORS, CREDITS and the ChangeLog, I've changed
this in the new patch.
The only things I've changed not directly related to <link>
is to create functions that returns the URL of the next and
previous page (like in tips). The function is called in the
'extra_header' function and in the normal function
(tips_list for tips). I've done this to avoid having two
times the code that compute the next and previous page.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Anonymous
-
2001-12-02
Logged In: YES
user_id=104776
Updated the patch:
o Cleaned ChangeLog entry
o Cleaned $html->extra_header (cut the lines, removed a slash)
o Moved the line concerning the "Up" <link> to
$html->extraheader, since this is available for all pages.
o Removed the stuff depending from Kilobug's computer
o Fixed $message->do_get_prev_next
o A var was between double quotes in $message->get_prev_next
By the way, I'm not sure that using static variables is
very clever. There may be a better way (same thing for
$message->get_first_last()).
o Added double quotes to the last two args in
$news->get_prev_next_news. Also used simple quotes instead
of double quotes around html tags.
No time to look at the tips part. We'll see that later.
Admin> Added the new patch, removed the third.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Anonymous
-
2001-12-03
Logged In: YES
user_id=104776
Admin> removed patchs 4 & 5. Patch 4 is obsolete, patch 5 is
corrupted.
Admin> will submit a new patch tonight or tomorrow
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Logged In: YES
user_id=54326
Here is a preliminary patch. It works only with the
'new.tmpl' template, where I find it the most usefull, but
it can be easily added to other pages too.
Logged In: YES
user_id=104776
Pascal: I'm not sure you should have commited that. It's a
development not patch, not finished, and that doesn't follow
daCode's rules (there are hardcoded URIs and .php3 files and
so on).
Moreover, I don't think the patch is actually useable: the
extra_header() function is misplaced (you need to do one for
EACH type of page if you want it to each page on a site,
which is pretty illogical). This feature is not _so_
important, we have some time to implement it. So instead of
commiting the patch and closing this feature, I think we
should better ponder about how to _cleverly_ and _properly_
add this feature.
The best whould be to remove the patch from the CVS and to
wait for a new one, I think.
'cause, well, if it _works_ we don't need to test it, right
? So we can wait for the final one (or make one by ourselves
if Gal doesn't have the time for this).
And why close the feature request ? the patch isn't finished :)
Logged In: NO
I also don't think it should have been commited right away.
You should wait more people with write access to the CVS
approve the patch before commiting.
As far as I did read it, I didn't understand what it does,
and I don't get what's <link> in Mozilla. Is it W3C compliant ?
I would prefer the patch to be removed, and we'll think
about it later. As said on dacode-dev list, I would prefer
to not have anything added in daCode so we could go in a
"freeze" state as soon as possible, to release daCode 1.4
(maybe) for december, 31st.
Maybe is it time to create a 1.4 branch, to go into a freeze
state ?
Logged In: YES
user_id=85509
I commited it because it works and it's easier to improve it
if it is in the CVS.
I agree some things need to be changed and it may not be
finished before the 1.4 release. So the best solution would
be to remove it and reapply it in the dev branch as soon as
a 1.4 branch is created.
Logged In: YES
user_id=54326
Some few answers:
* It is not w3c compliant due to the '/>' ending of tags,
but it'll be if the header in changed to XHTML (or if all />
are replaced by >).
* I find this feature really usefull, it greatly enhance the
usability of sites with a lot of comments like DLFP.
* The onyl hardcoded URI is htp://www.dacode.org, I don't
know if it's avaible in $config, but I find it hardcoded in
other places (such as sidebox.php3). For the hardcoded
'search.php3', you should replace with the correct URI from
sidebox.php3.
* I know 'news.php3' is not the best place to add it, but
the problem is that the nature of the links depends of the
current page. In user list it should be links to next or
previous page of users, in news to the next or previous
news, ... But it's really needed, IMHO, it's in news.php3.
Logged In: YES
user_id=104776
Removed it.
We'll see that later.
Basically, I see the thing like that:
- $html->link_header() (or whatever) in phplib/html.php[3],
which contains <link>'s to search, home (and perhaps
bookmarks, but I hardly see the usage of that).
- $news->link_header() (or whatever again) in news.php[3],
which contains a call to html->link_header(), + the
get_prev_next_news stuff.
- Same thing for tips
- Same thing for _all_ (and I mean _all_, *all*, _*all*_) pages.
And always use $config->php for PHP Extensions.
Logged In: YES
user_id=54326
Thank you for your advices. It's my first daCode patch, so
please excuse me for my mistakes.
I'll do it as you say next week (I won't have the time before)
Logged In: YES
user_id=54326
Here is a second patch. It should be cleaner than the
previous one.
The default link toolbar is here for every page and every
theme (in theory...); and additional links are provided for:
* news
* tips
* messages
I'm open to all suggestions/bug reports.
Logged In: YES
user_id=54326
Use the patch labelled "Second patch" not "Deuxime patch".
Logged In: YES
user_id=104776
Perhaps you should explain a bit more what your patch does,
since it seems to me that there isn't only changes for the
<link> bar (especially for tips).
I didn't investigate it a lot, but you definetely need to
explain all this stuff.
By the way, AUTHORS is for AUTHORS, CREDITS is for contributors.
Also don't forget to make a proper, comprehensive and clear
ChangeLog entry, not a crappy one like the one in your last
patch.
Admin> deleted the two first patch
Admin> Group -> v1.5
Logged In: YES
user_id=54326
Sorry for AUTHORS, CREDITS and the ChangeLog, I've changed
this in the new patch.
The only things I've changed not directly related to <link>
is to create functions that returns the URL of the next and
previous page (like in tips). The function is called in the
'extra_header' function and in the normal function
(tips_list for tips). I've done this to avoid having two
times the code that compute the next and previous page.
Logged In: YES
user_id=104776
Updated the patch:
o Cleaned ChangeLog entry
o Cleaned $html->extra_header (cut the lines, removed a slash)
o Moved the line concerning the "Up" <link> to
$html->extraheader, since this is available for all pages.
o Removed the stuff depending from Kilobug's computer
o Fixed $message->do_get_prev_next
o A var was between double quotes in $message->get_prev_next
By the way, I'm not sure that using static variables is
very clever. There may be a better way (same thing for
$message->get_first_last()).
o Added double quotes to the last two args in
$news->get_prev_next_news. Also used simple quotes instead
of double quotes around html tags.
No time to look at the tips part. We'll see that later.
Admin> Added the new patch, removed the third.
Logged In: YES
user_id=104776
Admin> removed patchs 4 & 5. Patch 4 is obsolete, patch 5 is
corrupted.
Admin> will submit a new patch tonight or tomorrow