Hi Daniel,
Done the changes
https://github.com/pradeepmurugesan/incubator-freemarker/commit/296a7a85a1f1683a3d20be0220881333cbdc4216
(ignore build.xml , have reverted it)
So previously we discussed to skip the following
1. Empty spaces
2. Comments
3. PIs
4. CDATA
Now 2 & 3s are not included in dom at all. even the ?previousSibling skips
those elements.
Now the challenge comes in skipping the CDATA. I tried to use the nodeType
check i.e ( if node.getNodeType == Node.CDATA_SECTION_NODE) but this is not
working since CDATA node is considered as text node and the node type is
returned as TEXT. Also the getNodeTextContent is returning the text inside the
CDATA tag (Not the string CDATA itself) so I am not sure how we will be
picking which is a characterData and which is a non empty text.
Eg:
<person>
<profession>Software Engineer</profession>
<![CDATA[ <a>test<a> ]]>
<phone>12345678</phone>
</person>
doing a doc.person.phone.@@previous returns the node type as text with value as
<a>test<a>;
I am not sure which is the criteria to check the CDATA node. Am i missing
something here ?
Pradeep.
________________________________________
From: Daniel Dekany <[email protected]>
Sent: Thursday, December 10, 2015 5:07 AM
To: Pradeep Murugesan
Cc: [email protected]
Subject: Re: Adding a new BuiltIn - previousSibling
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
