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

Updates SyntaxStringBuilder to return itself #7244

Merged
merged 3 commits into from
Dec 15, 2024

Conversation

cheeezburga
Copy link
Member

@cheeezburga cheeezburga commented Dec 7, 2024

Description

Makes the SyntaxStringBuilder's append methods return itself.
Allows for stuff nicer feeling stuff like:

String toString = new SyntaxStringBuilder(event, debug)
    .append(x)
    .append("and")
    .append(y)
    .toString()

Target Minecraft Versions: any
Requirements: none
Related Issues: none

@Efnilite
Copy link
Member

Efnilite commented Dec 7, 2024

You can already chain arguments like this

builder.append(x, "and", y);

Which is more readable than chained append calls on a single line (builder.append(x).append("and").append(y)) and the example you provided. This is something me and sovde have already discussed :)

This also encourages ternaries inside the append methods, as this encourages putting everything into a single expression, instead of being spread out over multiple statements with explicit ifs, the latter of which has better readability. Essentially, this would allow the SSB to be used in the same way as the massive string concatenations used currently, but without the toString(event, debug).

This is against the point of the SSB, which makes the toString more readable by forcing you to be explicit, which does sacrifice brevity.

Because of the unexpected popularity of the SSB, we may want to consider adding support for easy one-liners to it, e.g.

new SyntaxStringBuilder(event, debug).single(x, "and", y);

Or similar. Personally, I support this, but a better solution should probably be discussed.

@cheeezburga
Copy link
Member Author

cheeezburga commented Dec 7, 2024

You can already chain arguments like this

builder.append(x, "and", y);

Which is more readable than chained append calls on a single line (builder.append(x).append("and").append(y)) and the example you provided. This is something me and sovde have already discussed :)

I still think there's merit in having it return itself. The behaviour you describe is still there obviously, but I think this just adds an alternative which could be useful. If I'm alone in this line of thinking, then I can close it, but it seems like there's no downside to it.

@Fusezion
Copy link
Contributor

Fusezion commented Dec 7, 2024

I still think there's merit in having it return itself. The behaviour you describe is still there obviously, but I think this just adds an alternative which could be useful. If I'm alone in this line of thinking, then I can close it, but it seems like there's no downside to it.

I prefer having .append() return, imo it's nicer and easier to read at times than seeing an array of arguments

@sovdeeth sovdeeth added enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. labels Dec 8, 2024
@cheeezburga cheeezburga requested a review from Efnilite December 9, 2024 12:33
@sovdeeth sovdeeth merged commit 88ae3dd into SkriptLang:dev/feature Dec 15, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants