-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Add struct formatting to sway-fmt-v2 #2058
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
Changes from 13 commits
5f357ff
a6a95a0
f404fa6
9918cf8
a446416
fbf0ba2
a109e91
d02cbcf
b5e1338
c9a1511
60da558
024f771
6dc832d
a825ae3
9a0eb78
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,210 @@ | ||
| use crate::fmt::{Format, FormattedCode, Formatter}; | ||
| use crate::{ | ||
| config::items::ItemBraceStyle, | ||
| fmt::{Format, FormattedCode, Formatter}, | ||
| utils::{ | ||
| bracket::{AngleBracket, CurlyBrace}, | ||
| item_len::ItemLen, | ||
| }, | ||
| }; | ||
| use sway_parse::ItemStruct; | ||
| use sway_types::Spanned; | ||
|
|
||
| impl Format for ItemStruct { | ||
| fn format(&self, _formatter: &mut Formatter) -> FormattedCode { | ||
| todo!() | ||
| fn format(&self, formatter: &mut Formatter) -> FormattedCode { | ||
| // TODO: creating this formatted_code with FormattedCode::new() will likely cause lots of | ||
| // reallocations maybe we can explore how we can do this, starting with with_capacity may help. | ||
| let mut formatted_code = FormattedCode::new(); | ||
|
|
||
| // Get the unformatted | ||
|
|
||
| // Get struct_variant_align_threshold from config. | ||
| let struct_variant_align_threshold = | ||
| formatter.config.structures.struct_field_align_threshold; | ||
|
|
||
| // Should small structs formatted into a single line. | ||
| let struct_lit_single_line = formatter.config.structures.struct_lit_single_line; | ||
|
|
||
| // Get the width limit of a struct to be formatted into single line if struct_lit_single_line is true. | ||
| let config_whitespace = formatter.config.whitespace; | ||
| let width_heuristics = formatter | ||
| .config | ||
| .heuristics | ||
| .heuristics_pref | ||
| .to_width_heuristics(&config_whitespace); | ||
| let struct_lit_width = width_heuristics.struct_lit_width; | ||
|
|
||
| let multiline = !struct_lit_single_line || self.get_formatted_len() > struct_lit_width; | ||
| format_struct( | ||
| self, | ||
| &mut formatted_code, | ||
| formatter, | ||
| multiline, | ||
| struct_variant_align_threshold, | ||
| ); | ||
| formatted_code | ||
| } | ||
| } | ||
|
|
||
| /// Format the struct if the multiline is passed as false struct will be formatted into a single line. | ||
| /// | ||
| /// Example (multiline : false): | ||
| /// struct Foo { bar: u64, baz: bool } | ||
| /// | ||
| /// Example (multiline : true): | ||
| /// struct Foo { | ||
| /// bar: u64, | ||
| /// baz: bool, | ||
| /// } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When writing doc comments, try to be considerate of the markdown formatting, even if it's just for private functions. I believe they can show up in an LSP-enabled editor with markdown formatting when hovered, and they can also be generated using Here's how the current formatting looks: Here's how it can look with some slight reformatting: /// Format the struct if the multiline is passed as false struct will be formatted into a single line.
///
/// ## Examples
///
/// (multiline : false):
///
/// ```rust,ignore
/// struct Foo { bar: u64, baz: bool }
/// ```
///
/// (multiline : true):
///
/// ```rust,ignore
/// struct Foo {
/// bar: u64,
/// baz: bool,
/// }
/// ```
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, that is really handy thanks. I will be using that to ensure that docs seem as I intended 👍
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kayagokalp did you want to add this, or should I go ahead and approve as is?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @eureka-cpu After merging this I will be completing #2097. So I was planning to fix the doc there so that mitch won't need to approve again but if you prefer the other way we can go that way too. |
||
| fn format_struct( | ||
| item_struct: &ItemStruct, | ||
| formatted_code: &mut FormattedCode, | ||
| formatter: &mut Formatter, | ||
| multiline: bool, | ||
| struct_variant_align_threshold: usize, | ||
| ) { | ||
| // If there is a visibility token add it to the formatted_code with a ` ` after it. | ||
| if let Some(visibility) = &item_struct.visibility { | ||
| formatted_code.push_str(visibility.span().as_str()); | ||
| formatted_code.push(' '); | ||
| } | ||
| // Add struct token | ||
| formatted_code.push_str(item_struct.struct_token.span().as_str()); | ||
| formatted_code.push(' '); | ||
|
|
||
| // Add struct name | ||
| formatted_code.push_str(item_struct.name.as_str()); | ||
|
|
||
| // Check if there is generic provided | ||
| if let Some(generics) = &item_struct.generics { | ||
| // Push angle brace | ||
| ItemStruct::open_angle_bracket(formatted_code, formatter); | ||
| // Get generics fields | ||
| let generics = generics.parameters.inner.value_separator_pairs.clone(); | ||
| for (index, generic) in generics.iter().enumerate() { | ||
| // Push ident | ||
| formatted_code.push_str(generic.0.as_str()); | ||
| if index != generics.len() - 1 { | ||
| // Push `, ` if this is not the last generic | ||
| formatted_code.push_str(", "); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Handle openning brace | ||
| if multiline { | ||
| ItemStruct::open_curly_brace(formatted_code, formatter); | ||
| formatted_code.push('\n'); | ||
| } else { | ||
| // Push a single whitespace before `{` | ||
| formatted_code.push(' '); | ||
| // Push open brace | ||
| formatted_code.push('{'); | ||
| // Push a single whitespace after `{` | ||
| formatted_code.push(' '); | ||
| } | ||
|
|
||
| let items = item_struct | ||
| .fields | ||
| .clone() | ||
| .into_inner() | ||
| .value_separator_pairs; | ||
|
|
||
| // In first iteration we are going to be collecting the lengths of the enum variants. | ||
| let variant_length: Vec<usize> = items | ||
| .iter() | ||
| .map(|variant| variant.0.name.as_str().len()) | ||
| .collect(); | ||
|
|
||
| // Find the maximum length in the variant_length vector that is still smaller than enum_variant_align_threshold. | ||
| let mut max_valid_variant_length = 0; | ||
|
|
||
| variant_length.iter().for_each(|length| { | ||
| if *length > max_valid_variant_length && *length < struct_variant_align_threshold { | ||
| max_valid_variant_length = *length; | ||
| } | ||
| }); | ||
| for (item_index, item) in items.iter().enumerate() { | ||
| if multiline { | ||
| formatted_code.push_str(&formatter.shape.indent.to_string(formatter)); | ||
| } | ||
| let type_field = &item.0; | ||
| // Add name | ||
| formatted_code.push_str(type_field.name.as_str()); | ||
| let current_variant_length = variant_length[item_index]; | ||
| if current_variant_length < max_valid_variant_length { | ||
| // We need to add alignment between : and ty | ||
| // max_valid_variant_length: the length of the variant that we are taking as a reference to allign | ||
| // current_variant_length: the length of the current variant that we are trying to format | ||
| let required_alignment = max_valid_variant_length - current_variant_length; | ||
| // TODO: Improve handling this | ||
| formatted_code.push_str(&(0..required_alignment).map(|_| ' ').collect::<String>()); | ||
| } | ||
| // Add `:` | ||
| formatted_code.push_str(type_field.colon_token.ident().as_str()); | ||
| formatted_code.push(' '); | ||
| // TODO: We are currently converting ty to string directly but we will probably need to format ty before adding. | ||
| // Add ty | ||
| formatted_code.push_str(type_field.ty.span().as_str()); | ||
| // Add `, ` if this isn't the last field. | ||
| if !multiline && item_index != items.len() - 1 { | ||
| formatted_code.push_str(", "); | ||
| } else if multiline { | ||
| formatted_code.push_str(",\n"); | ||
| } | ||
| } | ||
| if !multiline { | ||
| // Push a ' ' | ||
| formatted_code.push(' '); | ||
| } | ||
| // Handle closing brace | ||
| ItemStruct::close_curly_brace(formatted_code, formatter); | ||
| } | ||
|
|
||
| impl ItemLen for ItemStruct { | ||
| fn get_formatted_len(&self) -> usize { | ||
| // TODO while determininig the length we may want to format to some degree and take length. | ||
| let str_item = &self.span().as_str().len(); | ||
| *str_item as usize | ||
| } | ||
| } | ||
|
|
||
| impl CurlyBrace for ItemStruct { | ||
| fn open_curly_brace(line: &mut String, formatter: &mut Formatter) { | ||
| let brace_style = formatter.config.items.item_brace_style; | ||
kayagokalp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| let extra_width = formatter.config.whitespace.tab_spaces; | ||
| let mut shape = formatter.shape; | ||
| match brace_style { | ||
| ItemBraceStyle::AlwaysNextLine => { | ||
| // Add openning brace to the next line. | ||
| line.push_str("\n{"); | ||
| shape = shape.block_indent(extra_width); | ||
| } | ||
| _ => { | ||
| // Add opening brace to the same line | ||
| line.push_str(" {"); | ||
| shape = shape.block_indent(extra_width); | ||
| } | ||
| } | ||
|
|
||
| formatter.shape = shape; | ||
| } | ||
|
|
||
| fn close_curly_brace(line: &mut String, formatter: &mut Formatter) { | ||
| line.push('}'); | ||
| // If shape is becoming left-most alligned or - indent just have the defualt shape | ||
| formatter.shape = formatter | ||
| .shape | ||
| .shrink_left(formatter.config.whitespace.tab_spaces) | ||
| .unwrap_or_default(); | ||
| } | ||
| } | ||
|
|
||
| impl AngleBracket for ItemStruct { | ||
| fn open_angle_bracket(line: &mut String, _formatter: &mut Formatter) { | ||
| line.push('<'); | ||
| } | ||
|
|
||
| fn close_angle_bracket(line: &mut String, _formatter: &mut Formatter) { | ||
| line.push('>'); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| pub trait ItemLen { | ||
| fn get_formatted_len(&self) -> usize; | ||
| } |


Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't worry about leaving these comments.
If it comes to it, we can potentially refactor the
Formattrait to look a little closer to something likeDisplaywhere we can use thewrite!andwriteln!macros to write directly into an existing buffer, but for now the existing approach is pretty simple and clean, and I doubt the string allocs in fmt-ing will ever be a hinderance/bottleneck for anyone...10 yrs later: people have 1,000,000 LOC Sway codebases and shake their fists at past @mitchmindtree.