Some minor issues I have noticed at a quick glance: - You have added some unused special-variable constants - You are using a mutable List as EMPTY_LIST constant (you could use Collections.emptyList()) - node.getTextContent().toCharArray(): Can't getTextContent() be null? Also what we need is not copying the String to an array, but to add a isTrimmableToEmpty overload that works with String-s. - isSignificantNode evaluates the test conditions that it might won't need. Yes, the performance cost is trivial (esp. in this XML wrapper which tends to do expensive things), but it's a code clarity issue. - Some tests are needlessly complied with the #if-s; you could just print the size and assert that.
I will merge this in the coming days anyway, and fix the further things I notice myself. Note that this will go into 2.3.25. -- Thanks, Daniel Dekany Monday, January 11, 2016, 5:04:01 PM, Pradeep Murugesan wrote: > I did a clean and then built. Its working fine now. > > Sent a pull request, > https://github.com/apache/incubator-freemarker/pull/9 > > Kindly review and let me know. > > Pradeep. > > ________________________________________ > From: Pradeep Murugesan <[email protected]> > Sent: Monday, January 11, 2016 8:50 PM > To: [email protected]; Daniel Dekany > Subject: Re: Adding a new BuiltIn - previousSibling > > Hi Daniel, > > I did the merge with the branch gae-2.3. Once I build and run the > tests I am getting the following error message > > Caused by: java.lang.RuntimeException: Clashing FreeMarker versions > (2.3.24-rc01-incubating and some post-2.3.x) detected: found > post-2.3.x class freemarker.core._2_4_OrLaterMarker. You probably > have two different freemarker.jar-s in the classpath. > > > I am not sure where the jars clash. Kindly let me know what I am missing. > > Pradeep. > > ________________________________________ > From: Daniel Dekany <[email protected]> > Sent: Friday, December 18, 2015 2:30 PM > To: Pradeep Murugesan > Subject: Re: Adding a new BuiltIn - previousSibling > > If you believe it's complete and is well tested, then yes. For > "freemarker", it's "2.3-gae" branch. For "docgen", it's the "master" > branch. (See also: > http://freemarker.incubator.apache.org/sourcecode.html) > > -- > Thanks, > Daniel Dekany > > > Friday, December 18, 2015, 3:13:50 AM, Pradeep Murugesan wrote: > >> So shall I go ahead and give a PR for the core changes.. Which >> branch I need to merge with and give PR??. >> >> Pradeep >> >>> On 18-Dec-2015, at 1:09 am, "Daniel Dekany" <[email protected]> wrote: >>> >>> It looks about right. >>> >>> We shouldn't require data-model for all examples, as in many cases it >>> would be empty anyway. >>> >>> -- >>> Thanks, >>> Daniel Dekany >>> >>> >>> Thursday, December 17, 2015, 2:52:45 PM, Pradeep Murugesan wrote: >>> >>>> Hi Daniel, >>>> >>>> I have made the changes in doc-gen as per the core changes. >>>> >>>> https://github.com/pradeepmurugesan/incubator-freemarker-docgen/commit/46d77c6b5a3cbe01a50c7756e1efb630ca00e18a >>>> >>>> I have added a formDataModel in /manual/dgui_quickstart_template.html >>>> >>>> Kindly check and let me know if its fine. >>>> >>>> Also should we need to add a datamodel section for all the template >>>> section in the manual ? >>>> >>>> Pradeep. >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> ________________________________________ >>>> From: Daniel Dekany <[email protected]> >>>> Sent: Monday, December 14, 2015 2:39 AM >>>> To: Pradeep Murugesan >>>> Cc: [email protected] >>>> Subject: Re: Adding a new BuiltIn - previousSibling >>>> >>>> You can build on these new "@@" keys in Docgen of course. As of when >>>> it will be merged into a stable release, I don't know yet, maybe >>>> 2.3.24, maybe 2.3.25. In any case, Docgen, as an internal project, can >>>> use nightly versions, so it doesn't have to wait for stable releases. >>>> >>>> For efficiency, I usually try to review contributions in one go, when >>>> the pull request is merged. But I took a quick glance at the commits, >>>> and hasn't spotted any problems. >>>> >>>> -- >>>> Thanks, >>>> Daniel Dekany >>>> >>>> >>>> Sunday, December 13, 2015, 9:48:17 AM, Pradeep Murugesan wrote: >>>> >>>>> Hi Daniel, >>>>> >>>>> I have added those cases for CDATA as well. >>>>> https://github.com/pradeepmurugesan/incubator-freemarker/commit/620d8a35e689bd6e94fb77ceb844105d66b90ca9 >>>>> >>>>> Renamed @@previous and @@next to @@previous_significant and >>>>> @@next_significant >>>>> https://github.com/pradeepmurugesan/incubator-freemarker/commit/cbe7025bfe8fe713b74d1b5499d14fd7cd35c4f8 >>>>> >>>>> Kindly review the same and let me know if we are good to integrate with >>>>> docgen. >>>>> >>>>> Pradeep. >>>>> >>>>> >>>>> ________________________________________ >>>>> From: Daniel Dekany <[email protected]> >>>>> Sent: Sunday, December 13, 2015 12:07 AM >>>>> To: Pradeep Murugesan >>>>> Cc: [email protected] >>>>> Subject: Re: Adding a new BuiltIn - previousSibling >>>>> >>>>> I guess you get it right. We have to ignore text that's white-space >>>>> only, and wether it's CDATA or not we will do the same. >>>>> >>>>> >>>>> Saturday, December 12, 2015, 7:45:17 AM, Pradeep Murugesan wrote: >>>>> >>>>>> 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 >>>>>> >>>>>> >>>>> >>>>> -- >>>>> Thanks, >>>>> Daniel Dekany >>>>> >>>>> >>>>> --- >>>>> This email has been checked for viruses by Avast antivirus software. >>>>> https://www.avast.com/antivirus >>>>> >>>>> >>>> >>>> >>>> --- >>>> This email has been checked for viruses by Avast antivirus software. >>>> https://www.avast.com/antivirus >>> >>> >>> --- >>> This email has been checked for viruses by Avast antivirus software. >>> https://www.avast.com/antivirus >>> >>> >> >> > > > --- > This email has been checked for viruses by Avast antivirus software. > https://www.avast.com/antivirus > > >
