[PATCH] to the book2menu in forrest skin

[Vadim Gritsenko <va...@verizon.net>]

Hey y'all,

*Please* apply attached patch! Preferably, to the "stable" branch. I'd 
do it myself ;-) but you folks got too many branches around so I'm 
afraid to mess things up!

Thanks,
Vadim

Re: [PATCH] to the book2menu in forrest skin

[David Crossley <cr...@indexgeo.com.au>]

Vadim Gritsenko wrote:
> Hey y'all,
> 
> *Please* apply attached patch! Preferably, to the "stable" branch. I'd 
> do it myself ;-) but you folks got too many branches around so I'm 
> afraid to mess things up!

Applied your patch, thanks.

Erk, i do not know how to do the tag thing, so i just applied to head.
I gather that the 0.5 release will go out soon and i presume that stable
tags are moved along. I don't think that we have branches as such for
"stable", just tags like "stable-20030721".

--David

Re: [PATCH] to the book2menu in forrest skin

[Juan Jose Pablos <ch...@che-che.com>]

David,

I thought you have this working when I was looking on this issue, I did 
not want to duplicate work.

Cheers,
Cheche


David Crossley wrote:
> 
> Hey Forresters, i am working this one to save anyone duplicating work.
> Please use they Issue tracker for bugs and patches.
> Anyway, thanks for the fix.
> 
> --David
> 
> 

Re: [PATCH] to the book2menu in forrest skin

[David Crossley <cr...@indexgeo.com.au>]

Vadim Gritsenko wrote:
> Hey y'all,
> 
> *Please* apply attached patch! Preferably, to the "stable" branch. I'd 
> do it myself ;-) but you folks got too many branches around so I'm 
> afraid to mess things up!

Hey Forresters, i am working this one to save anyone duplicating work.
Please use they Issue tracker for bugs and patches.
Anyway, thanks for the fix.

--David

Re: [PATCH] to the book2menu in forrest skin

[Vadim Gritsenko <va...@verizon.net>]

David Crossley wrote:
> Vadim Gritsenko wrote:
> <snip/>
> 
>>But what really should be done in addition to the patch I sent is to 
>>apply this patch to the other skins, as they have similar problem.
> 
> 
> Okay, done now. 
> 
> Before adding this patch to the other skins i did some testing.
> It seemed that this is already handled properly (because it is
> already present in common/...book2menu.xsl). The hidden menu-items
> were correctly not shown. However, on further investigation i saw
> that some dags were left on the krysalis skin ... there was a little
> triangle symbol where the menu-item used to be. Adding the special
> match for type=hidden to the actual skin fixed it.
> 
> So i am confused, but it works.

You shouldn't be :)

<xsl:template match="menu-item"/> from the skin xslt file overrides the 
<xsl:template match="menu-item[@hidden]"/> from the imported stylesheet. 
That's why you have to add this <xsl:template 
match="menu-item[@hidden]"/> again to the skin xslt. Otherwise, it 
produces empty <li/> elements which appear as these little symbols.

PS I made it change actually for the xindice website. Note how download 
link has been implemented:
http://cvs.apache.org/viewcvs.cgi/xml-xindice/src/documentation/content/xdocs/book.xml?rev=1.6&content-type=text/vnd.viewcvs-markup


Vadim

Re: [PATCH] to the book2menu in forrest skin

[David Crossley <cr...@indexgeo.com.au>]

Vadim Gritsenko wrote:
<snip/>

> But what really should be done in addition to the patch I sent is to 
> apply this patch to the other skins, as they have similar problem.

Okay, done now. 

Before adding this patch to the other skins i did some testing.
It seemed that this is already handled properly (because it is
already present in common/...book2menu.xsl). The hidden menu-items
were correctly not shown. However, on further investigation i saw
that some dags were left on the krysalis skin ... there was a little
triangle symbol where the menu-item used to be. Adding the special
match for type=hidden to the actual skin fixed it.

So i am confused, but it works.

--David

Re: [PATCH] to the book2menu in forrest skin

[Vadim Gritsenko <va...@verizon.net>]

David Crossley wrote:

> David Crossley wrote:
> 
>> David Crossley wrote:

...

> Oops, i had better keep quiet today.

:)

Vadim

Re: [PATCH] to the book2menu in forrest skin

[Juan Jose Pablos <ch...@che-che.com>]

David Crossley wrote:
> Juan Jose Pablos wrote:
> 
>>David,
>>
>>I just check the dtd and the comments from site2book.xsl are wrong, plus 
>>we need to handle the type attribute as well.
>>
>>A menu-item is considerer for display if the type is not hidden.
>>
>>Is that dtd the official one?
> 
> 
> Yes, it is a copy of the one from Cocoon CVS. As i said, it can be
> changed, but that does not really help much because we still need
> to maintain back-compatibility for the type=hidden.
> --David

No, I do not think that we need to change the dtd for this. But 
site2book.xsl should handle the type attribute as well.

Cheers,
Cheche

Re: [PATCH] to the book2menu in forrest skin

[David Crossley <cr...@indexgeo.com.au>]

Juan Jose Pablos wrote:
> David,
> 
> I just check the dtd and the comments from site2book.xsl are wrong, plus 
> we need to handle the type attribute as well.
> 
> A menu-item is considerer for display if the type is not hidden.
> 
> Is that dtd the official one?

Yes, it is a copy of the one from Cocoon CVS. As i said, it can be
changed, but that does not really help much because we still need
to maintain back-compatibility for the type=hidden.
--David

> Cheers,
> Cheche
> 
> David Crossley wrote:
> > Juan Jose Pablos wrote:
> > 
> >>David Croslsey wrote:
> >>
> >>>Oops, i had better keep quiet today. I take that back about being able
> >>>to remove the label attribute from book.xml menu-items - you cannot. 
> >>
> >>can you elaborate a bit more of why you can not remove @label from a 
> >>book.xml?
> > 
> > 
> > DTD validation error, @label is required. We could change
> > that i suppose, as making something optional would not
> > bust any back-compatibility.
> > 
> > However, we would still need to maintain handling anyway,
> > of the type=hidden thing.
> > 
> > --David
> > 

Re: [PATCH] to the book2menu in forrest skin

[Juan Jose Pablos <ch...@che-che.com>]

David,

I just check the dtd and the comments from site2book.xsl are wrong, plus 
we need to handle the type attribute as well.

A menu-item is considerer for display if the type is not hidden.

Is that dtd the official one?

Cheers,
Cheche

David Crossley wrote:
> Juan Jose Pablos wrote:
> 
>>David Croslsey wrote:
>>
>>>Oops, i had better keep quiet today. I take that back about being able
>>>to remove the label attribute from book.xml menu-items - you cannot. 
>>
>>can you elaborate a bit more of why you can not remove @label from a 
>>book.xml?
> 
> 
> DTD validation error, @label is required. We could change
> that i suppose, as making something optional would not
> bust any back-compatibility.
> 
> However, we would still need to maintain handling anyway,
> of the type=hidden thing.
> 
> --David
> 

Re: [PATCH] to the book2menu in forrest skin

[David Crossley <cr...@indexgeo.com.au>]

Juan Jose Pablos wrote:
> David Croslsey wrote:
> >
> > Oops, i had better keep quiet today. I take that back about being able
> > to remove the label attribute from book.xml menu-items - you cannot. 
> 
> can you elaborate a bit more of why you can not remove @label from a 
> book.xml?

DTD validation error, @label is required. We could change
that i suppose, as making something optional would not
bust any back-compatibility.

However, we would still need to maintain handling anyway,
of the type=hidden thing.

--David

Re: [PATCH] to the book2menu in forrest skin

[Juan Jose Pablos <ch...@che-che.com>]

David,

> 
> 
> Oops, i had better keep quiet today. I take that back about being able
> to remove the label attribute from book.xml menu-items - you cannot.
> 
> --David
> 

can you elaborate a bit more of why you can not remove @label from a 
book.xml?

Cheers,
Cheche

Re: [PATCH] to the book2menu in forrest skin

[David Crossley <cr...@indexgeo.com.au>]

David Crossley wrote:
>  David Crossley wrote:
> > Juan Jose Pablos wrote:
> > > There is not need to add a new attribute called type if a menu-itme is 
> > > hidden when there is not label.
> > > 
> > > This is the actual behavior that I found when I was looking at this 
> > > issue, what I would like you to do is to test if this suit your needs.
> > > 
> > > Can you remove a couple of labels from a site.xml and see if that works 
> > > for you?
> > 
> > Reminder to self: "use more words" ...
> > > >>David Crossley wrote:
> > > >>> Well i was adding Vadim's patch to handle the old book.xml menus.
> > > >>> I tested it using Cocoon's docs and everything worked fine.
> > 
> > Cocoon does not (yet) have a site.xml for their docs, so we need
> > to ensure that the old book.xml behaviour works properly. It does,
> > with Vadim's patch.
> > 
> > By the way, i tested another project that does have a site.xml
> > and the approach with missing label attr does work too. Nice.
> 
> Erk, another note to self: "re-read what people are saying" ...
> 
> There is another behaviour whereby you can remove the label attribute
> from *book.xml* menu-items so as to hide them. It does work too.
> 
> Thanks Cheche, now i understand.

Oops, i had better keep quiet today. I take that back about being able
to remove the label attribute from book.xml menu-items - you cannot.

--David

Re: [PATCH] to the book2menu in forrest skin

[David Crossley <cr...@indexgeo.com.au>]

 David Crossley wrote:
> Juan Jose Pablos wrote:
> > There is not need to add a new attribute called type if a menu-itme is 
> > hidden when there is not label.
> > 
> > This is the actual behavior that I found when I was looking at this 
> > issue, what I would like you to do is to test if this suit your needs.
> > 
> > Can you remove a couple of labels from a site.xml and see if that works 
> > for you?
> 
> Reminder to self: "use more words" ...
> > >>David Crossley wrote:
> > >>> Well i was adding Vadim's patch to handle the old book.xml menus.
> > >>> I tested it using Cocoon's docs and everything worked fine.
> 
> Cocoon does not (yet) have a site.xml for their docs, so we need
> to ensure that the old book.xml behaviour works properly. It does,
> with Vadim's patch.
> 
> By the way, i tested another project that does have a site.xml
> and the approach with missing label attr does work too. Nice.

Erk, another note to self: "re-read what people are saying" ...

There is another behaviour whereby you can remove the label attribute
from *book.xml* menu-items so as to hide them. It does work too.

Thanks Cheche, now i understand.

--David

Re: [PATCH] to the book2menu in forrest skin

[Juan Jose Pablos <ch...@che-che.com>]

David,

I am sorry if I did not express myself clear enough, It was too late I 
guess.

I wanted to give users a hint of where to use this new type and add this 
  new type on to a menu on the fresh-site Then I found this on 
site2book.xsl:

       <!-- No label, abandon the whole subtree -->
       <xsl:when test="not(@label)">
       </xsl:when>
       <!-- Below here, everything has a label, and is therefore 
considered "for display" -->

       <!-- No children -> must be a menu item -->
       <!-- Has children, but they are not for display -> menu item -->

So I drop an e-mail to verify with Vadim if when on book.xml you have 
not got a label on a menu item gives the same behaviour.

David Crossley wrote:
> 
> Cocoon does not (yet) have a site.xml for their docs, so we need
> to ensure that the old book.xml behaviour works properly. It does,
> with Vadim's patch.
> 

I did not know where it was going to be applied.

> By the way, i tested another project that does have a site.xml
> and the approach with missing label attr does work too. Nice.

so if produce the same result we should be using only one approach: 
Either @label or @type="hidden"



Cheers,
Cheche

Re: [PATCH] to the book2menu in forrest skin

[David Crossley <cr...@indexgeo.com.au>]

Juan Jose Pablos wrote:
> There is not need to add a new attribute called type if a menu-itme is 
> hidden when there is not label.
> 
> This is the actual behavior that I found when I was looking at this 
> issue, what I would like you to do is to test if this suit your needs.
> 
> Can you remove a couple of labels from a site.xml and see if that works 
> for you?

Reminder to self: "use more words" ...
> >>David Crossley wrote:
> >>> Well i was adding Vadim's patch to handle the old book.xml menus.
> >>> I tested it using Cocoon's docs and everything worked fine.

Cocoon does not (yet) have a site.xml for their docs, so we need
to ensure that the old book.xml behaviour works properly. It does,
with Vadim's patch.

By the way, i tested another project that does have a site.xml
and the approach with missing label attr does work too. Nice.

--David

Re: [PATCH] to the book2menu in forrest skin

[Juan Jose Pablos <ch...@che-che.com>]

Vadim

There is not need to add a new attribute called type if a menu-itme is 
hidden when there is not label.

This is the actual behavior that I found when I was looking at this 
issue, what I would like you to do is to test if this suit your needs.

Can you remove a couple of labels from a site.xml and see if that works 
for you?

Cheers,
cheche



Vadim Gritsenko wrote:
> Juan Jose Pablos wrote:
> 
>> David,
>>
>>> Well i was adding Vadim's patch to handle the old book.xml menus.
>>> I tested it using Cocoon's docs and everything worked fine.
>>>
>>> --David
>>>
>>
>> So we need to go use same way for both, what do you thin if we do a:
>>
>> <xsl:template match="menu-item[@label]"/>
> 
> 
> Why would you do that? There is already template <xsl:template 
> match="menu-item"/> which will cover all normal usage scenarios.
> 
> <xsl:template match="menu-item[@type='hidden']"/> is required to ignore 
> hidden links which previous stylesheet failed to do.
> 
> 
> But what really should be done in addition to the patch I sent is to 
> apply this patch to the other skins, as they have similar problem.
> 
> Vadim
> 

Re: [PATCH] to the book2menu in forrest skin

[Vadim Gritsenko <va...@verizon.net>]

Juan Jose Pablos wrote:

> David,
> 
>> Well i was adding Vadim's patch to handle the old book.xml menus.
>> I tested it using Cocoon's docs and everything worked fine.
>>
>> --David
>>
> 
> So we need to go use same way for both, what do you thin if we do a:
> 
> <xsl:template match="menu-item[@label]"/>

Why would you do that? There is already template <xsl:template 
match="menu-item"/> which will cover all normal usage scenarios.

<xsl:template match="menu-item[@type='hidden']"/> is required to ignore 
hidden links which previous stylesheet failed to do.


But what really should be done in addition to the patch I sent is to 
apply this patch to the other skins, as they have similar problem.

Vadim

Re: [PATCH] to the book2menu in forrest skin

[Juan Jose Pablos <ch...@che-che.com>]

David,

> Well i was adding Vadim's patch to handle the old book.xml menus.
> I tested it using Cocoon's docs and everything worked fine.
> 
> --David
> 

So we need to go use same way for both, what do you thin if we do a:

<xsl:template match="menu-item[@label]"/>

Cheers,
Cheche

Re: [PATCH] to the book2menu in forrest skin

[David Crossley <cr...@indexgeo.com.au>]

Jeff Turner wrote:
> Juan Jose Pablos wrote:
> > Vadim,
> > 
> > Your patch needs a bit more than that to work.  site2book.xsl needs to 
> > be modified as well.
> > 
> > would you mind test if a site element without label element does the 
> > same job that you want to use?
> 
> It does, I think..
> 
> --Jeff

Well i was adding Vadim's patch to handle the old book.xml menus.
I tested it using Cocoon's docs and everything worked fine.

--David

Re: [PATCH] to the book2menu in forrest skin

[Jeff Turner <je...@apache.org>]

On Tue, Sep 09, 2003 at 01:13:04AM +0200, Juan Jose Pablos wrote:
> Vadim,
> 
> Your patch needs a bit more than that to work.  site2book.xsl needs to 
> be modified as well.
> 
> would you mind test if a site element without label element does the 
> same job that you want to use?

It does, I think..

--Jeff

> Cheers,
> Cheche
> 
> 
> Vadim Gritsenko wrote:
> >Hey y'all,
> >
> >*Please* apply attached patch! Preferably, to the "stable" branch. I'd 
> >do it myself ;-) but you folks got too many branches around so I'm 
> >afraid to mess things up!
> >
> >Thanks,
> >Vadim
> 

Re: [PATCH] to the book2menu in forrest skin

[Juan Jose Pablos <ch...@che-che.com>]

Vadim,

Your patch needs a bit more than that to work.  site2book.xsl needs to 
be modified as well.

would you mind test if a site element without label element does the 
same job that you want to use?

Cheers,
Cheche


Vadim Gritsenko wrote:
> Hey y'all,
> 
> *Please* apply attached patch! Preferably, to the "stable" branch. I'd 
> do it myself ;-) but you folks got too many branches around so I'm 
> afraid to mess things up!
> 
> Thanks,
> Vadim