Skip to content

Conversation

@Emilxyz
Copy link
Contributor

@Emilxyz Emilxyz commented May 5, 2025

This pull request adds a KeyFormat class to allow for easier creation of custom Keys using the API.

KeyFormat has following functionality:

  • Configuring a fallback namespace to be used when no other namespace is specified in the key parsing
  • Configuring a default separator to be used when parsing keys
  • KeyFormat#parse(String) which parses a input string using the configured separator and fallback namespace
  • KeyFormat#key(String) which creates a key using the configured namespace and the input string as value
  • KeyFormat#parseable(String) to check if a input string can be parsed using the configuration of the key format
  • KeyFormat#asString(Key) to turn a key into a String using the configuration
  • KeyFormat#asMinimalString(Key) to turn a Key into its minimal String using the configuration

Additionally, extends Key by following functionality:

  • Key#parseable(String, char) to specify the separator character when checking if a string can be parsed into a key
  • Key#parseable(String, char, String) to specify the fallback namespace used when none is provided in the input string
  • Key#key(String, char, String) to specify the fallback namespace used when none is provided in the input string

Closes #1222

@Emilxyz Emilxyz marked this pull request as ready for review May 6, 2025 09:34
@Emilxyz
Copy link
Contributor Author

Emilxyz commented Jun 22, 2025

As for #1249, im unsure on what best to name that method, as we can't have another #key(String). I see 2 options and need some input on them:

A:
The method to parse an input string is renamed to #parse(String).
#key(String) takes in the value only and always uses the configured namespace.

B:
#key(String) remains the method for parsing.
And the method taking only a value is named something like #value(String).

I like A more, just cause it makes more sense to me and sounds better (at least in my head). But, we would be breaking consistency with Key#key(String), so I would also agree in going with B. When going with B we would need to come up with a better name for that method as I really don't like #value, but it's the best I could come up with rn.

I understand that the methods in Key were named to be used as static imports, but none of these methods are static and always require an instance of a key format. So, in my opinion, it might be better to disregard the consistency and go with what makes more sense/sounds nicer.

@Emilxyz
Copy link
Contributor Author

Emilxyz commented Jun 22, 2025

Another thing I thought about recently, is that it might be better to move the actual logic of creating/checking parsability of keys into the key format class. All the overloads I added in Key could be removed then and the existing ones just forward into KeyFormat.minecraft().

@Emilxyz Emilxyz requested a review from Machine-Maker June 23, 2025 07:09
@Emilxyz
Copy link
Contributor Author

Emilxyz commented Jun 23, 2025

Went ahead and implemented option A now, can always just rename the methods again if required.

@imDaniX
Copy link

imDaniX commented Jun 25, 2025

For consistency, we can use the name parseKey instead, create Key#parseKey, deprecate Key#key(String) and Key#key(String, char), and move their responsibility to newly created Key#parseKey. It'll also allow for creating Key#parseKey(String keyStr, String fallbackNamespace).

But ofc, I'm not a maintainer, just my thoughts on the topic.

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.

Improve Key creation customization

4 participants