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

Fragment support between Java / Template file #769

Merged
merged 1 commit into from
Jan 23, 2023

Conversation

angelozerr
Copy link
Contributor

Fragment support between Java / Template file

See redhat-developer/vscode-quarkus#563

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

@angelozerr
Copy link
Contributor Author

Here a demo with the current PR:

QuteFragmentDemo

@ia3andy
Copy link

ia3andy commented Jan 18, 2023

@angelozerr this looks amazing, great work!

@angelozerr angelozerr force-pushed the fragment-java branch 3 times, most recently from dafc10f to 0015610 Compare January 19, 2023 18:43
@angelozerr
Copy link
Contributor Author

@datho7561 if you have time you can start to review this PR. Code should be clean and there are tests for Qute LS side.

I need to write tests for Qute JDT side.

@datho7561 datho7561 self-requested a review January 19, 2023 19:16
@datho7561
Copy link
Contributor

datho7561 commented Jan 19, 2023

The link from a Java file to the template opens the template file, but it doesn't seem to place the cursor on the corresponding fragment declaration section. Do you think it's possible to do that?

It looks like the fragment id is being passed as a part of the code lens data

@@ -118,7 +121,7 @@ public boolean visit(FieldDeclaration node) {
.getLocationExpressionFromConstructorParameter(variable.getName().getIdentifier());
}
String fieldName = variable.getName().getIdentifier();
collectTemplateLink(node, locationExpression, getTypeDeclaration(node), null, fieldName);
collectTemplateLink(node, locationExpression, getTypeDeclaration(node), null, fieldName, false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't seem to get the links that appear on fields to show the fact that they link to a fragment. For instance if I have the field:

@Inject
@Location("myCoolTemplate.html")
Template myCoolTemplate$myFragment;

I get a code lens that links to the template and clicking it works as expected, but nothing is listed about the fragment in the code lens message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it allowed to write a field with $ to support fragment? I have not seen that in the doc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then ignoreFragments should be true here, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to my understand of the doc, the field name (which can contain $) has not the same behavior of method name of CheckedTemplate.

See https://quarkus.io/guides/qute-reference#fragments

@Inject
Template item;

String useTheFragment() {
   return item.getFragment("item_aliases") 
            .data("aliases", List.of("Foo","Bar")) 
            .render();
}

For field, fragment is supported when Template#getFragment is called. I don't support this usecase for the moment. But it makes no senses to write a Template instance field with $ to manage fragment.

@angelozerr angelozerr force-pushed the fragment-java branch 2 times, most recently from 43981e5 to b6c0bac Compare January 20, 2023 06:34
@angelozerr
Copy link
Contributor Author

The link from a Java file to the template opens the template file, but it doesn't seem to place the cursor on the corresponding fragment declaration section. Do you think it's possible to do that?

I think it is possible by implementing a custom command but I would like to do that in a separate PR.

It looks like the fragment id is being passed as a part of the code lens data

Indeed, it will be used in a future PR.

@angelozerr angelozerr force-pushed the fragment-java branch 2 times, most recently from df9e1c1 to 4159b14 Compare January 20, 2023 10:27
@angelozerr angelozerr marked this pull request as ready for review January 20, 2023 10:28
@angelozerr
Copy link
Contributor Author

angelozerr commented Jan 20, 2023

Ok now there are tests for codelens + diagnostics with fragment on Qute JDT side.

@datho7561
Copy link
Contributor

If I reference a fragment with id "" in a Java file, a CodeLens appears:

@CheckedTemplate()
public static class MyTemplate {
    public static native TemplateInstance myTemplate$();
}

However, if I try to declare a template with id "" in a Qute file, then it gets marked as an error, and no link back to the Java file gets created. I think we shouldn't create a CodeLens in the Java file in this case.

@datho7561
Copy link
Contributor

According to https://quarkus.io/guides/qute-reference#type_safe_fragments the fragment id is everything after the last $. However, if I have the follow:

public static native TemplateInstance myTemplate$otherText$fragment();

Then the CodeLens displays the fragment id as otherText$fragment instead of fragment

@angelozerr
Copy link
Contributor Author

However, if I try to declare a template with id "" in a Qute file, then it gets marked as an error, and no link back to the Java file gets created. I think we shouldn't create a CodeLens in the Java file in this case.

I think in this usecase, the method should be bound with the Qute html template src/main/resources/.../myTemplate$.html, no?

@datho7561
Copy link
Contributor

I think in this usecase, the method should be bound with the Qute html template src/main/resources/.../myTemplate$.html, no?

I don't think so. If I try to build the application with this in the code, then it gives the following compilation error:

[error]: Build step io.quarkus.qute.deployment.QuteProcessor#analyzeTemplates threw an exception: io.quarkus.qute.TemplateException: Fragment [] not defined in template GreetingResource/myTemplate.html

@angelozerr angelozerr force-pushed the fragment-java branch 3 times, most recently from 7d7800b to 73ca612 Compare January 23, 2023 09:52
@angelozerr
Copy link
Contributor Author

I don't think so. If I try to build the application with this in the code, then it gives the following compilation error:

Empty fragment should be take care of.

@datho7561 datho7561 self-requested a review January 23, 2023 16:05
Copy link
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Thanks, Angelo!

@datho7561 datho7561 added this to the 0.14.0 milestone Jan 23, 2023
@datho7561 datho7561 merged commit 820e5a9 into redhat-developer:master Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants