Generator preferred quote style#20434
Conversation
|
| fn round_trip_with( | ||
| indentation: &Indentation, | ||
| line_ending: LineEnding, | ||
| quote: Option<Quote>, |
There was a problem hiding this comment.
The documentation now needs updating
| } | ||
|
|
||
| #[test] | ||
| fn set_quote() { |
There was a problem hiding this comment.
| fn set_quote() { | |
| fn preferred_quote() { |
| indent: &'a Indentation, | ||
| /// The line ending to use. | ||
| line_ending: LineEnding, | ||
| /// Forced quote style to use. |
There was a problem hiding this comment.
The comment here is inaccurate. Setting quote doesn't force the generator to use the given quote everywhere. For example, setting Quote::Single for f'test{f"nested"}' doesn't change the quotes of the inner f-string. I also recommend adding a test showcasing this behavior
That's why I'd call this field preferred_quote. We should also extend the documentation to explain in more detail what it does (When None, the generator preserves the quote style when possible. When Some, the generator prefers the set quote style, disregarding the quote style used in the source)
There was a problem hiding this comment.
that's a better name for sure
|
|
||
| /// Force quote style for generated source code. | ||
| #[must_use] | ||
| pub fn with_quote(mut self, quote: Quote) -> Self { |
There was a problem hiding this comment.
Same as on the field documentation
| pub fn with_quote(mut self, quote: Quote) -> Self { | |
| pub fn with_preferred_quote(mut self, quote: Quote) -> Self { |
| let flags = if let Some(quote) = self.quote { | ||
| flags.with_quote_style(quote) | ||
| } else { | ||
| *flags | ||
| }; | ||
| self.p_str_repr(value, flags); |
There was a problem hiding this comment.
I think it's clearer if we move the self.quote override to self.p_str_repr instead of "patching" the flags here.
| if let Some(preferred_quote) = self.preferred_quote { | ||
| flags = flags.with_quote_style(preferred_quote); | ||
| } | ||
|
|
There was a problem hiding this comment.
Instead of patching the flags here, can we do something like this:
let quote_style = self.preferred_quote.unwrap_or(flags.quote_style());
let escape = UnicodeEscape::with_preferred_quote(s, quote_style);Do we need to add the same handling to:
unparse_interpolated_string_literal_elementp_bytes_reprunparse_interpolated_string
There was a problem hiding this comment.
That's a nice cleanup. will look into the other methods
| /// - If [`Some`], the generator will always use the specified quote style, | ||
| /// ignoring the one found in the source. | ||
| #[must_use] | ||
| pub fn with_preferred_quote(mut self, quote: Option<Quote>) -> Self { |
There was a problem hiding this comment.
@MichaReiser I've changed the signature to be able to take Option<Quote>, I think it makes more sense as it gives the ability to unset the preferred quote style & better aligns with the method's doc (IMO)
Closes #20432
Summary
Test Plan