Hi Daniel, So we can ignore a CDATA text of that is mere formatting right ?
Now it burns down to identify whether the text inside CDATA is a formatted one or not. Am I right or did you mean the other way ? Pradeep. ________________________________________ From: Daniel Dekany <[email protected]> Sent: Friday, December 11, 2015 12:44 AM To: Pradeep Murugesan Subject: Re: Adding a new BuiltIn - previousSibling Thursday, December 10, 2015, 9:28:31 AM, Pradeep Murugesan wrote: > 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 There's some kind of misunderstand here as CDATA shouldn't be there like that. But see later. > Now 2 & 3s are not included in dom at all. Depends on how the TemplateModel was created. There are some convenience methods included that remove commends and PI-s from the DOM itself before wrapping, but some applications will just give a DOM to wrap. > 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. CDATA is nothing just syntax for avoiding escaping special characters. So of course we don't want to ignore them in general. Just think about it, in <a/><![CDATA[foo bar]]><b/>, it's not like "foo bar" there can be ignored without losing useful information (as opposed to losing text that's just formatting). In fact, something being inside CDATA is a proof that it's not just formatting, even if it's white-space. But as we can't (reliably) tell if a piece of text is coming from CDATA or not, we should ignore that difference even where you can tell it. -- Thanks, Daniel Dekany > 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 > -- Thanks, Daniel Dekany --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus
