[CLOSED] Replace getParent() by findAncestorWithClass() method

[tduchateau]

Issue by eruiz from Tuesday Jun 25, 2013 at 17:33 GMT

---

TagSupport.getParent() method gives us access to parent TableTag as follows:

TableTag parent = (TableTag) getParent();

The problem is getParent() doesn't let us to compose tables as shown below:

<datatables:table ...>
  <c:if ...>
    <datatables:column .../>
  </c:if>
</datatables:table>

To solve it, we can use the method TagSupport.findAncestorWithClass() in spite of getParent() method:

TableTag parent = (TableTag) findAncestorWithClass(this, TableTag.class);

[tduchateau]

Comment by tduchateau from Wednesday Jun 26, 2013 at 07:03 GMT

Indeed, that would be great. Thanks Enrique!

[tduchateau]

Comment by eruiz from Wednesday Jun 26, 2013 at 07:31 GMT

Thibault, I could do the pull-request asap ... ok?

[tduchateau]

Comment by tduchateau from Wednesday Jun 26, 2013 at 07:53 GMT

Go for it! ;-)

[tduchateau]

Comment by eruiz from Monday Jul 08, 2013 at 16:39 GMT

Hi Thibault, I posted the pull request https://github.com/dandelion/dandelion-datatables/pull/7

I don't know if it is right because I think github accumulates all changes, including those done in pull request 6. Please take a look.

[tduchateau]

Comment by tduchateau from Wednesday Jul 10, 2013 at 06:01 GMT

Hi Enrique,

Normally it should work properly when your create a new branch dedicated to a pull request from your dandelion-datatables' master branch but this branch needs to remain untouched (at least up to date with the upstream).

Your issue appears to be the fact that you merged one your pull requests in your master branch. That's why the pull #7 embeds commits of the pull #6.

So, what do we do now? :-)

1) I'm gonna accept the #6 first and then #7 (which comes with ugly commits but anyway) 2) you absolutely need to clear your master branch or your next pull requests will result in the same issue, i.e. it must be kept synchronized with the dandelion-datatables/master branch. 3) I'm about to create a 0.9.1-wip branch for the next dev. Please use this one for the next PR

Regards, Thibault

[tduchateau]

Comment by eruiz from Wednesday Jul 10, 2013 at 07:52 GMT

Hi Thibault, agree.

If you prefer:

1) Accept PR 6 and discard PR 7 2) I clean and sync our master branch 3) I do a new PR over the sync master branch to send the changes introduced in PR 7 plus the code that solves issue 146. It would be great if you can include issue 146 in 0.9 :-)

Regards,

Enrique

[tduchateau]

Comment by tduchateau from Wednesday Jul 10, 2013 at 11:46 GMT

1) Sounds fine to me 2) Thanks :-) 3) OK for the new PR that includes changes for #138 but please don't include changes that solve #146. So either you make a separated PR for #146, or I'll take care of it but it will surely be part of the 0.9.0 ;-)

Thanks Enrique!

[tduchateau]

Comment by eruiz from Thursday Jul 11, 2013 at 06:29 GMT

Done! PR 8 ready

[tduchateau]

Comment by tduchateau from Thursday Jul 11, 2013 at 12:48 GMT

See https://github.com/dandelion/dandelion-datatables/pull/8