Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Scripteditor codeassist and code completion #54

Closed
wants to merge 10 commits into from
Closed

Scripteditor codeassist and code completion #54

wants to merge 10 commits into from

Conversation

Squareys
Copy link
Contributor

Hello everybody!

This adds codecompletion/-assist to the script editor using fifesoft Autocomplete and RSTALanguageSupport. This branch is based on #51, which is needed to be merged first.

Support for Autocompletion can be extend by extending the LanguageSupportPlugin. The repository RSTALanguageSupport also implements outline trees for java for example, which one could take into consideration for the script editor. (Not implemented in this PR!)

For this pullrequest, code assist is implemented for Java and JavaScript.

Have a nice day,
Squareys.

@hinerm hinerm mentioned this pull request Aug 20, 2015
Closed
5 tasks
@Squareys Squareys changed the title [Blocked] Scripteditor codeassist and code completion Scripteditor codeassist and code completion Aug 20, 2015
@Squareys
Copy link
Contributor Author

I will do a quick rebase onto master to make sure, then this can be merged, too.

@Squareys
Add dependencies to com.fifesoft.(autocomplete|languagesupport)
944c5a1 
Needed for the upcoming autocompletion support in the script editor.

Signed-off-by: squareys <[email protected]>
@Squareys
This method does not need to be static
6349f8f 
Signed-off-by: squareys <[email protected]>
@Squareys
Add API for Languagesupport/Autocompletion
6a4f073 
Add `LanguageSupportPlugin` which is a ScijavaPlugin for
`LanguageSupport` and the corresponding `LanguageSupportService` which
is currently created manually in `EditorPane`. Whenever the
`ScriptLanguage` for the `EditorPane` is set, corresponding language
support will be installed on it, uninstalling existing.

Signed-off-by: squareys <[email protected]>
@Squareys
Add autocompletion/code assistance for java and javascript
56f328e
@Squareys
Copy link
Contributor Author

Ah, much better :) You may have a look at this, it's ready from my side.

@hinerm
Copy link
Member

hinerm commented Aug 20, 2015

@Squareys this is really cool stuff.

I think how the API is actually being used needs a bit of tweaking though:

  • You don't need to call initialize on the service. It's called at context startup.
  • The LanguageSupportService should just be a (non-static) @Parameter like the other services
  • You shouldn't need to cache the LanguageSupport instance. When setLanguage is called, just check the current language, use that information to get the LanguageSupport from the LanguageSupportService, uninstall it, then install the new service like you were.

As a side note - is it possible to install the language support on the tab instead of the pane..? Does switching tabs cause setLanguage to be called? (just can't remember offhand) If not sure it matters.. just wondering.

@Squareys
Copy link
Contributor Author

@hinerm:

You don't need to call initialize on the service. It's called at context startup.

The Service is not added to the context. (Or is it... automatically?)

The LanguageSupportService should just be a (non-static) @parameter

Well, see above.

You shouldn't need to cache the LanguageSupport instance.

Alright, your suggestion makes sense, will do so tomorrow (German time, therefore in 12-14 hrs)

is it possible to install the language support on the tab instead of the pane

You mean install it on the pane but from the outside? Theoretically yes.

Does switching tabs cause setLanguage to be called?

I do not believe so. It should at least only be called, when:

  1. Set by the user manually
  2. A file is opened/reloaded/saved.

@ctrueden
Copy link
Member

The Service is not added to the context. (Or is it... automatically?)

Yes, it is. You wrote @Plugin(type = Service.class) so when a full-blown Context is created during ImageJ startup, it will instantiate and initialize a LanguageSupportService which is part of that context. You should never call new on it directly, or initialize for that matter.

@Squareys
Copy link
Contributor Author

Yes, it is.

Awesome! I will change the code respectively.

Best to have a single version property for the com.fifesoft stuff that is set to 2.5.7 and reuse it in all relevant deps.

Alright, will do.

@Squareys
Use property for all org.fifesoft dependency versions
59beecb 
They all get released simultaneously which means having a single
property to change makes updating easier.

Signed-off-by: squareys <[email protected]>
@Squareys
Correctly use LanguageSupportService
5642f04 
Service is added to the context automatically and will be injected when
the Parameter annotation is present.

Also: Remove language cache for uninstalling.

Signed-off-by: squareys <[email protected]>
@Squareys
Use "name" instead of "label" in LanguageSupportPlugins
b604005 
... for matching them to ScriptLanguages.

Signed-off-by: squareys <[email protected]>
@Squareys
Rename getCompletionProvider to getLanguageSupport
7bfd3db 
Better matches the return type.

Signed-off-by: squareys <[email protected]>
@Squareys
Use name rather than label to define language for SyntaxHighliter plu…
af875d8 
…gins

Signed-off-by: squareys <[email protected]>
@Squareys
Copy link
Contributor Author

@ctrueden Alright, I think I addressed all the issues mentioned above.

@Squareys
Copy link
Contributor Author

Well, for some reason, AutoCompletion is not working anymore. I will need to investigate that first, before this can be merged.

@Squareys
Copy link
Contributor Author

Alright, done! This may be merged.

@hinerm
Copy link
Member

hinerm commented Aug 21, 2015

Hi @Squareys . This is looking really good.

The main problem I'm finding is that it seems like an uncaught exception is thrown by code completion parser when there's a compilation error. It's redundant because the error is already displayed in the editor.. so we don't want the stack trace to pop up in the console window. Is there a way to suppress these IOExceptions?

You can see this easily by dropping this into Fiji and opening any Java template. It currently is inserting .java into the class names which causes compilation errors. (I'm looking into fixing the .java error)

@hinerm
Copy link
Member

hinerm commented Aug 21, 2015

OK, fixed the .java class name: 5ba7bf8

@hinerm
Copy link
Member

hinerm commented Aug 21, 2015

bleh.. so there is a catch block guarding these exceptions, it's being caught earlier here and printed.

@hinerm
Copy link
Member

hinerm commented Aug 21, 2015

see: bobbylight/RSTALanguageSupport#26

@hinerm hinerm closed this in a01fd37 Aug 21, 2015
@hinerm
Copy link
Member

hinerm commented Aug 21, 2015

Alright @Squareys ... I just commented out the Java language plugin for now, until the stacktrace printing is resolved upstream. Everything on your end looked good so I merged. Thanks again for your work on this!

@Squareys
Copy link
Contributor Author

@hinerm You're welcome, thank you, too! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Squareys @hinerm @ctrueden