imagej / imagej-ui-swing

ImageJ UI for Java Swing.
BSD 2-Clause "Simplified" License
10 stars 20 forks source link

Restore addImports functionality #67

Open hinerm opened 8 years ago

hinerm commented 8 years ago

Allows package detection of imports in Java in the script editor.

See http://fiji.sc/bugzilla/show_bug.cgi?id=817

hinerm commented 8 years ago

@ctrueden @imagejan - any complaints?

ctrueden commented 8 years ago

Firstly: cool idea.

Questions and concerns:

hinerm commented 8 years ago

Performance of classpath scanning en masse?

Slow. Takes a second or two. So you can tell it's happening but it doesn't feel ridiculous.

That said, I wrote it so that it only scans once and caches the discovered collection. Iterating the org-filtered collection of items is not really noticeable on subsequent adds.

Regardless, we could possibly add a progress bar.

The auto-imports feature in general should be in a UI-agnostic layer in SciJava Common probably.

Yep I agree with this plan.

since it hardcodes sc.fiji in a net.imagej component, which we try very hard not to do.

Agreed that this should be changed; that was a primary motivator for the TODO :smile: . I guess I can put a plugin for sc.fiji in https://github.com/fiji/fiji now that it has a src directory!

ctrueden commented 8 years ago

I guess I can put a plugin for sc.fiji in https://github.com/fiji/fiji now that it has a src directory!

Or perhaps repurpose the fiji-scripting project. I dunno.

hinerm commented 8 years ago

Isn't that the behavior the user expects anyway? Auto-import any available class?

Yes, I would rather do this. But on my computer it takes 9s to scan all resources without filters, and 1.5s with filters. Anyway if we use an extensible model we can easily add more packages as needed.

The auto-imports feature in general should be in a UI-agnostic layer in SciJava Common probably. Part of the ScriptLanguage, perhaps? And then each scripting- component can implement it the way it needs to—so scripting-java would have the reflections dependency. Does this make sense?

@ctrueden - after further thought here is my take on the architecture:

any concerns?

hinerm commented 8 years ago

Yes, I would rather do this. But on my computer it takes 9s to scan all resources without filters, and 1.5s with filters. Anyway if we use an extensible model we can easily add more packages as needed.

per @ctrueden's suggestion we can just run this initialization on a background thread at startup

imagejan commented 6 years ago

The TextEditor now lives in scijava/script-editor, so we should move this PR over there.