redhat-developer / quarkus-ls

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

Support for Qute Type-safe Message Bundles #845

Closed angelozerr closed 1 year ago

angelozerr commented 1 year ago

Support for Qute Type-safe Message Bundles

Fixes #800

angelozerr commented 1 year ago

The support for Qute Type-safe Message Bundles starts working with Java interfaces (but not with properties file).

See the following demo:

MessageWithJavaInterfaceDemo

@mkouba @FroMage I have started the support for Qute Type-safe Message Bundles which works only with Java interface. I need to support properties file.

Any feedback are welcome if you need some feature.

angelozerr commented 1 year ago

@mkouba I have tried to play with @MessageBundle and I have several questions:

No namespace resolver found for [msg] in expression {msg:message('hello_name',name)}

angelozerr commented 1 year ago

I have improved the support and now you can see message content as inlay hint in the template:

image

@mkouba @FroMage @fbricon @datho7561 @JessicaJHee for now I display as inlay hint the message but I wonder if it should be better to display the evaluaded message .

For instance:

{msg:hello_name('Lucie')} [Hello Lucie!]

If yes what about when name comes from a variable like

{msg:hello_name(person.name)}

Should we display:

mkouba commented 1 year ago

I have improved the support and now you can see message content as inlay hint in the template:

image

@mkouba @FroMage @fbricon @datho7561 @JessicaJHee for now I display as inlay hint the message but I wonder if it should be better to display the evaluaded message .

For instance:

{msg:hello_name('Lucie')} [Hello Lucie!]

If yes what about when name comes from a variable like

{msg:hello_name(person.name)}

Should we display:

* `{msg:hello_name(person.name)}  [Hello person.name!]`

* or don't evaluate to display `{msg:hello_name(person.name)}  [Hello {name}!]`

I think that we should display the message template and not the evaluaded message. Mainly because we are not always able to evaluate the message template and then it might become confusing... (the {msg:hello_name(person.name)} [Hello person.name!] example).

angelozerr commented 1 year ago

I think that we should display the message template and not the evaluaded message. Mainly because we are not always able to evaluate the message template and then it might become confusing... (the {msg:hello_name(person.name)} [Hello person.name!] example).

Ok thanks for your feedback @mkouba !

angelozerr commented 1 year ago

I need to clean my code and write tests. This PR provides the first support for Qute messages but it need to be improved with:

I will need that in separate PRs to avoid having a big PR.

@datho7561 @JessicaJHee you can start to play with this PR. Once tests and code will be cleaned I will notice you.

angelozerr commented 1 year ago

To play with Qute messages, I suggest that you use the qute-messages that I will use for Qute JDT tests.

angelozerr commented 1 year ago

Please note that I have encountered 2 bugs in Qute messages support:

FroMage commented 1 year ago

I think that we should display the message template and not the evaluaded message. Mainly because we are not always able to evaluate the message template and then it might become confusing... (the {msg:hello_name(person.name)} [Hello person.name!] example).

I don't know. I was going to answer the other way, and display the text substituted. At a point (in the view) where we don't know what {name} maps to, because we don't know the name of the parameters, I find it more useful to display Hello Lucie for constants, and Hello {person.name} for non-constant expressions.

angelozerr commented 1 year ago

I don't know. I was going to answer the other way, and display the text substituted. At a point (in the view) where we don't know what {name} maps to, because we don't know the name of the parameters, I find it more useful to display Hello Lucie for constants, and Hello {person.name} for non-constant expressions.

It was my original idea by @mkouba think we should not evaluate the message https://github.com/redhat-developer/quarkus-ls/pull/845#issuecomment-1493927666

angelozerr commented 1 year ago

@datho7561 @JessicaJHee the PR can be reviewed now, code is clean and tests exists now.

angelozerr commented 1 year ago

I played around and looks great!

Thanks!

I think this feature you mentioned would be very nice to have in a future PR:

It is in my plan, but this PR is already big, so I prefer providing message support in separate PRs to have easier review.

angelozerr commented 1 year ago

@JessicaJHee I have improved inlay hint and now you click on it to edit the Java message by going to the java method.

MessageInlayHintClickable

It requires to improve it to set the location to the @Message annotation content and not the Java method but we can do that in a separate PR (if you are interested you could work on it).

Once message properties support will be available https://github.com/redhat-developer/quarkus-ls/issues/847 my idea is to provide the capability to open the msg.properties file.

angelozerr commented 1 year ago

@datho7561 @JessicaJHee can we merge this PR?

datho7561 commented 1 year ago

I haven't had a chance to look at it yet, but I know that Jessica reviewed it. Do you think it's ready to merge, Jessica?