Repository navigation
Validate single quotes in themes - #37931
Conversation
caa49cc to
c3f1d43
Compare
e48891a to
8831a24
Compare
There was a problem hiding this comment.
I think rather than solving this here on the front-end the API should normalize these values. This will avoid other consumers from having to do the same.
There was a problem hiding this comment.
I was thinking the same first, but then wasn't sure if someone might want to use the original Java Messageformat to do a replacement. WDYT - should I still do it only in the backend?
There was a problem hiding this comment.
My vote is to do it on the backend. Unless I am somehow misunderstanding the formatting here. What is the purpose of the double ''? I thought it was properties file specific trait, and not related to MessageFormat.
There was a problem hiding this comment.
The '' is related to MessageFormat only, that is when you have placeholders in the string. Not all strings in the property file are then processed by a MessageFormat.
The logic of MessageFormat in a nutshell: If you want to escape a something that would normally be a placeholder, put it in single quotes. It will then be printed without the single quotes. If you then want to print one single quote, have two single quotes in the original string.
I assume no-one is consume this strings except the original admin UI and account UI, so it doesn't really matter. I'll move it to the backend in the next commit.
There was a problem hiding this comment.
Yeah, sounds like it can be handled in the API then. At some point we should really look to unify these messages. There is a standard MessageFormat API proposal in the works for JavaScript that will implement the MessageFormat 2 standard.
8831a24 to
a2803c4
Compare
|
@jonkoops - I gave it a thought to convert the message resources on the backend, but then it would affect all resource themes, not only those used in the React UI. So I chose to update the docs that in Admin/Account UI theme you MUST NOT use WDYT? I think it is a step in the right direction, but there is a lot more to clean up around translations... |
There was a problem hiding this comment.
I agree with the approach of using the MessageFormat syntax only for the Java related messages and adding some instructions. Long-term, we need to reach an agreement on a unified format for translations.
There was a problem hiding this comment.
As per the mailing list, we no longer need to include copyright notices in every file.
|
Thank you for the approval of this next step in the journey. There are some conflicts that I need to resolve. As this one is touching a lot of files, I've now locked the Weblate translations. Once the last Weblate changes are merged, I'll rebase this one. Once this PR is merged, I'll unlock Weblate. |
Closes keycloak#37930 Signed-off-by: Alexander Schwartz <aschwart@redhat.com>
Closes keycloak#37930 Signed-off-by: Alexander Schwartz <aschwart@redhat.com>
a2803c4 to
b2d477f
Compare
|
@jmallach / @Ecron / @EdoardoTona / @GioviQ: With this change in place, you now need to check where you add quotes: While typographic quotes are preferred, it is |
|
Thanks for this specific clarification: people were saying all around that '' was always needed, but it was clearly not the case. We should try to search&replace all apostrophe's in our Catalan translations to use ‘ instead, and use ' only where it is really needed. |
|
@jmallach - we are learning and are untangling this bit by bit. Sometimes we might take a detour, or correct previous assumptions. We try to keep the To do the search-and-replace, you can choose to create a pull request for the files if you don't want to use the UI for that. |
Closes #37930