ExternalTreeModel skips insertFunc and removeFunc on root level

[Sciss]

I was chasing a bug when finally arriving at step debugging into model insertions and deletions with ExternalTreeModel. My external insertFunc and removeFunc keep a children list up to date, so I was bitten in the foot by insertUnder and remove when the changes happen on the root level.

Perhaps I'm misunderstanding the external model (still), but IMO it makes no sense that insertFunc and removeFunc are only called on sub trees but not the root level. E.g.

def insertUnder(parentPath: Path[A], newValue: A, index: Int): Boolean = {
  val succeeded = if (parentPath.nonEmpty) {
    insertFunc(parentPath, newValue, index)
  }
  else {     // not call to insertFunc !!
    val (before, after) = rootsVar splitAt index
    rootsVar = before ::: newValue :: after
    true 
  }
  ...
}

Shouldn't that be

def insertUnder(parentPath: Path[A], newValue: A, index: Int): Boolean = {
  val succeeded = insertFunc(parentPath, newValue, index)
  if(parentPath.isEmpty) {
    val (before, after) = rootsVar splitAt index
    rootsVar = before ::: newValue :: after
  }
  ...
}

?

[Sciss]

I am using a reduced version of external tree model now: https://github.com/Sciss/Mellite/blob/elements/src/main/scala/de/sciss/mellite/gui/impl/TreeModelImpl.scala

Since the model is only accessed by me directly, I am ensuring that external data is updated prior to insertUnder and after remove. This seems to work fine.

[kenbot]

Hey Sciss, does this issue still require action? I just realised there were still open issues here.

[Sciss]

I have mostly moved to a mixed Tree/Table based component. In the case where I do use Tree, I can still use my model implementation as above; so I don't know what the best action would be. I would suggest that the model either needs an extensive refactoring, or leave it as is for now.

I do plan to look at Tree again at some point and make my suggestion regarding the model API, but I can't promise when I will have time to do so.

[OndrejSpanel]

I was hit by the same issue. The fact I am not notified of the root changes and they are done without any callback on my side to validate them is a show stopper for me. In my case the roots must never be deleted and no other nodes may become roots, and I do not see how to prevent this.

[kenbot]

Thanks for the report Ondrej - it's time for me to look at this again. I'll try to find some time over the next few days.

On 5 December 2015 at 01:54, Ondřej Španěl notifications@github.com wrote:

I was hit by the same issue. The fact I am not notified of the root changes and they are done without any callback on my side to validate them is a show stopper for me. In my case the roots must never be deleted and no other nodes may become roots, and I do not see how to prevent this.

— Reply to this email directly or view it on GitHub https://github.com/benhutchison/ScalaSwingContrib/issues/5#issuecomment-161985588 .

[OndrejSpanel]

I might be able to create a PR for you to review, if that helps to make the change happen.

[benhutchison]

Sure, if there's a PR to merge I will publish a new version within a day

On Sat, Dec 5, 2015 at 7:21 PM, Ondřej Španěl notifications@github.com wrote:

I might be able to create a PR for you to review, if that helps to make the change happen.

— Reply to this email directly or view it on GitHub https://github.com/benhutchison/ScalaSwingContrib/issues/5#issuecomment-162160510 .

[kenbot]

@OndrejSpanel 's PR has been merged, which used code similar to @Sciss 's snippet to ensure that the user-supplied update/remove functions get called on root nodes too.

[OndrejSpanel]

@kenbot Would it be possible to have a new version release based on this? If not, I guess I will have to create my own private repository and created a forked release, as this is currently blocking my development progress.

[benhutchison]

Hi Ondřej,

I am working on publish a new version now.

-Ben

On Fri, Dec 11, 2015 at 12:47 AM, Ondřej Španěl notifications@github.com wrote:

Would it be possible to have a new version release based on this? If not, I guess I will have to create my own private repository and created a forked release, as this is currently blocking my development progress.

— Reply to this email directly or view it on GitHub https://github.com/benhutchison/ScalaSwingContrib/issues/5#issuecomment-163622946 .

[benhutchison]

Published version 1.6 to sonatype

On Mon, Dec 14, 2015 at 5:05 PM, Ben Hutchison brhutchison@gmail.com wrote:

Hi Ondřej,

I am working on publish a new version now.

-Ben

On Fri, Dec 11, 2015 at 12:47 AM, Ondřej Španěl notifications@github.com wrote:

Would it be possible to have a new version release based on this? If not, I guess I will have to create my own private repository and created a forked release, as this is currently blocking my development progress.

— Reply to this email directly or view it on GitHub https://github.com/benhutchison/ScalaSwingContrib/issues/5#issuecomment-163622946 .