Wednesday, December 9, 2015, 10:11:04 AM, Pradeep Murugesan wrote:
> Daniel,
>
> you got a chance to review this ?
>
> Pradeep.
>
> ________________________________________
> From: Pradeep Murugesan <[email protected]>
> Sent: Monday, December 7, 2015 10:15 AM
> To: Daniel Dekany
> Cc: [email protected]
> Subject: Re: Adding a new BuiltIn - previousSibling
>
> Hi daniel,
>
> I have a question on the @@previous and @@next being null. So we
> will return the previous significant node if exists but will return
> an empty set of nodes if its null. which means we will return a
> NodeListModel with an empty ArrayList.
Yes.
> In that case shouldn't we be wrapping the non null node too in
> NodeListModel instead of NodeModel ?
>
> Right now the code might look like this
>
> if(previousSibling == null) {
> return new NodeListModel(EMPTY_ARRAYLIST, null);
> } else {
> return wrap(previousSibling);
> }
Looks OK to me.
> we need to return one dataType right ? it should be like
>
> if(previousSibling == null) {
> return new NodeListModel(EMPTY_ARRAYLIST, null);
> } else {
> return NodeListModel(previousSibling);
> }
>
> Let me know your inputs.
NodeModel-s (like ElementModel) implement TemplateSequenceModel, just
like NodeListModel does, so as far as the template is concerned, they
are both list-like. The main difference is that a NodeModel can only
represent a node sequence of size 1, while NodeListModel can represent
a node sequence of arbitrary size. When your node sequence happens to
be of size 1, you should always use NodeModel instead of
NodeListModel, because only NodeModel-s implement TemplateScalarModel
and so can be treated as a single strings in the template.
I have to add though that the DOM wrapper is a part of the code that
I'm not familiar with, and that wasn't cleaned up by me either. So
watch out.
> Pradeep
>
> ________________________________________
> From: Daniel Dekany <[email protected]>
> Sent: Monday, December 7, 2015 4:05 AM
> To: Pradeep Murugesan
> Cc: [email protected]
> Subject: Re: Adding a new BuiltIn - previousSibling
>
> Sunday, December 6, 2015, 4:28:11 PM, Pradeep Murugesan wrote:
>
>>
>> Hi Daniel,
>>
>> sorry for this huge gap.. Actually got caught up in Chennai
>> floods ( Chennai, India). Just back to home town and safe.
>>
>> I have done the unit tests and the renaming of the files you
>> suggested previously. Please review and let me know the changes.
>>
>> https://github.com/pradeepmurugesan/incubator-freemarker/commit/1db672a2ba3db1f08c594df663b4dd7e68d36d4a
>
> One random detail that I have spotted is:
> node.getTextContent().trim().isEmpty(). It's not very efficient if you
> think about it. Something like StringUtil.isTrimmableToEmpty would be
> better, only with String argument of course.
>
>> I need to cover a case for which I need your inputs.
>>
>> Lets say we are in the last sibling and trying to access the next,
>> same applies for previous as well what should we return ? null ? Kindly let
>> me know your thoughts.
>
> "?previous" and "?next" should just return null. But "@@previous" and
> "@@next" should behave like the other "@@" keys, that is, with XPath
> logic, which says that the result is an empty set of nodes. Again
> similarly to other "@@" keys and XPath expression, they should work
> correctly on node sets that contains 0 or multiple nodes.
>
> --
> Thanks,
> Daniel Dekany
>
>> Pradeep.
>> ________________________________________
>> From: Daniel Dekany <[email protected]>
>> Sent: Saturday, November 21, 2015 2:34 AM
>> To: Pradeep Murugesan
>> Cc: [email protected]
>> Subject: Re: Adding a new BuiltIn - previousSibling
>>
>> Friday, November 20, 2015, 8:51:31 AM, Pradeep Murugesan wrote:
>>
>>> Hi Daniel,
>>>
>>> Took a long break due to some personal reasons. Sorry for the same. I have
>>> a question in your email.
>>>
>>> What do you mean by
>>>
>>> "Also I guess inside the testPreviousSibling you don't really need
>>> output capturing, nor ?trim. "
>>>
>>> I am not sure what you are coming to say there. We need to assert somehow
>>> the expected o/p right ?
>>>
>>> so we can't assert against empty spaces since we don't know how
>>> many spaces , So I thought of asserting the same after trimming the o/p.
>>
>> We don't need capturing for sure, I guess you see that now. As of
>> trimming, that's a minor issue really, but in fact we know how many
>> spaces are there, since we provide the XML.
>>
>>> Let me know if I am missing something.
>>>
>>> Pradeep.
>>> ________________________________________
>>> From: Daniel Dekany <[email protected]>
>>> Sent: Tuesday, November 10, 2015 2:44 AM
>>> To: Pradeep Murugesan
>>> Subject: Re: Adding a new BuiltIn - previousSibling
>>>
>>> Tuesday, November 3, 2015, 7:19:17 AM, Pradeep Murugesan wrote:
>>>
>>>> Hi Daniel,
>>>>
>>>> I have made the changes you have said and writing unit tests. I
>>>> have written an unit test and need to check whether can I proceed in
>>>> the same fashion. One important question I have is accessing the
>>>> (XML) datamodel required for the testcase.
>>>>
>>>> Now I am overriding the function getDataModel() and read the xml
>>>> from a file. Kindly let me know if that is acceptable.
>>>
>>> You don't need to override getDateModel(). Just add "doc" to the data
>>> model with the TemplateTest.addToDataModel.
>>>
>>> Loading the XML file via java.io.File API is not entirely correct,
>>> especially not with that relative path ("build/test-classes/..."). You
>>> don't know what the current directory will be on the CI server for
>>> example. Also, though an extreme case, but it can also occur that a
>>> test suite is ran from an unexploded jar (i.e., you don't even have
>>> real files anywhere). Just like outside tests, the correct solution is
>>> using Class.getResource or Class.getResourceAsStream to read
>>> class-loader resources.
>>>
>>> Also I guess inside the testPreviousSibling you don't really need
>>> output capturing, nor ?trim.
>>>
>>>> https://github.com/pradeepmurugesan/incubator-freemarker/commit/42132df19b6f8e53f66ff3f6cbbce459376c65a6
>>>>
>>>>
>>>> P.S : I have removed the author name in next commit. Intellij adds
>>>> it and I am missing it everytime. Sorry!!.
>>>>
>>>> Pradeep.
>>>>
>>>>
>>>>
>>>> ________________________________________
>>>> From: Pradeep Murugesan <[email protected]>
>>>> Sent: Thursday, October 29, 2015 7:46 AM
>>>> To: [email protected]
>>>> Subject: Re: Adding a new BuiltIn - previousSibling
>>>>
>>>> oh now I got it.
>>>>
>>>> So we can also expect something like
>>>> <a/> there is some text here <b/>
>>>>
>>>> Now when the user do a @@previous on node 'b' he will get node 'a'
>>>> but he might expect "there is some text here" which is still a valid text
>>>> node.
>>>>
>>>> I thought there can be no such scenario so kept hanging on with
>>>> blindly skipping all till we get a node. So I will do the following .
>>>>
>>>> 1. rename to @@previous_significant
>>>> 2. skip the siblings when its in any of the blacklisted candidates.
>>>> ( whitespaces, CDATA, \n(ofcourse))
>>>>
>>>> Pradeep.
>>>>
>>>> ________________________________________
>>>> From: Daniel Dekany <[email protected]>
>>>> Sent: Thursday, October 29, 2015 4:12 AM
>>>> To: Pradeep Murugesan
>>>> Subject: Re: Adding a new BuiltIn - previousSibling
>>>>
>>>> Wednesday, October 28, 2015, 6:21:19 PM, Pradeep Murugesan wrote:
>>>>
>>>>> Hi Daniel,
>>>>>
>>>>> I agree with that but I have a question kindly don't take it as an
>>>>> argument. Just curious to know
>>>>>
>>>>> 1. <a/>cdata<b/>
>>>>> 2. <a/> \n<b/>
>>>>> 3. <a/>comments<b/>
>>>>> 4. <a/>some PI's<b/>
>>>>>
>>>>> In all the above 4 scenarios when we do a @@previous on node 'b' we
>>>>> expect node 'a'.
>>>>
>>>> With what you have implemented so far, that is.
>>>>
>>>>> I am suggesting we will keep iterating until we find a node type
>>>>> ELEMENT_NODE and return it.
>>>>> you are suggesting to keep iterating until we find a node that is not in
>>>>> \n, CDATA, PIs etc.
>>>>>
>>>>> I think both will work. Do you think any of it which should be
>>>>> skipped will also have node type ELEMENT_NODE.
>>>>
>>>> Nope.
>>>>
>>>>> I am not sure about what is a better logic though. Kindly let me
>>>>> know if I am not getting something which you are telling.
>>>>
>>>> Silently skipping non-whitespace text is dangerous. But if you call
>>>> this @@previous_element, then the user will expect it to happen, so
>>>> then what you have implemented can be OK.
>>>>
>>>> As of my @@previous definition, the name is problematic even there, as
>>>> it doesn't just return the previous sibling (?previousSibling does
>>>> that). It does some magic, by skipping whitespace and such. So
>>>> certainly it should be called @@prevous_significant or
>>>> @@previous_non_ws, so that it's clear that some trickery is involved.
>>>> As of the semantic, the motivation is simply to return what many
>>>> naturally expect to be the previous node. Like remember your case;
>>>> getting some text instead of the preceding programlisting element was
>>>> unexpected at first, I assume. Yes, your definition of @@previous
>>>> fixes that too. But if you had some non-whitespace text between those
>>>> two programlisting elements, certainly you expect to get that text,
>>>> not the element before it. People don't see non-whitespace text as
>>>> ignorable, because in fact it hardly ever is.
>>>>
>>>> So after renaming both operations are OK, but I think
>>>> @@previous_significant is a safer operation than @@previous_element,
>>>> because you won't unintentionally skip non-whitespace text with it.
>>>> Surely @@previous_element is quite clear about what it does (that it
>>>> will skip text), but then, what can the users do about it? They will
>>>> have to hope that there won't be any non-whitespace text before the
>>>> target element, ever. Because when there is, they won't know about it,
>>>> they can't give an error or something. With @@prevous_significant,
>>>> when that assumption fails, they will get the text node and the
>>>> template that expects an element can fail or take some special action,
>>>> so there's no silent information loss.
>>>>
>>>> --
>>>> Thanks,
>>>> Daniel Dekany
>>>>
>>>>> Pradeep.
>>>>> ________________________________________
>>>>> From: Daniel Dekany <[email protected]>
>>>>> Sent: Wednesday, October 28, 2015 1:33 PM
>>>>> To: Pradeep Murugesan
>>>>> Subject: Re: Adding a new BuiltIn - previousSibling
>>>>>
>>>>> Wednesday, October 28, 2015, 3:52:35 AM, Pradeep Murugesan wrote:
>>>>>
>>>>>>> By that do you mean that you intend to continue it later so that it
>>>>>>> will only skip whitespace, etc., or you think this approach is more
>>>>>>> practical? (If it's the later, why?)
>>>>>>
>>>>>> ---- So by @@previous the user expects the previous node. But
>>>>>> currently it returns the \n , spaces, as you mentioned CDATA etc.
>>>>>> To skip these we need to maintain a list of blacklisted candidates
>>>>>> to skip. Today we have 3 candidates (let's assume) later we may get
>>>>>> lot to skip which we should be adding as blacklisted.
>>>>>> I went for this approach assuming there won't be any scenario
>>>>>> where we skip any nodes of type ELEMENT_NODE to fetch the
>>>>>> previousSibling node. If we will skip ELEMENT_NODE as well then no
>>>>>> other go we need to maintain a list of candidates to skip.
>>>>>
>>>>> I'm not sure what you mean be "maintaining". We just check the node on
>>>>> the fly, and decide if we proceed with its sibling or return it. What
>>>>> we want to skip certainly won't change over time, as the information
>>>>> model of XML won't change any time soon, if ever. It's WS-only text
>>>>> (it doesn't mater if it's plain text or a CDATA section), comments and
>>>>> PI-s. (We never skip elements.)
>>>>>
>>>>>> Kindly let me know if I am wrong.
>>>>>>
>>>>>> Regarding the nullPointer exception I have handled it. But Didn't
>>>>>> commit. Its like parent directive right we will return null if its
>>>>>> the root node, similarly we can return null if its first and last
>>>>>> accessing previous and next respectively.
>>>>>>
>>>>>> Pradeep.
>>>>>> ________________________________________
>>>>>> From: Daniel Dekany <[email protected]>
>>>>>> Sent: Wednesday, October 28, 2015 2:45 AM
>>>>>> To: Pradeep Murugesan
>>>>>> Subject: Re: Adding a new BuiltIn - previousSibling
>>>>>>
>>>>>> Tuesday, October 27, 2015, 6:04:19 PM, Pradeep Murugesan wrote:
>>>>>>
>>>>>>> Hi Daniel,
>>>>>>>
>>>>>>> Have fixed the code review comments here.
>>>>>>> https://github.com/pradeepmurugesan/incubator-freemarker/commit/2e1b0d834e941eaf4ea8aafad720333c7ec1040c
>>>>>>
>>>>>> It's minor issue, but BuiltInsExtForNode and BuiltInExtForNod still
>>>>>> don't follow the same convention as the others. The ...BI classes
>>>>>> should just be inside BuiltInsForNodes (no need for
>>>>>> BuiltInsExtForNode), and BuiltInExtForNode should be called
>>>>>> BuiltInForNodeEx.
>>>>>>
>>>>>>> Regarding the @@previous and @@next we decided to skip the
>>>>>>> whitespaces and other character data. Instead I tried to find first
>>>>>>> occurrence of the node which is of type Node.ELEMENT_NODE
>>>>>>
>>>>>> By that do you mean that you intend to continue it later so that it
>>>>>> will only skip whitespace, etc., or you think this approach is more
>>>>>> practical? (If it's the later, why?)
>>>>>>
>>>>>> Also, I believe that the current implementation will throw
>>>>>> NullPointerException after you have reached the first or the last
>>>>>> node.
>>>>>>
>>>>>>> https://github.com/pradeepmurugesan/incubator-freemarker/commit/2e1b0d834e941eaf4ea8aafad720333c7ec1040c#diff-a029bb56a7cedf8c6272a6d8b566f446
>>>>>>>
>>>>>>> I tried few cases and things worked fine there. Kindly let me know your
>>>>>>> thoughts.
>>>>>>>
>>>>>>> P.S : I am working on the Junit test cases.
>>>>>>>
>>>>>>> Pradeep.
>>>>>>>
>>>>>>> ________________________________________
>>>>>>> From: Daniel Dekany <[email protected]>
>>>>>>> Sent: Tuesday, October 27, 2015 3:36 AM
>>>>>>> To: Pradeep Murugesan
>>>>>>> Subject: Re: Adding a new BuiltIn - previousSibling
>>>>>>>
>>>>>>> OK, let's see. I have ran through the diff and have spotted these
>>>>>>> (just in the order as I find then):
>>>>>>>
>>>>>>> putBI("previousSibling", new previousSiblingBI()), etc. should be
>>>>>>> putBI("previous_sibling", "previousSibling", new previousSiblingBI()).
>>>>>>>
>>>>>>> BuiltInExtForNode: Doesn't follow the naming pattern of the other
>>>>>>> BuiltIns... classes.
>>>>>>>
>>>>>>> TemplateNodeModelExt: Should be TemplateNodeModelEx (as we already
>>>>>>> have other Ex models, we are stuck with that convention...)
>>>>>>>
>>>>>>> BuiltinVariable: You have registered two new names there, but these
>>>>>>> aren't built-in variables.
>>>>>>>
>>>>>>> In ElementModel: @@previous and @@next doesn't yet implement what we
>>>>>>> were talking about. I mean, it doesn't just skip white-space and
>>>>>>> comments and PI-s, but any text nodes. (Also an XPath-based
>>>>>>> implementation won't be very fast. org.w3c.dom.Node-s has
>>>>>>> getPreviousSibling()/getNextSibling() methods. Also, if you will skip
>>>>>>> WS text only, you won't be able to do that with XPath anyway.)
>>>>>>>
>>>>>>> (As a policy, there should not be author comments ("created by") in
>>>>>>> the source code.)
>>>>>>>
>>>>>>> --
>>>>>>> Thanks,
>>>>>>> Daniel Dekany
>>>>>>>
>>>>>>>
>>>>>>> Friday, October 23, 2015, 9:09:56 PM, Pradeep Murugesan wrote:
>>>>>>>
>>>>>>>> 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
>>>>>
>>>>> --
>>>>> Thanks,
>>>>> Daniel Dekany
>>>>>
>>>
>>> --
>>> Thanks,
>>> Daniel Dekany
>>
>> --
>> Thanks,
>> Daniel Dekany
>>
>
>
> ---
> This email has been checked for viruses by Avast antivirus software.
> https://www.avast.com/antivirus
>
--
Thanks,
Daniel Dekany
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
