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.

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

Improve completion for binary infix operator #941

Conversation

datho7561
Copy link

@datho7561 datho7561 commented Nov 8, 2024

In order to accomplish this,I have changed the logic for adjusting the offset used for the node search.

This causes some regressions:

  • A few cases where the test expects the AST not to be recovered but javac does recover it (eg. org.eclipse.jdt.core.tests.model.CompletionTests_1_5.test0197)
  • @| in Javadoc is not handled. Before it got no completion due to not finding the correct node, but now it gets default completion, which causes some regressions. I'll fix these when dealing with @| properly
  • The test cases sometimes expect the current class to be suggested in new | completion even when the type doesn't match, but sometimes they don't. I'll need to investigate further

@datho7561 datho7561 force-pushed the dom-with-javac-completino-in-binary-operator branch 2 times, most recently from 91b1e16 to 4665375 Compare November 13, 2024 21:53
@datho7561 datho7561 changed the title [WIP] improve completion for binary infix operator Improve completion for binary infix operator Nov 13, 2024
@datho7561 datho7561 force-pushed the dom-with-javac-completino-in-binary-operator branch from 4665375 to 888e191 Compare November 14, 2024 16:12
@datho7561 datho7561 marked this pull request as ready for review November 14, 2024 16:22
@datho7561
Copy link
Author

Going to merge this if the test failures go down

In order to accomplish this,
I have changed the logic for adjusting the offset used for the node search.

This causes some regressions:
- A few cases where the test expects the AST not to be recovered but javac does recover it (eg. org.eclipse.jdt.core.tests.model.CompletionTests_1_5.test0197)
- @| in Javadoc is not handled. Before it got no completion due to not finding the correct node, but now it gets default completion, which causes some regressions. I'll fix these when dealing with @| properly
- The test cases sometimes expect the current class to be suggested in new | completion even when the type doesn't match, but sometimes they don't. I'll need to investigate further

Signed-off-by: David Thompson <[email protected]>
@datho7561 datho7561 force-pushed the dom-with-javac-completino-in-binary-operator branch from 888e191 to 657896c Compare November 14, 2024 16:37
@datho7561
Copy link
Author

Test failures go up by the expected amount (5 new passes 6 new failures, net 1 new failure), merging.

@datho7561 datho7561 merged commit fedf23d into eclipse-jdtls:dom-with-javac Nov 14, 2024
4 of 5 checks passed
@datho7561 datho7561 deleted the dom-with-javac-completino-in-binary-operator branch November 14, 2024 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant