Tree: Should treePathToPath handle null? And how?

[Sciss]

The documentation says

/**
 * Implicitly converts javax.swing.tree.TreePath objects to Tree.Path[A] lists recognised in Scala Swing.  
 * TreePaths will include the underlying JTree's hidden root node, which is omitted for Tree.Paths.
 */
implicit def treePathToPath(tp: jst.TreePath): Path[A] = model treePathToPath tp

And treePathToPath maps null to null. Since as I understand, the Scala version drops root from the path, we cannot use Path.empty here, right?

Then I suggest that getClosestPathForLocation should return an Option[Path[A]] instead of a nullable Path[A].

[kenbot]

Good points. Java Swing tends to treat TreePaths as always nullable, which would mean a faithful Scala representation would either be an Option[Path] or an ADT with a NullPath case. Path is currently a rebadged IndexedSeq, which I like, but that means an ADT is impossible. OTOH, having the treePathToPath/pathToTreePath methods use Option[Path] would either be very clumsy, or asymmetric.

So I don't have a better thought than passing on the nulls for the conversion methods. For getClosestPathForLocation though, I totally agree: this should return Option[Path]. Someone looking for "closest X to Y" should have no expectation of a definite result.

Another interesting one: should getClosestRowForLocation return Option[Int] rather than -1?

[Sciss]

Using -1 as 'none' value for integers is perfectly fine IMO. This is standard behaviour e.g. for indexOf and indexWhere on sequences. I wouldn't change that.

I put the change for getClosestPathForLocation in the pull request 2352a3f

[kenbot]

I reckon yes to both. I'll push when I get back later, and have some time to play around with it.

[kenbot]

Hey Sciss, do you know if this got resolved? The last commit of note is your pull request #3 .

[Sciss]

Hey Ken; sorry no, I don't remember. I would have to investigate this again...