________________________________________
From: Daniel Dekany <[email protected]>
Sent: Tuesday, October 27, 2015 3:41 AM
To: Pradeep Murugesan
Subject: Re: Adding a new BuiltIn - previousSibling
Monday, October 26, 2015, 3:01:56 PM, Pradeep Murugesan wrote:
> HI Daniel,
>
> You got a chance to review this ?
Sorry, I haven't forgotten it, really, it's just that I'm "bit"
overburden... See other mail.
I have forgotten to mention there, but of course eventually we need
JUnit tests too.
- sure I will cover them.
> I have now added a datamodel for the template in the page
> "incubator-freemarker/build/manual/dgui_quickstart_template.html".
> Template block containing the sentence "Welcome ${user}<#if user ==
> "Big Joe">, our beloved leader</#if>!".
>
> I have pushed the code both
> freemarker(https://github.com/pradeepmurugesan/incubator-freemarker/)
> and
> freemarker-docgen(https://github.com/pradeepmurugesan/incubator-freemarker-docgen).
Um... I don't see your commits there.
- I am sorry. Forgot to mention the branch.
https://github.com/pradeepmurugesan/incubator-freemarker/tree/FM-OnlineConnect
https://github.com/pradeepmurugesan/incubator-freemarker-docgen/tree/FM-OnlineConnect
> When you find time Kindly check and let me know if we can proceed?
>
> Pradeep.
>
>
>
> ________________________________________
> From: Pradeep Murugesan <[email protected]>
> Sent: Saturday, October 24, 2015 1:39 AM
> To: [email protected]
> Subject: RE: Adding a new BuiltIn - previousSibling
>
> Hi Daniel,
>
> https://github.com/pradeepmurugesan/incubator-freemarker/commit/465ed1bd768e8a5bee91bea7d3b291a5872efae5
> I have added the builtIns which will return blindly the previous
> and next sibling and also the special variables @@previous and
> @@next which will return the valid node. In the special variable
> case I have used the xpath to get the required nodes.
> Kindly review and let me know your thoughts.
> Pradeep.
>> Date: Sun, 18 Oct 2015 11:42:04 +0200
>> From: [email protected]
>> To: [email protected]
>> Subject: Re: Adding a new BuiltIn - previousSibling
>>
>> Returning the sibling node without skipping stuff is not XML-specific,
>> so certainly that should be ?previous (and a new method in the new
>> TemplateNodeModelEx interface), not a hash key that starts with "@".
>>
>> BTW, of course all of these has an opposite direction variant, like
>> "@next". And "@prev" may should be "@previous".
>>
>> --
>> Thanks,
>> Daniel Dekany
>>
>>
>> Sunday, October 18, 2015, 5:31:50 AM, Pradeep Murugesan wrote:
>>
>> > yeah makes sense..
>> > so we need to return a valid element node he is looking for
>> > skipping all the whitespace, CDATA etc...
>> > I am wondering if the user will have any reason to look for a CDATA
>> > sibling or any non element sibling which we will skip.
>> > In that case can we have 2 special cases.
>> > 1. @prev which will return the immediate sibling2. @prevNode or
>> > something intutive which will return a valid element skipping few .
>> > Pradeep.
>> >> Date: Sat, 17 Oct 2015 20:15:57 +0200
>> >> From: [email protected]
>> >> To: [email protected]
>> >> Subject: Re: Adding a new BuiltIn - previousSibling
>> >>
>> >> Saturday, October 17, 2015, 7:09:49 PM, Pradeep Murugesan wrote:
>> >>
>> >> > hmm then I think @@prev should return the immediate sibling with the
>> >> > following issues/advantages.
>> >> > 1. In xml doc its a overhead for user to call it twice to get the
>> >> > previous element node2.
>> >>
>> >> It much worse than that, if it just returns the previous sibling on
>> >> the DOM, as you can't know if you have to call it once, twice, 3
>> >> times, etc.
>> >>
>> >> > For less document centric it is not a problem.
>> >>
>> >> For non-document XML it's similarly desirable. I meant JSON and such,
>> >> where @@prev doesn't exist anyway...
>> >>
>> >> > 3. for Non-normalized dom we wont do anything before they normalize it .
>> >>
>> >> Actually, we can do a little effort... skipping *all* the
>> >> white-space-only character date nodes and comments and PI-s. But
>> >> that's all.
>> >>
>> >> > Let me know If I got that correctly.
>> >> > If so I will add @@prev as a special case and use
>> >> > .node.@@prev.@@prev to get to theprevious sibling node.
>> >>
>> >> You mean, you will use: .node.@@prev
>> >>
>> >> > Pradeep.
>> >> >
>> >> >> Date: Fri, 16 Oct 2015 01:09:36 +0200
>> >> >> From: [email protected]
>> >> >> To: [email protected]
>> >> >> Subject: Re: Adding a new BuiltIn - previousSibling
>> >> >>
>> >> >> Thursday, October 15, 2015, 10:44:10 PM, Pradeep Murugesan wrote:
>> >> >>
>> >> >> > Hi Daniel,
>> >> >> > So you are saying we need to have it that way and leave the
>> >> >> > responsibility to the caller. Lets say in case of us to get to check
>> >> >> > if template is preceded by formDataModel we will do the follwing ?
>> >> >> > <#local siblingElement = .node.@@prev.@@prev>
>> >> >> > then check the role attribute of siblingElement ?
>> >> >> > I assume the semantic for @@prev should return the immediate
>> >> >> > sibling being it a whitespace , CDATA or \n as in our case.
>> >> >> > Let me know your thoughts.
>> >> >>
>> >> >> I think that in almost all cases the user means the previous DOM node
>> >> >> ignoring white-space nodes and comments, and certainly PI-s too.
>> >> >> (That's also why ?previous or such wouldn't solve the problem you ran
>> >> >> into, while it can be still very useful in some other applications,
>> >> >> like where the tree is not from XML but something less
>> >> >> document-centric.) (Non-normalized DOM-s, like one with sibling cdata
>> >> >> nodes, could also complicate what we need, but I belive that such
>> >> >> cases can only be addressed reasonably be ensuring that the whole DOM
>> >> >> is normalized before we do anything with it... so it doesn't mater
>> >> >> now.)
>> >> >>
>> >> >> > Pradeep.
>> >> >> >> Date: Thu, 15 Oct 2015 20:32:33 +0200
>> >> >> >> From: [email protected]
>> >> >> >> To: [email protected]
>> >> >> >> Subject: Re: Adding a new BuiltIn - previousSibling
>> >> >> >>
>> >> >> >> Thursday, October 15, 2015, 4:13:18 PM, Pradeep Murugesan wrote:
>> >> >> >>
>> >> >> >> > HI Daniel,
>> >> >> >> > Its not preceeded by white spaces but "\n" is taken as sibling.
>> >> >> >>
>> >> >> >> \n is whitespace, and it's a sibling in XML.
>> >> >> >>
>> >> >> >> > In book.xml <programlisting role="formDataModel">dsadsd fdfsdfdsf
>> >> >> >> > dfds</programlisting>
>> >> >> >> > <programlisting role="template"><#if cargo.weight <
>> >> >> >> > <emphasis>100</emphasis>>Light
>> >> >> >> > cargo</#if></programlisting>
>> >> >> >> > I am trying to get the programlisting with role formDataModel as
>> >> >> >> > previousSibling. But the "\n" is returned as the sibling. To
>> >> >> >> > confirm
>> >> >> >> > the same I just checked it with
>> >> >> >> > node.previousSibling().previousSibling() and I am able to get to
>> >> >> >> > formDataModel.
>> >> >> >> > What should we need to do for this here ?
>> >> >> >>
>> >> >> >> Nothing... it's correct that way. it's that you want the sibling
>> >> >> >> *element*, as I said. Actually, it's a bit trickier than that. You
>> >> >> >> want to get the sibling element, unless the interfering character
>> >> >> >> data
>> >> >> >> is non-whitespace. Because, if you have <a/>cdata<b/>, then surely
>> >> >> >> you
>> >> >> >> don't want to say that <b/> is preceded bu <a/>, but "cdata".
>> >> >> >>
>> >> >> >> > I have also added a key with @@prev in ElementModel and that
>> >> >> >> > works fine.
>> >> >> >>
>> >> >> >> So what exactly is the semantic of @@prev?
>> >> >> >>
>> >> >> >> > Pradeep.
>> >> >> >> >> Date: Wed, 14 Oct 2015 22:32:40 +0200
>> >> >> >> >> From: [email protected]
>> >> >> >> >> To: [email protected]
>> >> >> >> >> Subject: Re: Adding a new BuiltIn - previousSibling
>> >> >> >> >>
>> >> >> >> >> I'm not sure what's improper in the result (I don't know what was
>> >> >> >> >> expected). Isn't that node preceded by white space? That would
>> >> >> >> >> explain
>> >> >> >> >> it. You might rather want the previous *element*. But that will
>> >> >> >> >> be
>> >> >> >> >> difficult to express on the TemplateNodeModel level, which is not
>> >> >> >> >> bound to XML.
>> >> >> >> >>
>> >> >> >> >> One important point is that you can't add new methods to
>> >> >> >> >> TemplateNodeModel, as that breaks backward compatibility. It can
>> >> >> >> >> only
>> >> >> >> >> be added to a new sub-interface, like TemplateNodeModelEx. But
>> >> >> >> >> even
>> >> >> >> >> that won't solve getting the sibling element node.
>> >> >> >> >>
>> >> >> >> >> So another approach is instead of adding a built-in, adding a new
>> >> >> >> >> special key that's specific to freemarker.ext.dom models, like
>> >> >> >> >> "@@prev" and "@@next".
>> >> >> >> >>
>> >> >> >> >> --
>> >> >> >> >> Thanks,
>> >> >> >> >> Daniel Dekany
>> >> >> >> >>
>> >> >> >> >>
>> >> >> >> >> Wednesday, October 14, 2015, 9:10:25 PM, Pradeep Murugesan wrote:
>> >> >> >> >>
>> >> >> >> >> > Hi Daniel,
>> >> >> >> >> > I tried to add a new built in & of course it DIDN'T work �.
>> >> >> >> >> > I did the following.
>> >> >> >> >> > 1. added putBI("previousSibling", new previousSiblingBI()); in
>> >> >> >> >> > BuiltIn.java2. added a static class in BuiltInForNodes.java
>> >> >> >> >> > static
>> >> >> >> >> > class previousSiblingBI extends BuiltInForNode {
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> > @Override
>> >> >> >> >> > TemplateModel calculateResult(TemplateNodeModel
>> >> >> >> >> > nodeModel,
>> >> >> >> >> > Environment env) throws TemplateModelException {
>> >> >> >> >> > return nodeModel.getPreviousSibling();
>> >> >> >> >> > }
>> >> >> >> >> > }
>> >> >> >> >> > 3. added a method in Interface TemplateNodeModel.java
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> > TemplateNodeModel getPreviousSibling() throws
>> >> >> >> >> > TemplateModelException;
>> >> >> >> >> > 4. In package freemarker.ext.dom's NodeModel added the
>> >> >> >> >> > following method
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> > public TemplateNodeModel getPreviousSibling() { Node
>> >> >> >> >> > previousSibling = node.getPreviousSibling();
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> > return wrap(previousSibling);}
>> >> >> >> >> > Once this is done I tried to access it as .node?previousSibling
>> >> >> >> >> > from template and it reached till the NodeModel class i
>> >> >> >> >> > defined in
>> >> >> >> >> > the 4th step. But the returned previousSibling is not proper.
>> >> >> >> >> > It's
>> >> >> >> >> > not returning the programListingNode with formDataModel
>> >> >> >> >> > instead returns someother node.
>> >> >> >> >> > I tried to log the node returned and I got the following o/p
>> >> >> >> >> > [docgen:transform] [#text:
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> > [docgen:transform] ]
>> >> >> >> >> > I clearly understand the implementation of getPreviousSibling
>> >> >> >> >> > is
>> >> >> >> >> > not proper, but I couldn't figure out where we have
>> >> >> >> >> > implemented the same.
>> >> >> >> >> > Please advise.
>> >> >> >> >> > Pradeep.
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >>
>> >> >> >> >
>> >> >> >>
>> >> >> >> --
>> >> >> >> Thanks,
>> >> >> >> Daniel Dekany
>> >> >> >>
>> >> >> >
>> >> >>
>> >> >> --
>> >> >> Thanks,
>> >> >> Daniel Dekany
>> >> >>
>> >> >
>> >>
>> >> --
>> >> Thanks,
>> >> Daniel Dekany
>> >>
>> >
>>
--
Thanks,
Daniel Dekany
