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.

Sign up for GitHub

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

Jump to bottom

ANSIComponentSerializer #898

Merged
merged 9 commits into from
Apr 25, 2023
Merged

ANSIComponentSerializer #898

merged 9 commits into from
Apr 25, 2023

Conversation

rymiel
Copy link
Member

@rymiel rymiel commented Apr 10, 2023 •
edited by zml2008
Loading

Supersedes #140
Fixes KyoriPowered/ansi#7
Fixes #125

@rymiel rymiel self-assigned this Apr 10, 2023
@rymiel rymiel force-pushed the feat/ansi-component-serializer branch 2 times, most recently from b4f56a3 to 8d7c3f6 Compare April 10, 2023 01:12
@rymiel rymiel marked this pull request as ready for review April 10, 2023 01:17
@rymiel rymiel requested a review from zml2008 April 10, 2023 01:18
zml2008
zml2008 requested changes Apr 10, 2023
View reviewed changes
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.

can't rly words rn but some initial thoughts

...zer-ansi/src/main/java/net/kyori/adventure/text/serializer/ansi/ANSIComponentSerializer.java Outdated Show resolved Hide resolved
...zer-ansi/src/main/java/net/kyori/adventure/text/serializer/ansi/ANSIComponentSerializer.java Outdated Show resolved Hide resolved
...zer-ansi/src/main/java/net/kyori/adventure/text/serializer/ansi/ANSIComponentSerializer.java Outdated Show resolved Hide resolved
@rymiel rymiel requested a review from zml2008 April 10, 2023 19:08
zml2008
zml2008 requested changes Apr 11, 2023
View reviewed changes
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 pretty close, just a few nits here!

We should see about introducing an extra interface for these sorts of serializers that only defines the serialize() method -- maybe ComponentEncoder? or something? We've already used up serializer and renderer lol

...zer-ansi/src/main/java/net/kyori/adventure/text/serializer/ansi/ANSIComponentSerializer.java Outdated Show resolved Hide resolved
...zer-ansi/src/main/java/net/kyori/adventure/text/serializer/ansi/ANSIComponentSerializer.java Outdated Show resolved Hide resolved
...ansi/src/main/java/net/kyori/adventure/text/serializer/ansi/ANSIComponentSerializerImpl.java Outdated Show resolved Hide resolved
...ansi/src/main/java/net/kyori/adventure/text/serializer/ansi/ANSIComponentSerializerImpl.java Outdated Show resolved Hide resolved
...zer-ansi/src/main/java/net/kyori/adventure/text/serializer/ansi/ANSIComponentSerializer.java Outdated Show resolved Hide resolved
rymiel added a commit that referenced this pull request Apr 11, 2023
@rymiel
feat(api): add ComponentEncoder
1acc0f3 
ComponentEncoder is akin to ComponentSerializer, except it doesn't
provide deserialization. This is intended to be used in #898.

ComponentSerializer now extends ComponentEncoder, but still overrides
the methods with exact copies, to keep the javadocs more clear.
These originals also retain their old `@since` tags.
@rymiel rymiel mentioned this pull request Apr 11, 2023
Merged
rymiel added a commit that referenced this pull request Apr 11, 2023
@rymiel
feat(api): add ComponentEncoder
2d073b3 
ComponentEncoder is akin to ComponentSerializer, except it doesn't
provide deserialization. This is intended to be used in #898.

ComponentSerializer now extends ComponentEncoder, but still overrides
the methods with exact copies, to keep the javadocs more clear.
These originals also retain their old `@since` tags.
@rymiel rymiel force-pushed the feat/ansi-component-serializer branch from 268ecd3 to 3f20aaa Compare April 14, 2023 14:57
@rymiel rymiel requested a review from zml2008 April 14, 2023 14:57
zml2008
zml2008 approved these changes Apr 24, 2023
View reviewed changes
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.

Looks good to merge as soon as you bump the ansi dependency to the release version :)

@zml2008 zml2008 added this to the 4.14.0 milestone Apr 24, 2023
zml2008
zml2008 requested changes Apr 25, 2023
View reviewed changes
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.

ok one last thing sorry :(

...zer-ansi/src/main/java/net/kyori/adventure/text/serializer/ansi/ANSIComponentSerializer.java Outdated Show resolved Hide resolved
@rymiel rymiel requested a review from zml2008 April 25, 2023 12:46
@rymiel rymiel added this pull request to the merge queue Apr 25, 2023
Merged via the queue into main/4 with commit 62ca6f4 Apr 25, 2023
@rymiel rymiel deleted the feat/ansi-component-serializer branch April 25, 2023 15:28
@qixils qixils mentioned this pull request Jun 15, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zml2008 zml2008 zml2008 approved these changes

Assignees

@rymiel rymiel

Labels
area: text-serialiser/ansi type: enhancement
Projects
None yet
Milestone
4.14.0
2 participants
@rymiel @zml2008