redhat-developer / quarkus-ls

Language server for Quarkus tooling
Eclipse Public License 2.0
43 stars 15 forks source link

Support for #include #insert and validation for closed template #810

Closed angelozerr closed 1 year ago

angelozerr commented 1 year ago

Support for #include #insert

Fixes #438

angelozerr commented 1 year ago

the QuteValidationWithClosedTemplate

If you open a Qute template (just to start the Qute LS) ,and you close it (or not), and you wait for several seconds, you should see all Qute errors of the project in the PROBLEMS view:

image

angelozerr commented 1 year ago

@mkouba @FroMage I'm supporting completion for template ids in #include. Here a screenshot with the https://github.com/FroMage/quarkus-renarde-todo/blob/7784cba6de7f31e937726eec6725df3c95eca1f6/src/main/resources/templates/Login/login.html#L1

image

As you can see, the completion shows the .html, .txt extensions, but in doc https://quarkus.io/guides/qute-reference#include_helper sample doesn't use the .html file extension. So I assume that we can write:

{#include base} or {#include base.html}

My question is what is the prefered syntax (with or without html?) Completion should show:

Should we have a settings to decide if 1), 2), or 3) should be done?

FroMage commented 1 year ago

Well, good question. I didn't know you could do {#include base} without extension. Does it find the correct extension in case of several alternatives?

I guess that's a question for @mkouba I never tried that. I should try it.

As for the completion, I'd start by suggesting that you put toplevel templates first, which will get you main.html (with or without extension, depending on the outcode of this discussion) first, because in my experience, includes are 100% always including base templates at the root, never anything else.

mkouba commented 1 year ago

Should we have a settings to decide if 1), 2), or 3) should be done?

That's a good question and I don't have a simple answer ;-).

Well, good question. I didn't know you could do {#include base} without extension. Does it find the correct extension in case of several alternatives?

* `{#include email}` in `Emails/hello.txt` will include `email.txt`

* `{#include email}` in `Emails/hello.html` will include `email.html`

I guess that's a question for @mkouba I never tried that. I should try it.

Currently, it does not reflect the selected variant/suffix of the including template but iterates over the suffixes defined in quarkus.qute.suffixes, i.e. the default value is qute.html,qute.txt,html,txt so the {#include email} will try email, email.qute.html, email.qute.txt, email.html and finally email.txt.

In theory, we could reflect this kind of information but if you think about it, when you're inside the Emails/hello.html then you know that you're going to include the html variant, right? That said, the variant without suffix is IMO easier to read.

It might make sense to provide the file with suffix if multiple variants exists...

angelozerr commented 1 year ago

Here a demo with #insert #include support (it should work when included document is opened or closed):

QuteIncludeDemo

@datho7561 @JessicaJHee you can start to play with the PR.

angelozerr commented 1 year ago

As for the completion, I'd start by suggesting that you put toplevel templates first,

Very good feedback! Done:

image

angelozerr commented 1 year ago

Thanks @mkouba for your feedback.

If I understand your idea, completion should show template id without suffix, and for the case of conflicts like email.txt, email.html, completion should show file extension.

@FroMage do you like this idea?

FroMage commented 1 year ago

That's a good idea, I think.

angelozerr commented 1 year ago

That's a good idea, I think.

Here the result:

image

FroMage commented 1 year ago

Looks great!

fbricon commented 1 year ago

looks like completion proposes the current template (Application/index), which sounds like a bad idea

angelozerr commented 1 year ago

Looks great!

Glad the feature please you.

looks like completion proposes the current template (Application/index), which sounds like a bad idea

Good catch! Fixed:

image

datho7561 commented 1 year ago

The validation for unopened templates doesn't appear until you open the first Qute template. Is this intentional? I think it could be interesting to validate all the templates as soon as the language server is able to.

angelozerr commented 1 year ago

The validation for unopened templates doesn't appear until you open the first Qute template. Is this intentional?

No:)

I think it could be interesting to validate all the templates as soon as the language server is able to.

Totally agree with you! I created the issue https://github.com/redhat-developer/quarkus-ls/issues/813

angelozerr commented 1 year ago

I wonder if it's a good idea to show the elements we could insert in the completion first?

@datho7561 @FroMage @mkouba what do you think about this idea? If I understand your idea, completion should show:

I also was wondering if we support the simple example in https://quarkus.io/guides/qute-reference#include_helper. I'm not quite sure how it works.

It should work, what is your problem?

datho7561 commented 1 year ago

It should work, what is your problem?

Maybe this is a different issue, but what I'm seeing is if I have the template "other-template.html":

<h3>{#insert myTitle}MY TITLE :D{/}</h3>

<p>
{#insert}
    this is the paragraph used in this template
{/insert}
</p>

Then that means I can pass in a myTitle when I {#include} the template that will replace the "MY TITLE :D" value:

{#include other-template.html myTitle="New Title"}
  Content
{/}

However, right now we don't have completion or validation for myTitle when we include "other-template.html". Is this expected? If so, then I think it's a good idea to open issues for that.

I misread the documentation, this isn't how it works. My bad.

angelozerr commented 1 year ago

However, right now we don't have completion or validation for myTitle when we include "other-template.html". Is this expected? If so, then I think it's a good idea to open issues for that.

Indeed parameter in include is not supported, please create a new issue for that.

FroMage commented 1 year ago

Indeed parameter in include is not supported, please create a new issue for that.

That's now how we pass parameters to included files, though.

angelozerr commented 1 year ago

That's now how we pass parameters to included files, though.

Thanks for your feedback. We have created the issue for that https://github.com/redhat-developer/quarkus-ls/issues/815