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

feat(text-minimessage): Experimental MM template processor to try out new string templates #975

Draft
wants to merge 1 commit into
base: main/4
Choose a base branch
from

Conversation

zml2008
Copy link
Member

@zml2008 zml2008 commented Sep 23, 2023

This PR is unlikely to be merged in the near future, it's mostly here as an experiment with the new preview feature in Java 21.

Some open questions for once this becomes stable are:

  • Should this be part of adventure proper? maybe a new template processors module with a higher Java requirement?
  • How well do the restrictions of the string template format mesh with MM syntax? String templates are pretty linear, do we want
  • Is it worth having customizable template processor instances that map values differently?

I'd also like to work on a template processor for LinearComponents at some point. That seems like something more likely to mesh well with the linear string template format.

how this currently works

We try to map template parameters to MM tags. This is currently:

  • ComponentLike -> selfClosingInserting
  • a wrapper for pre process parsed tags -> preProcessParsed of the inner content
  • anything StyleBuilderApplicable -> styling
  • anything else -> a selfClosingInserting tag of a Component.text() containing the string value of the template value

Notably, most of these intentionally have no impact on following content.

The tests are a good starting point for playing with the format -- what do people think?

@zml2008 zml2008 added type: enhancement status: blocked area: text-minimessage status: needs discussion An issue or PR that requires design decisions or further consensus building before it can be merged labels Sep 23, 2023
@Minikloon
Copy link

Came up with my own before someone showed me this Draft: https://gist.github.com/Minikloon/e0ec3a6ada42c127a2d4caf7cd8f0686

I don't know if mine is just simpler or I'm naive.

@zml2008
Copy link
Member Author

zml2008 commented Mar 20, 2024

The logic in this processor allows passing Components directly into the string, so you could do something like MM."<red>Hello \{player.displayName()}" and it'd be formatted correctly. Yours would just toString it.

@Sparky983
Copy link

Sparky983 commented May 2, 2024

Should this be part of adventure proper? maybe a new template processors module with a higher Java requirement?

The current use of a multi-release JAR is technically unsupported, so probably not. The JAR File Spec requires that "[the] public API exported by the classes in a multi-release JAR file must be exactly the same across versions", however that requirement is not verified due to it being "difficult and costly to perform".

Additionally, implementation of this change may become more difficult once we get the new string templates. The JDK has taken the approach of creating StringTemplate overloads to the appropriate methods. If the Mini Message were to take the same approach (i.e. MiniMessage.deserialize(StringTemplate, TagResolver...)), it would necessitate a Java baseline that includes the new string template API due to the same multi-release JAR issues I mentioned above (because of differing APIs).

@PukPukov
Copy link

Java 23 nuked string templates, lol.

@zml2008
Copy link
Member Author

zml2008 commented Jun 24, 2024

please read the mailing list comments before making uninformed comments - the plan is to make significant changes, not remove them entirely forever

@PukPukov
Copy link

please read the mailing list comments before making uninformed comments - the plan is to make significant changes, not remove them entirely forever

As far as I understand, this is not forever, but yes, they removed it from jdk 23 because they had not enough time to implement them properly again until jdk 23 will be branched. https://www.youtube.com/watch?v=c6L4Ef9owuQ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: text-minimessage status: blocked status: needs discussion An issue or PR that requires design decisions or further consensus building before it can be merged type: enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants