Skip to content

Expose ArgumentReader#skip() #2596

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

Open
wants to merge 2 commits into
base: api-12
Choose a base branch
from

Conversation

MrSlimeDiamond
Copy link

@MrSlimeDiamond MrSlimeDiamond commented May 28, 2025

Sponge | SpongeAPI

@aromaa
Copy link
Member

aromaa commented Jun 9, 2025

This should follow the convention we already use for the other skip methods, so skipChar. You most likely also want to couple this with peekChar to have this to be more usably.

I'm also not opposed to just have a method to set the cursor position directly.

@MrSlimeDiamond
Copy link
Author

It looks like there's already an ArgumentReader#peekCharacter method, but Mutable's methods use a char suffix instead of character. (for example parseChar). Perhaps this should be made consistent?

I'm happy to rename it to skipChar, but this means I'd need to open a PR to Sponge

@aromaa
Copy link
Member

aromaa commented Jun 10, 2025

Sigh, there's also peekString inside the Mutable instead of being in the base interface which I was glancing at for other similar functions. We can't accept breaking changes in 12 so that shouldn't be done here. Let's just rename it to skipChar to keep it consistent with the Mutable inteface.

@MrSlimeDiamond
Copy link
Author

@aromaa Updated, there's now a Sponge PR to implement this

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

Successfully merging this pull request may close these issues.

2 participants