Extend Jython auto-completion to instantiated classes by tferr · Pull Request #51 · scijava/script-editor

tferr · 2020-12-04T16:12:36Z

@acardona , @haesleinhuepf , This implements the following changes by extending Albert's recent work:

Adds auto-completion to non-static methods
Auto-completion does not 'crash' if import statements are wrong (with typos, etc.)
Improves the description of suggestions by including type of parameters and return values

Currently, if there is e.g., a typo in an import declaration completion will fail with a ClassNotFoundException. This commit adds placeholders to the completion list to warn user about such cases,

imagesc-bot · 2020-12-04T16:13:57Z

This pull request 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/22

tferr · 2020-12-04T16:36:26Z

@acardona , there are still two things that are missing in this PR:

Import with parenthesis are not recognized. E.g. in from ij import (ImagePlus, ImageProcessor), neither ImagePlus nor ImageProcessor get recognized as being imported
the description of the auto-completion is missing the URL for the class's Javadoc. This would be straightforward to include the a tag here. I think ClassUtil, has already all that information cached?
Another issue: currently completion works only with instantiated classes. It does not work with instances retrieved from static calls. I.e.: this works well:

from ij import ImagePlus
imp = ImagePlus()

But no completions are retrieved if you use:

from ij import IJ
imp = IJ.getImage()

The latter would be the most important issue. I will wait for your feedback on this.

imagesc-bot · 2020-12-04T17:51:04Z

This pull request 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/25

acardona · 2020-12-04T18:04:31Z

Thanks a lot @tferr, this is great.

On from ij import (ImagePlus, ImageProcessor): are these even valid? If so, I had no idea. In python, one can use multi-imports like:

from ij import ImagePlus, ImageProcessor

or even:

from ij import ImagePlus,\
                 ImageProcessor

acardona · 2020-12-04T18:08:11Z

On this use case:

from ij import IJ
imp = IJ.getImage()

If it's from a static method, the object class can be retrieved from the return type of the static method.

If it's from a function, like:

from ij import IJ
def getImage():
  return IJ.getImage()

imp = getImage()

... then no cookie: it's the perennial problem in non-strongly typed languages. Python is fixing this with annotations, which seem to be a part of the language in 3.8. There is currently an effort to bring jython to python 3+, but it's very understaffed (one very part-time developer).

(not just those which start with it)

acardona · 2020-12-04T18:41:09Z

+//				System.out.println("Hit: line #" + i + ": " + line);
+//				System.out.println("Matcher g1: " + matcher.group(1));
+//				System.out.println("Matcher g2: " + matcher.group(2));
+				if (variable.equals(matcher.group(1))) {


This approach is dangerous in that it doesn't consider the scope of the variable. I would do it differently, by parsing the code like jython's interpreter with a ParserFacade, see e.g. https://github.com/acardona/scripts/blob/master/python/imagej/jython/parse_code.py

Good point. Are you able to work on this? To me the priority would be to support he return type of the static method so I'd prefer to work on that instead (unless you say otherwise)

Yes, proper jython code parsing is something I've been working on but haven't committed yet. Will lead to:

autocompletion for variable names within scope, in addition to possible java class imports;

autocompletion and auto-import for python modules included in the jython jar or discoverable from its PYTHONPATH-equivalent;

type resolution for variables within scope, in order to autocomplete their methods. Won't always work (e.g. when an object derives from a call on a function argument), but it will work in many cases.

For the latter case, the AutoCompletionListener interface will allow e.g. @haesleinhuepf's update site to add variable name-based imports (e.g. for imp, ip, fp, etc.)

@acardona, Great! On a second thought I will hold for now, as you are likely to revamp a bunch of things. But do let me know if there is something you want me too look into. I won't work on this until I hear from you (so feel free to merge), but do let me know if you want me to look into something. I will post below a summary of what this (snowballed) PR now has.

acardona · 2020-12-04T18:42:18Z

 	static private final Pattern importPattern = Pattern.compile("^(from[ \\t]+([a-zA-Z_][a-zA-Z0-9._]*)[ \\t]+|)import[ \\t]+([a-zA-Z_][a-zA-Z0-9_]*[ \\ta-zA-Z0-9_,]*)[ \\t]*([\\\\]*|)[  \\t]*(#.*|)$"),
-								 tripleQuotePattern = Pattern.compile("\"\"\"");
+								 tripleQuotePattern = Pattern.compile("\"\"\""),
+								 variableDeclarationPattern = Pattern.compile("([a-zA-Z_][a-zA-Z0-9._]*)\\s*=\\s*([A-Z_][a-zA-Z0-9._]*)(?:\\()"); // E.g., in 'imp=ImagePlus()' grou1: imp; group2: ImagePlus


Not all kinds of whitespace are valid, for example a line break isn't valid if the line doesn't end with a backslash. That's why I used [ \t] instead of \s.

Good point. will fix.

acardona · 2020-12-04T18:44:17Z

-			String packageName = m2.group(3),
-					 className = m2.group(4); // incomplete or empty, or multiple separated by commas with the last one incomplete or empty
+			final String packageName = m2.group(3); // incomplete or empty, or multiple separated by commas with the last one incomplete or empty
+			String className = m2.group(4);


Not sure why this code was changed here? Seems like you have an opinionated code formatter. Now the comment is in the wrong place.

Yes. My Eclipse has a will of its own. I hate it when it does that. Will fix.

acardona · 2020-12-04T18:46:02Z

+
+				// a call to an instantiated class
+				final String[] varAndSeed = getVariableAnSeedAtCaretLocation();
+				if (varAndSeed == null) return Collections.emptyList();;


double semicolon.

…ontain_term auto-complete words that contain a term

As per: - scijava#51 (comment) - scijava#51 (comment)

This reverts commit b7d5201.

As per scijava#51 (comment)

Import as statement (Adds support for lower case aliases)

…pletion-vars

- Include parameters type in replacement text (`imp.draw(int, int, int, int)` vs `imp.draw(` - Improve summary - Display type of fields - Include javadoc link in description - Fix dup. text

...This facilitates access to other future providers (Groovy, etc.)

(also fixes missing .html suffix in URL)

At least for now. Somehow breaks variable completion This reverts commit 9a584df.

tferr · 2020-12-05T05:03:07Z

For clarity, here is what this PR implements:

Adds preliminary support for non-static methods. This is fragile and will be revamped by @acardona's ParserFacade approach)
Shows an error in the popup list if import is not found (rather than exception)
Does approximate string matching (exact matches are still displayed on top of list)
Revamps auto-completion descriptions:
- provides functional URLs to the class's JavaDoc
- lists type of parameters, fields and return values and includes them in the replacement String
Includes' @acardona revisions and comments
Moves some common code to ClassUtil, so that it can be reused by future Completion providers

Here are some snapshots:

Field:

Fuzzy string search (methods):

Invalid import:

I have to say, I find it quite functional even at this stage.
NB:

I haven't implemented the retrieval of class from the return type of the static method, as I expect @acardona work will move things around
Scijava #@ parameters type of imports are not yet supported
Hyperlinks could likely be improved by attaching the anchor for the field/method to the html address of the class
@haesleinhuepf , i had to revert your commit 9a584df. Not so sure why, but it broke some of the downstream auto-completion. This means lower case aliases in import statements are still not supported.
(all these are probably things that can be addressed later on)

At least for now. Somehow breaks variable completion This reverts commit 9a584df.

…pletion-vars

acardona · 2020-12-11T18:10:49Z

Hi @tferr, the code may have changed significantly. Would appreciate if you could revise your commits and see which can be applied. The branch I pushed forward is "fix-autocompletion-listener".

And @haesleinhuepf: notice I've had to change the AutoCompletionListener interface significantly. Please have a look, should be straightforward to adapt for CLIJ to insert expansions for commonly used variable names.

Currently, if there is e.g., a typo in an import declaration completion will fail with a ClassNotFoundException. This commit adds placeholders to the completion list to warn user about such cases,

(not just those which start with it)

As per: - scijava#51 (comment) - scijava#51 (comment)

This reverts commit b7d5201.

As per scijava#51 (comment)

- Include parameters type in replacement text (`imp.draw(int, int, int, int)` vs `imp.draw(` - Improve summary - Display type of fields - Include javadoc link in description - Fix dup. text

...This facilitates access to other future providers (Groovy, etc.)

(also fixes missing .html suffix in URL)

At least for now. Somehow breaks variable completion This reverts commit 9a584df.

tferr · 2021-01-05T18:50:31Z

@acardona, to include JavaDoc URLs and list types in the completions, I would have to modify DotAutoCompletions#get() to return both the completion String and the completion description (also a String). Are you OK with it?

acardona · 2021-01-05T20:48:13Z

Hi @tferr, sounds good, should then return a new object that has both as fields rather than just a String.

acardona · 2021-01-12T22:27:34Z

By the way @tferr, when done, after review/test, I'd appreciate very much if you could release it all: the jython-autocompletion and the script-editor, so that it all appears in the Fiji updater.

tferr · 2021-01-24T16:27:02Z

@acardona, sincere apologies for the delay. 'Real work' got in the way. I think I have it working, but still need to test it more seriously. Hyperlinks seem to be working, and the completion lists the type for both fields and methods. If there are no major issues I will push and release tonight.

There are still some (minor) issues like the .acess$000 entry below, but we can always tackle this at a later time point), otherwise it will never be ready

acardona · 2021-01-24T17:30:19Z

Sounds great! On the subclasses: a one-liner filtering for dollar signs '$', indicative of subclasses, would do, at the level of the returned stream or list of autocompletions.

... in https://github.com/fiji/jython-autocompletion While at it, do some code cleanup

…pletion-vars

tferr · 2021-01-25T03:59:26Z

@acardona, I don't seem to have write access to this repository. Could you please merge this AND release version 5.0.10 of the script editor? Otherwise, give me write permissions and I will do so. Thanks!

This file was accidentally added when merging #51 with commit 6c48389.

tferr added 3 commits December 3, 2020 21:44

Jython: Add auto-completion support for non-static methods!

3ef7091

Warn user on invalid imports

f7899f6

Currently, if there is e.g., a typo in an import declaration completion will fail with a ClassNotFoundException. This commit adds placeholders to the completion list to warn user about such cases,

Add parameters and definitions to the description of completions

422f160

Fix visibility of field

092a86c

auto-complete words that contain a term

686f15b

(not just those which start with it)

acardona reviewed Dec 4, 2020

View reviewed changes

haesleinhuepf and others added 13 commits December 4, 2020 20:29

added compatibility for import-as statements

b7d5201

allow classes and aliases with lower case first character

77c3664

Merge pull request #1 from haesleinhuepf/autocomplete_commands_that_c…

d2a6256

…ontain_term auto-complete words that contain a term

Suggestion list: Ensure entries starting with seed remain on top of list

121ad9c

Fix formatting issues

1f3bea6

As per: - scijava#51 (comment) - scijava#51 (comment)

Revert "added compatibility for import-as statements"

721fe91

This reverts commit b7d5201.

Restrict allowed whitespace in variable declaration

f9c6a69

As per scijava#51 (comment)

Merge pull request #2 from haesleinhuepf/import-as_statment

9a584df

Import as statement (Adds support for lower case aliases)

Merge remote-tracking branch 'tferr/autocompletion-vars' into autocom…

0d076ff

…pletion-vars

Improve autocompletions

daff927

- Include parameters type in replacement text (`imp.draw(int, int, int, int)` vs `imp.draw(` - Improve summary - Display type of fields - Include javadoc link in description - Fix dup. text

Refactoring: move generic code to ClassUtil...

e7870c0

...This facilitates access to other future providers (Groovy, etc.)

Use frames when accessing javadocs

89f5e08

(also fixes missing .html suffix in URL)

Revert "Merge pull request #2 from haesleinhuepf/import-as_statment"

b7c51fc

At least for now. Somehow breaks variable completion This reverts commit 9a584df.

tferr added 2 commits December 5, 2020 01:28

Revert "Merge pull request #2 from haesleinhuepf/import-as_statment"

155c8aa

At least for now. Somehow breaks variable completion This reverts commit 9a584df.

Merge remote-tracking branch 'tferr/autocompletion-vars' into autocom…

0050715

…pletion-vars

frauzufall and others added 2 commits December 16, 2020 10:56

[maven-release-plugin] prepare release script-editor-0.5.9

96da72a

Jython: Add auto-completion support for non-static methods!

34a2e05

tferr and others added 14 commits January 5, 2021 01:24

Warn user on invalid imports

b17cd7a

Currently, if there is e.g., a typo in an import declaration completion will fail with a ClassNotFoundException. This commit adds placeholders to the completion list to warn user about such cases,

Add parameters and definitions to the description of completions

49d37b0

Fix visibility of field

ce3bb7a

auto-complete words that contain a term

92dfdb3

(not just those which start with it)

added compatibility for import-as statements

1a3ffaa

allow classes and aliases with lower case first character

8d155fa

Suggestion list: Ensure entries starting with seed remain on top of list

8b4bf53

Fix formatting issues

05ee489

As per: - scijava#51 (comment) - scijava#51 (comment)

Revert "added compatibility for import-as statements"

0dfa760

This reverts commit b7d5201.

Restrict allowed whitespace in variable declaration

1d1f255

As per scijava#51 (comment)

Improve autocompletions

1e46e56

- Include parameters type in replacement text (`imp.draw(int, int, int, int)` vs `imp.draw(` - Improve summary - Display type of fields - Include javadoc link in description - Fix dup. text

Refactoring: move generic code to ClassUtil...

ae945c0

...This facilitates access to other future providers (Groovy, etc.)

Use frames when accessing javadocs

3751e7c

(also fixes missing .html suffix in URL)

Revert "Merge pull request #2 from haesleinhuepf/import-as_statment"

2ce9f88

At least for now. Somehow breaks variable completion This reverts commit 9a584df.

tferr added 3 commits January 24, 2021 22:40

Implement CompletionText and modify ClassUtil for upcoming changes ...

cd732e2

... in https://github.com/fiji/jython-autocompletion While at it, do some code cleanup

Merge remote-tracking branch 'tferr/autocompletion-vars' into autocom…

0e90654

…pletion-vars

Merge branch 'master' into autocompletion-vars

04e945c

tferr mentioned this pull request Jan 25, 2021

Pin to script-editor version 0.5.9 scijava/script-editor-jython#5

Closed

haesleinhuepf mentioned this pull request Jan 25, 2021

New Jython auto-completion pulldown filled wrongly #53

Closed

acardona merged commit 6c48389 into scijava:master Feb 12, 2022

imagejan added a commit that referenced this pull request Feb 14, 2022

Delete release.properties

f295d3b

This file was accidentally added when merging #51 with commit 6c48389.

Conversation

tferr commented Dec 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

imagesc-bot commented Dec 4, 2020

Uh oh!

tferr commented Dec 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

imagesc-bot commented Dec 4, 2020

Uh oh!

acardona commented Dec 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acardona commented Dec 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acardona Dec 4, 2020

Choose a reason for hiding this comment

Uh oh!

tferr Dec 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

acardona Dec 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tferr Dec 5, 2020

Choose a reason for hiding this comment

Uh oh!

acardona Dec 4, 2020

Choose a reason for hiding this comment

Uh oh!

tferr Dec 4, 2020

Choose a reason for hiding this comment

Uh oh!

acardona Dec 4, 2020

Choose a reason for hiding this comment

Uh oh!

tferr Dec 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

acardona Dec 4, 2020

Choose a reason for hiding this comment

Uh oh!

tferr Dec 5, 2020

Choose a reason for hiding this comment

Uh oh!

tferr commented Dec 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acardona commented Dec 11, 2020

Uh oh!

tferr commented Jan 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acardona commented Jan 5, 2021

Uh oh!

acardona commented Jan 12, 2021

Uh oh!

tferr commented Jan 24, 2021

Uh oh!

acardona commented Jan 24, 2021

Uh oh!

tferr commented Jan 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tferr commented Dec 4, 2020 •

edited

Loading

tferr commented Dec 4, 2020 •

edited

Loading

acardona commented Dec 4, 2020 •

edited

Loading

acardona commented Dec 4, 2020 •

edited

Loading

tferr Dec 4, 2020 •

edited

Loading

acardona Dec 4, 2020 •

edited

Loading

tferr Dec 4, 2020 •

edited

Loading

tferr commented Dec 5, 2020 •

edited

Loading

tferr commented Jan 5, 2021 •

edited

Loading

tferr commented Jan 25, 2021 •

edited

Loading