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

Add Writer and ToSmolStr #52

Merged
merged 1 commit into from
Jan 16, 2024
Merged

Add Writer and ToSmolStr #52

merged 1 commit into from
Jan 16, 2024

Conversation

novacrazy
Copy link
Contributor

Provides a fmt::Write Writer that will try not to allocate immediately, a trait similar to ToString that makes use of the Writer, and a macro format_smolstr!() that is similar to format!().

Cannot use specialization with ToSmolStr, sadly, as ToString does.

@novacrazy
Copy link
Contributor Author

Is there any chance of getting this added? It's such a waste to allocate a short string for formatting just to immediately convert it to an inline string.

@novacrazy
Copy link
Contributor Author

Actually, though github says it's fine, I do need to update it to 0.2. Will do that today.

src/lib.rs Outdated Show resolved Hide resolved
Copy link
Member

@Veykril Veykril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for not getting back earlier. Looks good to me, if you could rebase on the recent changes that would be great. Then I'll merge this.

@novacrazy
Copy link
Contributor Author

Github closed this automatically when I pushed the refreshed branch. Should be good to go now, though.

@novacrazy novacrazy reopened this Jan 15, 2024
@novacrazy
Copy link
Contributor Author

I went ahead and discarded the other cleanup changes, focusing on the actual feature, could open a new PR for those next.

@Veykril Veykril merged commit 1192514 into rust-analyzer:master Jan 16, 2024
2 checks passed
@Veykril
Copy link
Member

Veykril commented Jan 16, 2024

Feel free to open another PR for those.
Thanks!

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

Successfully merging this pull request may close these issues.

2 participants