PR #46 did not completely fix Maven support

[gluon-bot]

Originally reported by: Lennard Fonteijn (Bitbucket: LennardF1989, GitHub: LennardF1989)

---

Having a Maven project, I was wondering why I could not select @FXML-annotated things from my controller in Scene Builder (running 8.4.1). I looked at the changes of PR #46, and found a huge oversight in this fix.

Situation

Consider the following standard setup of folders:

• src/main/resources/fxml/Scene.fxml
• src/main/java/com/mycompany/mavenproject/controllers/SceneController.java

Problem

Right now, the Maven fix from PR #46 does the following.

1. Check if the current path to the fxml-file contains "src/main/resources"
2. If it does, grab the parent-folder of the fxml-file and replace "src/main/resources" with "src/main/java". Going back to the situation, you'll end up with: "src/main/java/fxml"
3. Scan up to 2 directories deep, for a java-file with the same name as the fxml-file, or one postfixed with "Controller.java". Going back to situation we'll start looking for a "Scene.java" or "SceneController.java" inside "src/main/java/fxml".

Solution

As you can see, grabbing the parent inside step 2 is a pretty naive approach, as you'll end up looking inside a non-existing folder, or plain wrong folder for that matter.

The code should instead start scanning from "src/main/java" up to a configurable depth.

From a performance point of view, I understand you don't want to recursively walk through your whole file-tree. But in most cases, you can prevent doing so by considering the fx:controller attribute in the FXML. Which also removes the need for "guessing" file-names unless absolutely nescessary.

I can easily provide a PR to fix this issue, if someone can give his thoughts on a potential fix. Recursively scanning everything is the quick-n-dirty fix, using the fx:controller when available the nice one. Thoughts?

---

• Bitbucket: https://bitbucket.org/gluon-oss/scenebuilder/issue/151

[gluon-bot]

Original comment by Lennard Fonteijn (Bitbucket: LennardF1989, GitHub: LennardF1989):

---

Just a follow-up, I have succesfully created the "nice" fix using the fx:controller from the FXML file, which ended up being roughly 10 lines of code. There are two ways to go about the fix though:

1. I instance my own ControllerClass and return that immediately upon resolving the controller.
2. I Grab the parent folder of the resulting file location, and let it run through the recursive scanDirectory.

Option 2 seems like a waste, but it has the added effect of populating the controller dropdown in the UI, whereas option 1 will only displays the current controller in the dropdown. In both cases however, you can still manually type a new path that's not in the dropdown yet and make it take effect immediately (after a save).

I'm leaning towards option 2 myself as the final solution.

On a side node, discoverFXMLControllerClasses is being called a lot! Specifically, once for every dropdown that has to be populated with something. But that's beyond the scope of this issue.

With the above in mind though, from a performance point of view, I can probably use a modified version of readFile which stops as soon as it encounters the fx:controller-line. Rather than reading the whole file into memory x times per UI action... Or simply add a cache which keeps track of the modification date...

[gluon-bot]

Original comment by Kerry Billingham (Bitbucket: kerry, GitHub: kerry):

---

Hi,
The original PR does make it clear that is assumes the FXML file will be under the same package directory and it is difficult to accommodate individual preferences as to where the FXML file is placed, thus it seemed natural to make an assumption that the FXML file will be under the same package as its controller. It also made sense (to me at least!) have the FXML file under the same package structure particularly in the case of a large project.

But obviously that is my assumption as I prefer standard over bespoke ways of doing things so there is a commandline switch to turn off this behaviour.

My only concern is that to try and accommodate all permutations of where the controller file might be placed the code ends up being too complex and there will always be edge cases one forgets about. There didn't seem to be any strong argument to me to not put the FXML file under the same package thus this simple solution was offered.