Skip to content

Conversation

@MichaReiser
Copy link
Member

Summary

This is a small refactor to simplify AnyStringPart. I always found it difficult to reason about how the quoting flows through the formatting
and this was part due to it being routed to parts, AnyStringPart and finally its Format implementation.

This PR makes AnyStringPart a simple enum over the different parts as it used to be and moves the logic for
formatting a part into the FormatImplicitConcatenatedString, the only place where it is used.

Test Plan

cargo test

@MichaReiser MichaReiser added internal An internal refactor or improvement formatter Related to the formatter labels Oct 8, 2024
@MichaReiser MichaReiser force-pushed the micha/refactor-any-string-part branch from 7356e4c to f903e30 Compare October 8, 2024 15:08
pub(crate) fn is_multiline(self, source: &str) -> bool {
match self {
AnyString::String(_) | AnyString::Bytes(_) => {
self.parts(Quoting::default())
Copy link
Member Author

Choose a reason for hiding this comment

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

This is kind of what triggered the refactor. It felt off that it is necessary to pass an arbitrary Quoating to iterate over the parts.

@MichaReiser MichaReiser force-pushed the micha/refactor-any-string-part branch from 47c1630 to f903e30 Compare October 8, 2024 15:17
@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2024

ruff-ecosystem results

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@MichaReiser MichaReiser changed the title Remove layout values from `AnyStringPart Remove layout values from AnyStringPart Oct 8, 2024
Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

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

Thanks!

@MichaReiser MichaReiser merged commit b9827a4 into main Oct 9, 2024
@MichaReiser MichaReiser deleted the micha/refactor-any-string-part branch October 9, 2024 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

formatter Related to the formatter internal An internal refactor or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants