scijava / script-editor

Script Editor and Interpreter for SciJava script languages
http://imagej.net/Script_Editor
BSD 2-Clause "Simplified" License
12 stars 11 forks source link

New Python Language support plugin breaks old #48

Closed haesleinhuepf closed 3 years ago

haesleinhuepf commented 4 years ago

Hey @acardona ,

I'm happy about your efforts on the auto-completion side. Just please be careful. I just downloaded and compiled the master branch of script-editor. It breaks the auto-completion of CLIJ. This is how it worked before: image

And now, there is no pulldown opening anymore behind otsu (also not when hitting CTRL+Space).

However, what works now, is auto-completion of static methods (thanks to your great work!): image image

Do you think it would be possible to combine your implementation with mine? E.g. could we integrate this class in your implementation to receive custom completions including documentation?

Thanks for your support!

Cheers, Robert

acardona commented 4 years ago

Hi Robert, Sorry about that. I wasn't aware of any autocompletion being implemented for jython in the Script Editor before (the "support" was null for jython). Are you subclassing the Script Editor in CLIJX? Also, your class implements autocompletion by writing in the specific completions that you wanted using a sort of Hungarian notation on variables, rather than discovering all possible class and static method completion for that instance. I tried to stay away from doing that because it isn't future-proof regarding API changes to autocompleted classes. The autocompletion that I implemented strictly limits itself to class names and their static methods; eventually I'll parse the script up to the line, discover the class of each variable, and provide autocompletions based on that. Albert

haesleinhuepf commented 4 years ago

Hey @acardona

I wasn't aware of any autocompletion being implemented for jython in the Script Editor before

I pointed you to that thread earlier ;-)

Are you subclassing the Script Editor in CLIJX?

No.

Also, your class implements autocompletion by writing in the specific completions that you wanted using a sort of Hungarian notation on variables, rather than discovering all possible class and static method completion for that instance. I tried to stay away from doing that because it isn't future-proof regarding API changes to autocompleted classes. The autocompletion that I implemented strictly limits itself to class names and their static methods; eventually I'll parse the script up to the line, discover the class of each variable, and provide autocompletions based on that.

That's all great! Really, I love it. Nevertheless, it breaks the CLIJ-auto-completion some people are used to. Can we try to combine both before delivering this to end-users? I'm happy to chat about details at I2K.

Cheers, Robert

acardona commented 4 years ago

Hi Robert,

Sorry, I am not understanding where the autocompletion that you implemented comes from. Is it then an autodiscovered injected magic sort of approach, with the source originating in a Fiji update site?

As for merging: how can it be merged if it's two different @Plugin(type = LanguageSupportPlugin.class) ? Is there a mechanism for delegating from one to the other when an autocompletion isn't found?

haesleinhuepf commented 4 years ago

I am not understanding where the autocompletion that you implemented comes from. Is it then an autodiscovered injected magic sort of approach, with the source originating in a Fiji update site?

Correct! It comes if you install the clij and clij2 update sites.

As for merging: how can it be merged if it's two different @Plugin(type = LanguageSupportPlugin.class) ? Is there a mechanism for delegating from one to the other when an autocompletion isn't found?

If you introduce a method that allows adding custom entries to the auto-completion pulldown, I'm happy to change my code so that it uses that method. I think we shouldn't have two LanguageSupportPlugins for Jython

Cheers, Robert

acardona commented 4 years ago

Sounds good, add it! Will review later today or tonight, then we release again, which we have to do anyway to include recent fixes.

Albert

On Nov 30, 2020, at 12:03 PM, Robert Haase notifications@github.com wrote:

 I am not understanding where the autocompletion that you implemented comes from. Is it then an autodiscovered injected magic sort of approach, with the source originating in a Fiji update site?

Correct! It comes if you install the clij and clij2 update sites.

As for merging: how can it be merged if it's two different @Plugin(type = LanguageSupportPlugin.class) ? Is there a mechanism for delegating from one to the other when an autocompletion isn't found?

If you introduce a method that allows adding custom entries to the auto-completion pulldown, I'm happy to change my code so that it uses that method. I think we shouldn't have two LanguageSupportPlugins for Jython

Cheers, Robert

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

acardona commented 4 years ago

The easiest may be to rename getCompletionsImpl to e.g. getClassCompletions, and then implement a new getCompletionsImpl that collects from both getClassCompletions and from another list of completions that originate from elsewhere.

That elsewhere would be from a listener pattern, so that the text to match is handled to each listener and a list of completions is obtained from them.

haesleinhuepf commented 4 years ago

Hey @acardona ,

can you demonstrate in a @test or main function how to access this from outside?

Thanks!

acardona commented 4 years ago

Hi @haesleinhuepf , I didn't implement the listener (yet), was suggesting you could ;-)

May do later tonight.

acardona commented 4 years ago

I've done it. See https://github.com/scijava/script-editor/pull/50

acardona commented 4 years ago

Did you try it? Can I release again?

imagesc-bot commented 3 years ago

This issue has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/auto-code-completion-for-python-and-groovy-in-imagej-fiji/20515/23