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(mini-message): add Formatter#joining #938

Open
wants to merge 4 commits into
base: main/4
Choose a base branch
from

Conversation

tahmid-23
Copy link

This PR adds a method to Formatter to join components using JoinConfiguration.

Copy link
Member

@zml2008 zml2008 left a comment

Choose a reason for hiding this comment

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

This is a nice addition -- just a few comments. Could you also open a PR on adventure-docs documenting this addition?

@tahmid-23
Copy link
Author

from what I can see, there is also a lastSeparatorIfSerial method which is mutually exclusive, in terms of functionality, with lastSeparator
should I add something like tag:separator:last_separator:serial? or tag:separator:last_separator:true? in order to use lastSeparatorIfSerial

@kezz
Copy link
Member

kezz commented Jun 26, 2023

from what I can see, there is also a lastSeparatorIfSerial method which is mutually exclusive, in terms of functionality, with lastSeparator should I add something like tag:separator:last_separator:serial? or tag:separator:last_separator:true? in order to use lastSeparatorIfSerial

IIRC lastSeparatorIfSerial is only used where there are three or more elements in the list (e.g. x, y, and z). It will still use lastSeparator in the case where there are two elements (e.g. x and y).

@Joo200
Copy link
Contributor

Joo200 commented Jun 26, 2023

I already use such a resolver for my project but I don't use a collection of ComponentLike. Instead I use a Collection of TagResolvers instead, with:

  public static TagResolver joining(@TagPattern final @NotNull String key, final @NotNull Iterable<? extends TagResolver> components) {
    return TagResolver.resolver(key, (arguments, ctx) -> {
        String format = arguments.popOr("expected list format").value();
        String separator = arguments.popOr("expected separator").value();
        if (arguments.hasNext()) {
            String emptyVal = arguments.pop().value();
            if (resolvers.isEmpty()) return Tag.inserting(ctx.deserialize(emptyVal));
        }
        List<Component> components = resolvers.stream().map(r -> ctx.deserialize(format, r)).toList();
        Component separatorComponent = ctx.deserialize(separator);
        Component all = components.stream().collect(Component.toComponent(separatorComponent));

        if (arguments.hasNext()) {
            int amount = arguments.popOr("maximum amount").asInt().orElse(5);
            String lastInfo = arguments.popOr("last entry").value();
            String hoverFormat = arguments.popOr("hover").value();
            if (resolvers.size() > amount) {
                Component hover = ctx.deserialize(hoverFormat,
                        Formatter.number("total", resolvers.size()),
                        Formatter.number("remaining", resolvers.size() - amount),
                        Placeholder.component("all", all));
                Component remaining = ctx.deserialize(lastInfo,
                        Formatter.number("total", resolvers.size()),
                        Formatter.number("remaining", resolvers.size() - amount)).hoverEvent(hover);
                return Tag.selfClosingInserting(components.stream().limit(amount).collect(Component.toComponent(separatorComponent)).append(remaining));
            }
        }
        return Tag.selfClosingInserting(all);
  });
}

My implementation is complex and I don't think we should use that as api. However you can easily change the format of the list entries, e.g. with such a minimessage format string

Governors: <gray><governors:'<online:green:gray><name></online>':', ':'<dark_gray>-- no one --</dark_gray>':'5':'<gold> and <remaining> more</gold>':'<gold>Total <total> governors:<br><gray><all>'>

This will be formatted to:

Governors: First, second, third, forth, fifth and 3 more.

Where and 3 more has an hover effect.

I don't think we should add such complex logic to minimessage however I would add the TagResolver approach as you can easily modify the formatting.

@tahmid-23
Copy link
Author

I added an argument for lastSeparatorIfSerial.

I'm not sure if TagResolvers are the best approach here.
Personally, I think I think that using Components would be more convenient. If I needed a placeholder, I could render it myself.

@tahmid-23 tahmid-23 changed the title feat(mini-message): add Formatter#join feat(mini-message): add Formatter#joining Jul 16, 2023
@tahmid-23
Copy link
Author

any love? 🙂

Copy link
Member

@kezz kezz left a comment

Choose a reason for hiding this comment

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

Minor changes that can be done before merge

@kezz kezz self-assigned this Oct 30, 2024
@kezz kezz added this to the 4.18.0 milestone Oct 30, 2024
Copy link
Member

@kezz kezz left a comment

Choose a reason for hiding this comment

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

Made the changes myself, LGTM :p

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants