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

Manage client snippet on server side #251

Merged
merged 1 commit into from
Mar 23, 2020

Conversation

angelozerr
Copy link
Contributor

Manage client snippet on server side

Fixes redhat-developer/vscode-quarkus#119

Signed-off-by: azerr azerr@redhat.com

@angelozerr
Copy link
Contributor Author

This PR manages Quarkus snippets on server side. Snippets are the same than vscode snippet except that it adds a context to show the snippet according the project dependency. Ex : qds appears only if there is quarkus-agroal in the classpath:

"context": {
	"extension": "quarkus-agroal"
}

For the moment it's a draft PR since I must comment all codes and write tests. You can play with this PR and you should see qds twice (from vscode snippet and from MicroProfile LS) and compare the 2 behaviors. It should be the same except qds (from MicroProfile LS) should appear only if quarkus-agroal is on the classpath.

When this PR will be merged, we could methe the PR from vscode-quarkus redhat-developer/vscode-quarkus#228 which removes those snippets on client side.

@angelozerr angelozerr force-pushed the snippet branch 9 times, most recently from c23564f to 901fd52 Compare March 20, 2020 15:35
@angelozerr
Copy link
Contributor Author

Here a demo, you can see that there one qds (client side) when there is not dependency with quarkus-agroal, when you add it, you should see two qds snippets (client and server side):

QDSSnippetDemo

To test this PR:

  • check that qds (server) appears only when project has quarkus-agroal dependency
  • check open qds completion on server side has the same behavior than qds completion on client side (check qds are provided in same time if you type only, q, and qd, and other tests)
  • check apply completion is the same behavior for cliengt and server snippet.

Once this PR will be merged, we will able to to remove client snippets.

@angelozerr
Copy link
Contributor Author

This PR includes too snippets for java files mp rest and mp openapi snippets written by @rzgry #246 (comment)

OperationSnippetDemo

@rzgry do you want I remove your snippets from this PR? Or is it OK to keep it and create a new PR to update them?

Today it should work like vscode snippet client, but I need to manage filter :

  • filter according dependency (show open api snippet only if project have mp pen api dependency)
  • filter according to the completion offset (ex: show operation snippet only if completion offset is before a class,method declaration)

@rzgry
Copy link
Contributor

rzgry commented Mar 20, 2020

@angelozerr its fine for you to keep the snippets in this PR. I can open other PRs to update the existing ones / add snippets for other microprofile specifications

@angelozerr
Copy link
Contributor Author

@angelozerr its fine for you to keep the snippets in this PR.

Nice, I will write some tests around those snippets. I think this PR will not manage filter, I would like to do that in an another PR. When we did that we will update snippets json to manage context.

@xorye
Copy link

xorye commented Mar 20, 2020

The application.prooperties snippets work well on my machine.

For the Java snippets, I realized that the server side snippets do not adhere to the current indentation.

I noticed this when invoking a snippet inside of a class:
gif of indentation issue

whereas the vs code snippet follows the current indentation
gif of vs code snippet

@xorye
Copy link

xorye commented Mar 20, 2020

Another comment about the Java snippets:
I'm not sure if this is a limitation with VS Code, but sometimes I have a difficult time invoking snippets like @Metric.

If VS Code provides completion after the @ is typed, as I type Metric, the snippet does not appear.
snippet gif
To have the snippet appear, I have to press Escape to cancel the completion and invoke completion again.

@angelozerr angelozerr force-pushed the snippet branch 3 times, most recently from 0e7711d to 673241c Compare March 22, 2020 16:47
@angelozerr angelozerr marked this pull request as ready for review March 22, 2020 16:48
@angelozerr
Copy link
Contributor Author

I realized that the server side snippets do not adhere to the current indentation.

It should be fixed.

f VS Code provides completion after the @ is typed, as I type Metric, the snippet does not appear.

It should be fixed.

The PR is now ready, I write tests too.

The load of snippets is done by Java SPI (see https://github.com/redhat-developer/quarkus-ls/pull/251/files#diff-56d60c257da46fc9af0a48f34ca4798eR67) and for the moment Quarkus snippets are hosted into MicroProfile LS too. We need to create a new project com.redhat.quarkus.ls which provides by SPI those snippets, but I think we could do that when MicroProfile LS will move to Eclipse.org. @fbricon are you OK with that?

Today the snippet of application.properties are shown according to the dependency of the project. For java snippets for the momen tit is not done. I would like to manage that in an another PR.

@angelozerr angelozerr force-pushed the snippet branch 2 times, most recently from 4fe65dc to 7ef830c Compare March 23, 2020 14:53
@xorye
Copy link

xorye commented Mar 23, 2020

The new fixes work great on my machine.

Please fix this failed test:

Running com.redhat.microprofile.services.ApplicationPropertiesCompletionTest
Tests run: 17, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 2.8 sec <<< FAILURE!
completionOnProfile(com.redhat.microprofile.services.ApplicationPropertiesCompletionTest)  Time elapsed: 0.007 sec  <<< FAILURE!
java.lang.AssertionError: expected:<3> but was:<5>
        at org.junit.Assert.fail(Assert.java:88)
        at org.junit.Assert.failNotEquals(Assert.java:834)
        at org.junit.Assert.assertEquals(Assert.java:645)
        at org.junit.Assert.assertEquals(Assert.java:631)
        at com.redhat.microprofile.services.MicroProfileAssert.assertCompletions(MicroProfileAssert.java:176)
        at com.redhat.microprofile.services.MicroProfileAssert.testCompletionFor(MicroProfileAssert.java:161)
        at com.redhat.microprofile.services.MicroProfileAssert.testCompletionFor(MicroProfileAssert.java:122)
        at com.redhat.microprofile.services.ApplicationPropertiesCompletionTest.completionOnProfile(ApplicationPropertiesCompletionTest.java:138)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
        at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
        at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
        at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
        at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
        at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
        at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
        at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
        at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
        at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
        at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
        at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:252)
        at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:141)
        at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:112)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray(ReflectionUtils.java:189)
        at org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:165)
        at org.apache.maven.surefire.booter.ProviderFactory.invokeProvider(ProviderFactory.java:85)
        at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:115)
        at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:75)

@angelozerr angelozerr force-pushed the snippet branch 2 times, most recently from 7dafda3 to 5d1ca99 Compare March 23, 2020 16:28
@angelozerr
Copy link
Contributor Author

Please fix this failed test:

fixed.

microprofile.ls/README.md Outdated Show resolved Hide resolved
microprofile.ls/README.md Outdated Show resolved Hide resolved
microprofile.ls/README.md Outdated Show resolved Hide resolved
microprofile.ls/README.md Outdated Show resolved Hide resolved
microprofile.ls/README.md Outdated Show resolved Hide resolved
microprofile.ls/README.md Outdated Show resolved Hide resolved
microprofile.ls/README.md Outdated Show resolved Hide resolved
microprofile.ls/README.md Outdated Show resolved Hide resolved
microprofile.ls/README.md Outdated Show resolved Hide resolved
microprofile.ls/README.md Outdated Show resolved Hide resolved
microprofile.ls/README.md Outdated Show resolved Hide resolved
microprofile.ls/README.md Outdated Show resolved Hide resolved
microprofile.ls/README.md Outdated Show resolved Hide resolved
microprofile.ls/README.md Outdated Show resolved Hide resolved
microprofile.ls/README.md Outdated Show resolved Hide resolved
Fixes redhat-developer/vscode-quarkus#119

Signed-off-by: azerr <azerr@redhat.com>
@xorye xorye merged commit faa3dd1 into redhat-developer:master Mar 23, 2020
@xorye xorye added this to the v0.0.6 milestone Mar 23, 2020
@xorye xorye added the enhancement New feature or request label Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Manage client snippet on server side
3 participants