api: Advanced Component joining

[kezz]

This PR adds a new class, JoinConfiguration, which introduces a more advanced method of joining components. Essentially, this class would allow you to create a component like Online players: Bob, John, Harry and Martin. from a simple collection of names with ease. A join configuration contains a series of component parts:

• A separator to go between each component,
• A final separator to go between the final two components,
• A prefix to be prepended to the result,
• A suffix to be appended to the result, and
• An convertor to change each joined ComponentLike into a Component.
• A predicate to determine if each component is to be included.

There are also some unit tests to ensure everything works correctly.

[kezz]

Just made all those requested changes in addition to cleaning up the Javadoc a bit. I just have two further questions:

1. Could we replace the dl/dt/dd tags with just a normal unordered list? I know these tags are used elsewhere for similar purposes but they render terribly on web browsers and don't look very nice imo.
2. Is it okay that the use of the full word in JoinConfiguration conflicts with the shortened use of it elsewhere? For example, TextReplacementConfig.

[BomBardyGamer]

Looks great to me, would be very usefulness to be able to join components very easily. I think it might be useful though to be able to configure this from just a plain string as well as from component like objects, instead of having to construct a text component every time.

[kezz]

Changes since last commit:

• Support for creating lists separated by an Oxford comma for all you weirdos that use it
• Javadocs have been improved to look less ugly
• All methods now return Component instead of ComponentLike

---

Looks great to me, would be very usefulness to be able to join components very easily. I think it might be useful though to be able to configure this from just a plain string as well as from component like objects, instead of having to construct a text component every time.

Yeah I'm not sure about this - there's no place in the API that accepts components that also accepts methods to "easily" create components from strings, etc.

[astei]

I don't think we should deprecate this method - it is perfectly fine as a convenient shorthand.

[kashike]

Text replacement methods got the same treatment - deprecated in favour of configuration. It is easier to maintain JoinConfiguration and not worrying about updating things here too.

[astei]

I don't consider arbitrary configuration objects to be particularly idiomatic unless there's no good way to separate the logic from the configuration. To me, an API that joins things together is a natural fit for combining the two into a single interface with configuration and logic. (See a comment below for an example of my idealized version of the API.)

[kashike]

This PR is simply matching how we do this in the rest of the Adventure API, and I tend to stick with consistency with how to use things.

[zml2008]

This brings up a good point -- perhaps we should un-deprecate replaceText methods for the most simple case of Pattern -> Component, as a convenience option.

[kezz]

Another solution is to just add convenience methods to TextReplacementConfig similar to the separator methods in JoinConfiguration, for example.

[zml2008]

hm, I think if we were doing this from the start that would be good, but in the interest of minimizing API churn, I think maintaining the older methods would be the better way to go for both this and replacement.

[zml2008]

Thinking about this again, and with #418 I think replacing all these methods with one that takes a JoinConfiguration is the least awkward way forward.

[astei]

Ditto

[astei]

Would this API be more idiomatic as a builder, or a modifiable (albeit immutable) ComponentJoiner object?

  private static final ComponentJoiner PLUGIN_JOINER = Component.joiner(Component.text(", "))
    .prefix(prefix)
    .suffix(suffix);

  // ...

  public static void sendPluginList(Audience target) {
    target.sendMessage(PLUGIN_JOINER.join(getPluginNameComponents()));
  }

[astei]

There is precedent here: see Guava Joiner and Splitter which uses the pattern I've described here.

[kashike]

This PR is matching how we do this in the rest of the Adventure API, however.

[astei]

I'm still not a big fan of it. I guess it's something that can be revisited in Adventure 5, because I still believe this pattern feels unnatural to write.

[kezz]

I do agree that a shorthand method to join directly from a join configuration would be nice, an earlier version of the PR did include one. Although I do understand that the syntax doesn't differ too much whether you're doing config.join(components) or join(config, components) and the latter is certainly more in keeping with the rest of the API, such as the text replacement system.

[kezz]

Remaining questions and thoughts I have about this PR:

• Should an additional static method of creation for the configuration be made that includes the lastSeparatorIfSerial parameter (i.e. JoinConfiguration#separators(ComponentLike separator, ComponentLike lastSeparator, ComponentLike lastSeparatorIfSerial).
• Should these static methods just be completely dropped in favour of the builder? The equivalent TextReplacementConfig doesn't have any static methods like this.
• Is JoinConfiguration a good name or should it be JoinConfig in keeping with TextReplacementConfig?
• Should a utility method be created to allow config.join(components) to work?
• Can anyone think of a better name than lastSeparatorIfSerial?
• Do the most recent changes to the join system make sense? I believe it would be logical that if you put in a single component with no prefix or suffix that you would receive the exact component that you supplied. This does mean that the old join methods need a bit of hackery to wrap them up in a text component if the new join methods didn't return one. This might seem messier, but imo it is a lot more logical and prevents the addition of unneeded bulk.

[zml2008]

This brings up a good point -- perhaps we should un-deprecate replaceText methods for the most simple case of Pattern -> Component, as a convenience option.

[lucko]

Looks good but I agree with @astei re: the deprecations

[kezz]

Rebased and applied zml's suggestions.

As for the deprecation of the "shorthand" methods, I believe this PR should be merged with the deprecations in place, and the removal of the deprecations for this and the replaceText methods should be done together in another PR to avoid inconsistencies and to provide a more pertinent place for discussion.

[zml2008]

Looking back at some of your comments:

• JoinConfig is probably a better name
• The static methods on JoinConfig should be dropped, in favor of the existing methods on Component
• Why return TextComponent rather than just Component? The content of the result will always be empty, right?

[kezz]

• JoinConfig is probably a better name

Agreed, it's definitely more in keeping with TextReplacementConfig.

• The static methods on JoinConfig should be dropped, in favor of the existing methods on Component

Do you mean the methods like separators() and noSeparators()? Or the previously added join methods that have since been removed.

• Why return TextComponent rather than just Component? The content of the result will always be empty, right?

Yes, the return here is only done because that's what the old methods returned. TextComponent is never returned for any of the new methods added in this PR.

[zml2008]

Do you mean the methods like separators() and noSeparators()? Or the previously added join methods that have since been removed.

Yeah, I think those last two should be removed too.

Yes, the return here is only done because that's what the old methods returned. TextComponent is never returned for any of the new methods added in this PR.

oh oops, nvm that then

[kezz]

Just rebased this for a 4.9.0 release. I still think the questions regarding deprecation are valid, but I do still think they are better served in another PR that would undeprecate both sets of methods or one that provides shorthand methods for TextReplacementConfig.

[zml2008]

Can you deprecate TextComponent.ofChildren as well, to be replaced by join(JoinConfiguration.noSeparator())?

[zml2008]

Thinking about this again, and with #418 I think replacing all these methods with one that takes a JoinConfiguration is the least awkward way forward.