Browse Source

Fix: #2604, Fix: #2240, Fix: #2341, Fix #1355 - Better error handling and and spaces handling in autofmt (#2736)

* add new autofmt sample
* Feat: implement rustfmt::skip support for rsx
* generally improve error handling with better expect messages
* wip: nested rsx formatting and expression formatting
* nested rsx formatting works
* collapse autofmt crate
* cast indent through macros
* use proper whitespace
* no more eating comments!
* Use proper error handling
Jonathan Kelley 11 tháng trước cách đây
mục cha
commit
828cc502f1
47 tập tin đã thay đổi với 1551 bổ sung985 xóa
  1. 1 0
      Cargo.lock
  2. 1 1
      Cargo.toml
  3. 0 3
      packages/autofmt/src/buffer.rs
  4. 67 16
      packages/autofmt/src/collect_macros.rs
  5. 35 32
      packages/autofmt/src/lib.rs
  6. 314 147
      packages/autofmt/src/prettier_please.rs
  7. 0 444
      packages/autofmt/src/rsx_block.rs
  8. 695 199
      packages/autofmt/src/writer.rs
  9. 21 0
      packages/autofmt/tests/error_handling.rs
  10. 8 0
      packages/autofmt/tests/partials/no_parse.rsx
  11. 10 0
      packages/autofmt/tests/partials/okay.rsx
  12. 10 0
      packages/autofmt/tests/partials/wrong.rsx
  13. 14 9
      packages/autofmt/tests/samples.rs
  14. 2 2
      packages/autofmt/tests/samples/attributes.rsx
  15. 8 0
      packages/autofmt/tests/samples/basic_expr.rsx
  16. 26 0
      packages/autofmt/tests/samples/comments.rsx
  17. 1 1
      packages/autofmt/tests/samples/commentshard.rsx
  18. 7 4
      packages/autofmt/tests/samples/complex.rsx
  19. 2 2
      packages/autofmt/tests/samples/docsite.rsx
  20. 11 0
      packages/autofmt/tests/samples/ifchain_forloop.rsx
  21. 3 1
      packages/autofmt/tests/samples/long_exprs.rsx
  22. 1 1
      packages/autofmt/tests/samples/manual_props.rsx
  23. 39 15
      packages/autofmt/tests/samples/many_exprs.rsx
  24. 1 1
      packages/autofmt/tests/samples/misplaced.rsx
  25. 13 11
      packages/autofmt/tests/samples/nested.rsx
  26. 1 0
      packages/autofmt/tests/samples/oneline.rsx
  27. 1 1
      packages/autofmt/tests/samples/raw_strings.rsx
  28. 1 1
      packages/autofmt/tests/samples/simple.rsx
  29. 36 0
      packages/autofmt/tests/samples/skip.rsx
  30. 14 0
      packages/autofmt/tests/samples/spaces.rsx
  31. 9 7
      packages/autofmt/tests/samples/staged.rsx
  32. 9 1
      packages/autofmt/tests/wrong.rs
  33. 38 14
      packages/autofmt/tests/wrong/multiexpr-many.rsx
  34. 1 1
      packages/autofmt/tests/wrong/oneline-expand.rsx
  35. 18 8
      packages/autofmt/tests/wrong/simple-combo-expr.rsx
  36. 36 0
      packages/autofmt/tests/wrong/skipfail.rsx
  37. 32 0
      packages/autofmt/tests/wrong/skipfail.wrong.rsx
  38. 11 28
      packages/cli/src/cli/autoformat.rs
  39. 1 0
      packages/extension/Cargo.toml
  40. 19 11
      packages/extension/src/lib.rs
  41. 9 9
      packages/extension/src/main.ts
  42. 1 1
      packages/rsx-rosetta/src/lib.rs
  43. 3 3
      packages/rsx/src/component.rs
  44. 4 2
      packages/rsx/src/forloop.rs
  45. 8 2
      packages/rsx/src/ifchain.rs
  46. 1 7
      packages/rsx/src/ifmt.rs
  47. 8 0
      packages/rsx/src/literal.rs

+ 1 - 0
Cargo.lock

@@ -2616,6 +2616,7 @@ dependencies = [
  "dioxus-autofmt",
  "html_parser",
  "rsx-rosetta",
+ "syn 2.0.72",
  "wasm-bindgen",
 ]
 

+ 1 - 1
Cargo.toml

@@ -104,7 +104,7 @@ wasm-bindgen = "0.2.92"
 wasm-bindgen-futures = "0.4.42"
 html_parser = "0.7.0"
 thiserror = "1.0.40"
-prettyplease = { version = "0.2.16", features = ["verbatim"] }
+prettyplease = { version = "0.2.20", features = ["verbatim"] }
 manganis-cli-support = { git = "hhttps://github.com/DioxusLabs/manganis", features = ["html"] }
 manganis = { git = "https://github.com/DioxusLabs/manganis" }
 const_format = "0.2.32"

+ 0 - 3
packages/autofmt/src/buffer.rs

@@ -1,8 +1,5 @@
 //! The output buffer that supports some helpful methods
 //! These are separate from the input so we can lend references between the two
-//!
-//!
-//!
 
 use std::fmt::{Result, Write};
 

+ 67 - 16
packages/autofmt/src/collect_macros.rs

@@ -3,37 +3,66 @@
 //! Returns all macros that match a pattern. You can use this information to autoformat them later
 
 use proc_macro2::LineColumn;
-use syn::{visit::Visit, File, Macro};
+use syn::{visit::Visit, File, Macro, Meta};
 
 type CollectedMacro<'a> = &'a Macro;
 
 pub fn collect_from_file(file: &File) -> Vec<CollectedMacro<'_>> {
     let mut macros = vec![];
-    MacroCollector::visit_file(
-        &mut MacroCollector {
-            macros: &mut macros,
-        },
-        file,
-    );
+    let mut collector = MacroCollector::new(&mut macros);
+    MacroCollector::visit_file(&mut collector, file);
     macros
 }
 
 struct MacroCollector<'a, 'b> {
     macros: &'a mut Vec<CollectedMacro<'b>>,
+    skip_count: usize,
+}
+
+impl<'a, 'b> MacroCollector<'a, 'b> {
+    fn new(macros: &'a mut Vec<CollectedMacro<'b>>) -> Self {
+        Self {
+            macros,
+            skip_count: 0,
+        }
+    }
 }
 
 impl<'a, 'b> Visit<'b> for MacroCollector<'a, 'b> {
     fn visit_macro(&mut self, i: &'b Macro) {
-        if let Some("rsx" | "render") = i
-            .path
-            .segments
-            .last()
-            .map(|i| i.ident.to_string())
-            .as_deref()
-        {
-            self.macros.push(i)
+        // Visit the regular stuff - this will also ensure paths/attributes are visited
+        syn::visit::visit_macro(self, i);
+
+        let name = &i.path.segments.last().map(|i| i.ident.to_string());
+        if let Some("rsx" | "render") = name.as_deref() {
+            if self.skip_count == 0 {
+                self.macros.push(i)
+            }
         }
     }
+
+    // attributes can occur on stmts and items - we need to make sure the stack is reset when we exit
+    // this means we save the skipped length and set it back to its original length
+    fn visit_stmt(&mut self, i: &'b syn::Stmt) {
+        let skipped_len = self.skip_count;
+        syn::visit::visit_stmt(self, i);
+        self.skip_count = skipped_len;
+    }
+
+    fn visit_item(&mut self, i: &'b syn::Item) {
+        let skipped_len = self.skip_count;
+        syn::visit::visit_item(self, i);
+        self.skip_count = skipped_len;
+    }
+
+    fn visit_attribute(&mut self, i: &'b syn::Attribute) {
+        // we need to communicate that this stmt is skipped up the tree
+        if attr_is_rustfmt_skip(i) {
+            self.skip_count += 1;
+        }
+
+        syn::visit::visit_attribute(self, i);
+    }
 }
 
 pub fn byte_offset(input: &str, location: LineColumn) -> usize {
@@ -49,10 +78,32 @@ pub fn byte_offset(input: &str, location: LineColumn) -> usize {
             .sum::<usize>()
 }
 
+/// Check if an attribute is a rustfmt skip attribute
+fn attr_is_rustfmt_skip(i: &syn::Attribute) -> bool {
+    match &i.meta {
+        Meta::Path(path) => {
+            path.segments.len() == 2
+                && matches!(i.style, syn::AttrStyle::Outer)
+                && path.segments[0].ident == "rustfmt"
+                && path.segments[1].ident == "skip"
+        }
+        _ => false,
+    }
+}
+
 #[test]
 fn parses_file_and_collects_rsx_macros() {
     let contents = include_str!("../tests/samples/long.rsx");
-    let parsed = syn::parse_file(contents).unwrap();
+    let parsed = syn::parse_file(contents).expect("parse file okay");
     let macros = collect_from_file(&parsed);
     assert_eq!(macros.len(), 3);
 }
+
+/// Ensure that we only collect non-skipped macros
+#[test]
+fn dont_collect_skipped_macros() {
+    let contents = include_str!("../tests/samples/skip.rsx");
+    let parsed = syn::parse_file(contents).expect("parse file okay");
+    let macros = collect_from_file(&parsed);
+    assert_eq!(macros.len(), 2);
+}

+ 35 - 32
packages/autofmt/src/lib.rs

@@ -4,14 +4,13 @@
 
 use crate::writer::*;
 use dioxus_rsx::{BodyNode, CallBody};
-use proc_macro2::LineColumn;
-use syn::{parse::Parser, ExprMacro};
+use proc_macro2::{LineColumn, Span};
+use syn::parse::Parser;
 
 mod buffer;
 mod collect_macros;
 mod indent;
 mod prettier_please;
-mod rsx_block;
 mod writer;
 
 pub use indent::{IndentOptions, IndentType};
@@ -37,6 +36,16 @@ pub struct FormattedBlock {
     pub end: usize,
 }
 
+/// Format a file into a list of `FormattedBlock`s to be applied by an IDE for autoformatting.
+///
+/// It accepts
+#[deprecated(note = "Use try_fmt_file instead - this function panics on error.")]
+pub fn fmt_file(contents: &str, indent: IndentOptions) -> Vec<FormattedBlock> {
+    let parsed =
+        syn::parse_file(contents).expect("fmt_file should only be called on valid syn::File files");
+    try_fmt_file(contents, &parsed, indent).expect("Failed to format file")
+}
+
 /// Format a file into a list of `FormattedBlock`s to be applied by an IDE for autoformatting.
 ///
 /// This function expects a complete file, not just a block of code. To format individual rsx! blocks, use fmt_block instead.
@@ -45,19 +54,27 @@ pub struct FormattedBlock {
 /// back to the file precisely.
 ///
 /// Nested blocks of RSX will be handled automatically
-pub fn fmt_file(contents: &str, indent: IndentOptions) -> Vec<FormattedBlock> {
+///
+/// This returns an error if the rsx itself is invalid.
+///
+/// Will early return if any of the expressions are not complete. Even though we *could* return the
+/// expressions, eventually we'll want to pass off expression formatting to rustfmt which will reject
+/// those.
+pub fn try_fmt_file(
+    contents: &str,
+    parsed: &syn::File,
+    indent: IndentOptions,
+) -> syn::Result<Vec<FormattedBlock>> {
     let mut formatted_blocks = Vec::new();
 
-    let parsed = syn::parse_file(contents).unwrap();
-    let macros = collect_macros::collect_from_file(&parsed);
+    let macros = collect_macros::collect_from_file(parsed);
 
     // No macros, no work to do
     if macros.is_empty() {
-        return formatted_blocks;
+        return Ok(formatted_blocks);
     }
 
-    let mut writer = Writer::new(contents);
-    writer.out.indent = indent;
+    let mut writer = Writer::new(contents, indent);
 
     // Don't parse nested macros
     let mut end_span = LineColumn { column: 0, line: 0 };
@@ -69,14 +86,7 @@ pub fn fmt_file(contents: &str, indent: IndentOptions) -> Vec<FormattedBlock> {
             continue;
         }
 
-        let body = match item.parse_body_with(CallBody::parse_strict) {
-            Ok(v) => v,
-            //there is aparsing error, we give up and don't format the rsx
-            Err(e) => {
-                eprintln!("Error while parsing rsx {:?} ", e);
-                return formatted_blocks;
-            }
-        };
+        let body = item.parse_body_with(CallBody::parse_strict)?;
 
         let rsx_start = macro_path.span().start();
 
@@ -86,15 +96,16 @@ pub fn fmt_file(contents: &str, indent: IndentOptions) -> Vec<FormattedBlock> {
             .count_indents(writer.src[rsx_start.line - 1]);
 
         // TESTME
-        // If we fail to parse this macro then we have no choice to give up and return what we've got
+        // Writing *should* not fail but it's possible that it does
         if writer.write_rsx_call(&body.body).is_err() {
-            return formatted_blocks;
+            let span = writer.invalid_exprs.pop().unwrap_or_else(Span::call_site);
+            return Err(syn::Error::new(span, "Failed emit valid rsx - likely due to partially complete expressions in the rsx! macro"));
         }
 
         // writing idents leaves the final line ended at the end of the last ident
         if writer.out.buf.contains('\n') {
-            writer.out.new_line().unwrap();
-            writer.out.tab().unwrap();
+            _ = writer.out.new_line();
+            _ = writer.out.tab();
         }
 
         let span = item.delimiter.span().join();
@@ -124,7 +135,7 @@ pub fn fmt_file(contents: &str, indent: IndentOptions) -> Vec<FormattedBlock> {
         });
     }
 
-    formatted_blocks
+    Ok(formatted_blocks)
 }
 
 /// Write a Callbody (the rsx block) to a string
@@ -132,14 +143,7 @@ pub fn fmt_file(contents: &str, indent: IndentOptions) -> Vec<FormattedBlock> {
 /// If the tokens can't be formatted, this returns None. This is usually due to an incomplete expression
 /// that passed partial expansion but failed to parse.
 pub fn write_block_out(body: &CallBody) -> Option<String> {
-    let mut buf = Writer::new("");
-    buf.write_rsx_call(&body.body).ok()?;
-    buf.consume()
-}
-
-pub fn fmt_block_from_expr(raw: &str, expr: ExprMacro) -> Option<String> {
-    let body = CallBody::parse_strict.parse2(expr.mac.tokens).unwrap();
-    let mut buf = Writer::new(raw);
+    let mut buf = Writer::new("", IndentOptions::default());
     buf.write_rsx_call(&body.body).ok()?;
     buf.consume()
 }
@@ -147,8 +151,7 @@ pub fn fmt_block_from_expr(raw: &str, expr: ExprMacro) -> Option<String> {
 pub fn fmt_block(block: &str, indent_level: usize, indent: IndentOptions) -> Option<String> {
     let body = CallBody::parse_strict.parse_str(block).unwrap();
 
-    let mut buf = Writer::new(block);
-    buf.out.indent = indent;
+    let mut buf = Writer::new(block, indent);
     buf.out.indent_level = indent_level;
     buf.write_rsx_call(&body.body).ok()?;
 

+ 314 - 147
packages/autofmt/src/prettier_please.rs

@@ -1,142 +1,141 @@
-use prettyplease::unparse;
-use syn::{visit_mut::VisitMut, Expr, File, Item};
+use dioxus_rsx::CallBody;
+use syn::{parse::Parser, visit_mut::VisitMut, Expr, File, Item};
 
-use crate::Writer;
+use crate::{IndentOptions, Writer};
 
 impl Writer<'_> {
     pub fn unparse_expr(&mut self, expr: &Expr) -> String {
-        struct ReplaceMacros<'a, 'b> {
-            writer: &'a mut Writer<'b>,
-            formatted_stack: Vec<String>,
-        }
+        unparse_expr(expr, self.raw_src, &self.out.indent)
+    }
+}
 
-        impl VisitMut for ReplaceMacros<'_, '_> {
-            fn visit_stmt_mut(&mut self, _expr: &mut syn::Stmt) {
-                if let syn::Stmt::Macro(i) = _expr {
-                    // replace the macro with a block that roughly matches the macro
-                    if let Some("rsx" | "render") = i
-                        .mac
-                        .path
-                        .segments
-                        .last()
-                        .map(|i| i.ident.to_string())
-                        .as_deref()
-                    {
-                        // format the macro in place
-                        // we'll use information about the macro to replace it with another formatted block
-                        // once we've written out the unparsed expr from prettyplease, we can replace
-                        // this dummy block with the actual formatted block
-                        let formatted = crate::fmt_block_from_expr(
-                            self.writer.raw_src,
-                            syn::ExprMacro {
-                                attrs: i.attrs.clone(),
-                                mac: i.mac.clone(),
-                            },
-                        )
-                        .unwrap();
-
-                        *_expr = syn::Stmt::Expr(
-                            syn::parse_quote!(dioxus_autofmt_block__________),
-                            i.semi_token,
-                        );
-
-                        // Save this formatted block for later, when we apply it to the original expr
-                        self.formatted_stack.push(formatted);
-                    }
-                }
+const MARKER: &str = "dioxus_autofmt_block__________";
+const MARKER_REPLACE: &str = "dioxus_autofmt_block__________! {}";
 
-                syn::visit_mut::visit_stmt_mut(self, _expr);
-            }
+pub fn unparse_expr(expr: &Expr, src: &str, cfg: &IndentOptions) -> String {
+    struct ReplaceMacros<'a> {
+        src: &'a str,
+        formatted_stack: Vec<String>,
+        cfg: &'a IndentOptions,
+    }
 
-            fn visit_expr_mut(&mut self, _expr: &mut syn::Expr) {
-                if let syn::Expr::Macro(i) = _expr {
-                    // replace the macro with a block that roughly matches the macro
-                    if let Some("rsx" | "render") = i
-                        .mac
-                        .path
-                        .segments
-                        .last()
-                        .map(|i| i.ident.to_string())
-                        .as_deref()
-                    {
-                        // format the macro in place
-                        // we'll use information about the macro to replace it with another formatted block
-                        // once we've written out the unparsed expr from prettyplease, we can replace
-                        // this dummy block with the actual formatted block
-                        let formatted = crate::fmt_block_from_expr(
-                            self.writer.raw_src,
-                            syn::ExprMacro {
-                                attrs: i.attrs.clone(),
-                                mac: i.mac.clone(),
-                            },
-                        )
-                        .unwrap();
-
-                        *_expr = syn::parse_quote!(dioxus_autofmt_block__________);
-
-                        // Save this formatted block for later, when we apply it to the original expr
-                        self.formatted_stack.push(formatted);
-                    }
+    impl VisitMut for ReplaceMacros<'_> {
+        fn visit_macro_mut(&mut self, i: &mut syn::Macro) {
+            // replace the macro with a block that roughly matches the macro
+            if let Some("rsx" | "render") = i
+                .path
+                .segments
+                .last()
+                .map(|i| i.ident.to_string())
+                .as_deref()
+            {
+                // format the macro in place
+                // we'll use information about the macro to replace it with another formatted block
+                // once we've written out the unparsed expr from prettyplease, we can replace
+                // this dummy block with the actual formatted block
+                let body = CallBody::parse_strict.parse2(i.tokens.clone()).unwrap();
+                let multiline = !Writer::is_short_rsx_call(&body.body.roots);
+                let mut formatted = {
+                    let mut writer = Writer::new(self.src, self.cfg.clone());
+                    _ = writer.write_body_nodes(&body.body.roots).ok();
+                    writer.consume()
+                }
+                .unwrap();
+
+                // always push out the rsx to require a new line
+                i.path = syn::parse_str(MARKER).unwrap();
+                i.tokens = Default::default();
+
+                // Push out the indent level of the formatted block if it's multiline
+                if multiline || formatted.contains('\n') {
+                    formatted = formatted
+                        .lines()
+                        .map(|line| format!("{}{line}", self.cfg.indent_str()))
+                        .collect::<Vec<_>>()
+                        .join("\n");
                 }
 
-                syn::visit_mut::visit_expr_mut(self, _expr);
+                // Save this formatted block for later, when we apply it to the original expr
+                self.formatted_stack.push(formatted)
             }
+
+            syn::visit_mut::visit_macro_mut(self, i);
         }
+    }
 
-        // Visit the expr and replace the macros with formatted blocks
-        let mut replacer = ReplaceMacros {
-            writer: self,
-            formatted_stack: vec![],
-        };
-
-        // builds the expression stack
-        let mut modified_expr = expr.clone();
-        replacer.visit_expr_mut(&mut modified_expr);
-
-        // now unparsed with the modified expression
-        let mut unparsed = unparse_expr(&modified_expr);
-
-        // walk each line looking for the dioxus_autofmt_block__________ token
-        // if we find it, replace it with the formatted block
-        // if there's indentation we want to presreve it
-
-        // now we can replace the macros with the formatted blocks
-        for formatted in replacer.formatted_stack.drain(..) {
-            let fmted = if formatted.contains('\n') {
-                format!("rsx! {{{formatted}\n}}")
-            } else {
-                format!("rsx! {{{formatted}}}")
-            };
-            let mut out_fmt = String::new();
-            let mut whitespace = 0;
+    // Visit the expr and replace the macros with formatted blocks
+    let mut replacer = ReplaceMacros {
+        src,
+        cfg,
+        formatted_stack: vec![],
+    };
+
+    // builds the expression stack
+    let mut modified_expr = expr.clone();
+    replacer.visit_expr_mut(&mut modified_expr);
+
+    // now unparsed with the modified expression
+    let mut unparsed = unparse_inner(&modified_expr);
+
+    // now we can replace the macros with the formatted blocks
+    for fmted in replacer.formatted_stack.drain(..) {
+        let is_multiline = fmted.contains('{');
+
+        let mut out_fmt = String::from("rsx! {");
+        if is_multiline {
+            out_fmt.push('\n');
+        } else {
+            out_fmt.push(' ');
+        }
 
-            for line in unparsed.lines() {
-                if line.contains("dioxus_autofmt_block__________") {
-                    whitespace = line.chars().take_while(|c| c.is_whitespace()).count();
-                    break;
-                }
+        let mut whitespace = 0;
+
+        for line in unparsed.lines() {
+            if line.contains(MARKER) {
+                whitespace = line.matches(cfg.indent_str()).count();
+                break;
             }
+        }
 
-            for (idx, fmt_line) in fmted.lines().enumerate() {
-                // Push the indentation
-                if idx > 0 {
-                    out_fmt.push_str(&" ".repeat(whitespace));
-                }
+        let mut lines = fmted.lines().enumerate().peekable();
+
+        while let Some((_idx, fmt_line)) = lines.next() {
+            // Push the indentation
+            if is_multiline {
+                out_fmt.push_str(&cfg.indent_str().repeat(whitespace));
+            }
 
-                out_fmt.push_str(fmt_line);
+            // Calculate delta between indentations - the block indentation is too much
+            out_fmt.push_str(fmt_line);
 
-                // Push a newline
+            // Push a newline if there's another line
+            if lines.peek().is_some() {
                 out_fmt.push('\n');
             }
+        }
 
-            // Remove the last newline
-            out_fmt.pop();
-
-            // Replace the dioxus_autofmt_block__________ token with the formatted block
-            unparsed = unparsed.replacen("dioxus_autofmt_block__________", &out_fmt, 1);
-            continue;
+        if is_multiline {
+            out_fmt.push('\n');
+            out_fmt.push_str(&cfg.indent_str().repeat(whitespace));
+        } else {
+            out_fmt.push(' ');
         }
 
+        // Replace the dioxus_autofmt_block__________ token with the formatted block
+        out_fmt.push('}');
+
+        unparsed = unparsed.replacen(MARKER_REPLACE, &out_fmt, 1);
+        continue;
+    }
+
+    // stylistic choice to trim whitespace around the expr
+    if unparsed.starts_with("{ ") && unparsed.ends_with(" }") {
+        let mut out_fmt = String::new();
+        out_fmt.push('{');
+        out_fmt.push_str(&unparsed[2..unparsed.len() - 2]);
+        out_fmt.push('}');
+        out_fmt
+    } else {
         unparsed
     }
 }
@@ -145,9 +144,9 @@ impl Writer<'_> {
 ///
 /// This creates a new temporary file, parses the expression into it, and then formats the file.
 /// This is a bit of a hack, but dtonlay doesn't want to support this very simple usecase, forcing us to clone the expr
-pub fn unparse_expr(expr: &Expr) -> String {
+pub fn unparse_inner(expr: &Expr) -> String {
     let file = wrapped(expr);
-    let wrapped = unparse(&file);
+    let wrapped = prettyplease::unparse(&file);
     unwrapped(wrapped)
 }
 
@@ -164,7 +163,9 @@ fn unwrapped(raw: String) -> String {
         .join("\n");
 
     // remove the semicolon
-    o.pop();
+    if o.ends_with(';') {
+        o.pop();
+    }
 
     o
 }
@@ -184,36 +185,202 @@ fn wrapped(expr: &Expr) -> File {
     }
 }
 
-#[test]
-fn unparses_raw() {
-    let expr = syn::parse_str("1 + 1").unwrap();
-    let unparsed = unparse(&wrapped(&expr));
-    assert_eq!(unparsed, "fn main() {\n    1 + 1;\n}\n");
-}
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use proc_macro2::TokenStream;
 
-#[test]
-fn unparses_completely() {
-    let expr = syn::parse_str("1 + 1").unwrap();
-    let unparsed = unparse_expr(&expr);
-    assert_eq!(unparsed, "1 + 1");
-}
+    fn fmt_block_from_expr(raw: &str, tokens: TokenStream, cfg: IndentOptions) -> Option<String> {
+        let body = CallBody::parse_strict.parse2(tokens).unwrap();
+        let mut writer = Writer::new(raw, cfg);
+        writer.write_body_nodes(&body.body.roots).ok()?;
+        writer.consume()
+    }
 
-#[test]
-fn unparses_let_guard() {
-    let expr = syn::parse_str("let Some(url) = &link.location").unwrap();
-    let unparsed = unparse_expr(&expr);
-    assert_eq!(unparsed, "let Some(url) = &link.location");
-}
+    #[test]
+    fn unparses_raw() {
+        let expr = syn::parse_str("1 + 1").expect("Failed to parse");
+        let unparsed = prettyplease::unparse(&wrapped(&expr));
+        assert_eq!(unparsed, "fn main() {\n    1 + 1;\n}\n");
+    }
+
+    #[test]
+    fn weird_ifcase() {
+        let contents = r##"
+        fn main() {
+            move |_| timer.with_mut(|t| if t.started_at.is_none() { Some(Instant::now()) } else { None })
+        }
+    "##;
 
-#[test]
-fn weird_ifcase() {
-    let contents = r##"
-    fn main() {
-        move |_| timer.with_mut(|t| if t.started_at.is_none() { Some(Instant::now()) } else { None })
+        let expr: File = syn::parse_file(contents).unwrap();
+        let out = prettyplease::unparse(&expr);
+        println!("{}", out);
     }
-"##;
 
-    let expr: File = syn::parse_file(contents).unwrap();
-    let out = unparse(&expr);
-    println!("{}", out);
+    #[test]
+    fn multiline_madness() {
+        let contents = r##"
+        {
+        {children.is_some().then(|| rsx! {
+            span {
+                class: "inline-block ml-auto hover:bg-gray-500",
+                onclick: move |evt| {
+                    evt.cancel_bubble();
+                },
+                icons::icon_5 {}
+                {rsx! {
+                    icons::icon_6 {}
+                }}
+            }
+        })}
+        {children.is_some().then(|| rsx! {
+            span {
+                class: "inline-block ml-auto hover:bg-gray-500",
+                onclick: move |evt| {
+                    evt.cancel_bubble();
+                },
+                icons::icon_10 {}
+            }
+        })}
+
+        }
+
+        "##;
+
+        let expr: Expr = syn::parse_str(contents).unwrap();
+        let out = unparse_expr(&expr, contents, &IndentOptions::default());
+        println!("{}", out);
+    }
+
+    #[test]
+    fn write_body_no_indent() {
+        let src = r##"
+            span {
+                class: "inline-block ml-auto hover:bg-gray-500",
+                onclick: move |evt| {
+                    evt.cancel_bubble();
+                },
+                icons::icon_10 {}
+                icons::icon_10 {}
+                icons::icon_10 {}
+                icons::icon_10 {}
+                div { "hi" }
+                div { div {} }
+                div { div {} div {} div {} }
+                {children}
+                {
+                    some_big_long()
+                        .some_big_long()
+                        .some_big_long()
+                        .some_big_long()
+                        .some_big_long()
+                        .some_big_long()
+                }
+                div { class: "px-4", {is_current.then(|| rsx! { {children} })} }
+                Thing {
+                    field: rsx! {
+                        div { "hi" }
+                        Component {
+                            onrender: rsx! {
+                                div { "hi" }
+                                Component {
+                                    onclick: move |_| {
+                                        another_macro! {
+                                            div { class: "max-w-lg lg:max-w-2xl mx-auto mb-16 text-center",
+                                                "gomg"
+                                                "hi!!"
+                                                "womh"
+                                            }
+                                        };
+                                        rsx! {
+                                            div { class: "max-w-lg lg:max-w-2xl mx-auto mb-16 text-center",
+                                                "gomg"
+                                                "hi!!"
+                                                "womh"
+                                            }
+                                        };
+                                        println!("hi")
+                                    },
+                                    onrender: move |_| {
+                                        let _ = 12;
+                                        let r = rsx! {
+                                            div { "hi" }
+                                        };
+                                        rsx! {
+                                            div { "hi" }
+                                        }
+                                    }
+                                }
+                                {
+                                    rsx! {
+                                        BarChart {
+                                            id: "bar-plot".to_string(),
+                                            x: value,
+                                            y: label
+                                        }
+                                    }
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        "##;
+
+        let tokens: TokenStream = syn::parse_str(src).unwrap();
+        let out = fmt_block_from_expr(src, tokens, IndentOptions::default()).unwrap();
+        println!("{}", out);
+    }
+
+    #[test]
+    fn write_component_body() {
+        let src = r##"
+    div { class: "px-4", {is_current.then(|| rsx! { {children} })} }
+    "##;
+
+        let tokens: TokenStream = syn::parse_str(src).unwrap();
+        let out = fmt_block_from_expr(src, tokens, IndentOptions::default()).unwrap();
+        println!("{}", out);
+    }
+
+    #[test]
+    fn weird_macro() {
+        let contents = r##"
+        fn main() {
+            move |_| {
+                drop_macro_semi! {
+                    "something_very_long_something_very_long_something_very_long_something_very_long"
+                };
+                let _ = drop_macro_semi! {
+                    "something_very_long_something_very_long_something_very_long_something_very_long"
+                };
+                drop_macro_semi! {
+                    "something_very_long_something_very_long_something_very_long_something_very_long"
+                };
+            };
+        }
+    "##;
+
+        let expr: File = syn::parse_file(contents).unwrap();
+        let out = prettyplease::unparse(&expr);
+        println!("{}", out);
+    }
+
+    #[test]
+    fn comments_on_nodes() {
+        let src = r##"// hiasdasds
+    div {
+        attr: "value", // comment
+        div {}
+        "hi" // hello!
+        "hi" // hello!
+        "hi" // hello!
+        // hi!
+    }
+    "##;
+
+        let tokens: TokenStream = syn::parse_str(src).unwrap();
+        let out = fmt_block_from_expr(src, tokens, IndentOptions::default()).unwrap();
+        println!("{}", out);
+    }
 }

+ 0 - 444
packages/autofmt/src/rsx_block.rs

@@ -1,444 +0,0 @@
-use crate::{prettier_please::unparse_expr, Writer};
-use dioxus_rsx::*;
-use proc_macro2::Span;
-use quote::ToTokens;
-use std::{
-    fmt::Result,
-    fmt::{self, Write},
-};
-use syn::{spanned::Spanned, token::Brace, Expr};
-
-#[derive(Debug)]
-enum ShortOptimization {
-    /// Special because we want to print the closing bracket immediately
-    ///
-    /// IE
-    /// `div {}` instead of `div { }`
-    Empty,
-
-    /// Special optimization to put everything on the same line and add some buffer spaces
-    ///
-    /// IE
-    ///
-    /// `div { "asdasd" }` instead of a multiline variant
-    Oneliner,
-
-    /// Optimization where children flow but props remain fixed on top
-    PropsOnTop,
-
-    /// The noisiest optimization where everything flows
-    NoOpt,
-}
-
-impl Writer<'_> {
-    /// Basically elements and components are the same thing
-    ///
-    /// This writes the contents out for both in one function, centralizing the annoying logic like
-    /// key handling, breaks, closures, etc
-    pub fn write_rsx_block(
-        &mut self,
-        attributes: &[Attribute],
-        spreads: &[Spread],
-        children: &[BodyNode],
-        brace: &Brace,
-    ) -> Result {
-        // decide if we have any special optimizations
-        // Default with none, opt the cases in one-by-one
-        let mut opt_level = ShortOptimization::NoOpt;
-
-        // check if we have a lot of attributes
-        let attr_len = self.is_short_attrs(attributes, spreads);
-        let is_short_attr_list = (attr_len + self.out.indent_level * 4) < 80;
-        let children_len = self.is_short_children(children);
-        let is_small_children = children_len.is_some();
-
-        // if we have one long attribute and a lot of children, place the attrs on top
-        if is_short_attr_list && !is_small_children {
-            opt_level = ShortOptimization::PropsOnTop;
-        }
-
-        // even if the attr is long, it should be put on one line
-        // However if we have childrne we need to just spread them out for readability
-        if !is_short_attr_list && attributes.len() <= 1 && spreads.is_empty() {
-            if children.is_empty() {
-                opt_level = ShortOptimization::Oneliner;
-            } else {
-                opt_level = ShortOptimization::PropsOnTop;
-            }
-        }
-
-        // if we have few children and few attributes, make it a one-liner
-        if is_short_attr_list && is_small_children {
-            if children_len.unwrap() + attr_len + self.out.indent_level * 4 < 100 {
-                opt_level = ShortOptimization::Oneliner;
-            } else {
-                opt_level = ShortOptimization::PropsOnTop;
-            }
-        }
-
-        // If there's nothing at all, empty optimization
-        if attributes.is_empty() && children.is_empty() && spreads.is_empty() {
-            opt_level = ShortOptimization::Empty;
-
-            // Write comments if they exist
-            self.write_todo_body(brace)?;
-        }
-
-        // multiline handlers bump everything down
-        if attr_len > 1000 || self.out.indent.split_line_attributes() {
-            opt_level = ShortOptimization::NoOpt;
-        }
-
-        let has_children = !children.is_empty();
-
-        match opt_level {
-            ShortOptimization::Empty => {}
-            ShortOptimization::Oneliner => {
-                write!(self.out, " ")?;
-
-                self.write_attributes(attributes, spreads, true, brace, has_children)?;
-
-                if !children.is_empty() && !attributes.is_empty() {
-                    write!(self.out, " ")?;
-                }
-
-                for child in children.iter() {
-                    self.write_ident(child)?;
-                }
-
-                write!(self.out, " ")?;
-            }
-
-            ShortOptimization::PropsOnTop => {
-                if !attributes.is_empty() {
-                    write!(self.out, " ")?;
-                }
-
-                self.write_attributes(attributes, spreads, true, brace, has_children)?;
-
-                if !children.is_empty() {
-                    self.write_body_indented(children)?;
-                }
-
-                self.out.tabbed_line()?;
-            }
-
-            ShortOptimization::NoOpt => {
-                self.write_attributes(attributes, spreads, false, brace, has_children)?;
-
-                if !children.is_empty() {
-                    self.write_body_indented(children)?;
-                }
-
-                self.out.tabbed_line()?;
-            }
-        }
-
-        Ok(())
-    }
-
-    fn write_attributes(
-        &mut self,
-        attributes: &[Attribute],
-        spreads: &[Spread],
-        props_same_line: bool,
-        brace: &Brace,
-        has_children: bool,
-    ) -> Result {
-        enum AttrType<'a> {
-            Attr(&'a Attribute),
-            Spread(&'a Spread),
-        }
-
-        let mut attr_iter = attributes
-            .iter()
-            .map(AttrType::Attr)
-            .chain(spreads.iter().map(AttrType::Spread))
-            .peekable();
-
-        while let Some(attr) = attr_iter.next() {
-            self.out.indent_level += 1;
-
-            if !props_same_line {
-                self.write_attr_comments(
-                    brace,
-                    match attr {
-                        AttrType::Attr(attr) => attr.span(),
-                        AttrType::Spread(attr) => attr.expr.span(),
-                    },
-                )?;
-            }
-
-            self.out.indent_level -= 1;
-
-            if !props_same_line {
-                self.out.indented_tabbed_line()?;
-            }
-
-            match attr {
-                AttrType::Attr(attr) => self.write_attribute(attr)?,
-                AttrType::Spread(attr) => self.write_spread_attribute(&attr.expr)?,
-            }
-
-            if attr_iter.peek().is_some() {
-                write!(self.out, ",")?;
-
-                if props_same_line {
-                    write!(self.out, " ")?;
-                }
-            }
-        }
-
-        let has_attributes = !attributes.is_empty() || !spreads.is_empty();
-
-        if has_attributes && has_children {
-            write!(self.out, ",")?;
-        }
-
-        Ok(())
-    }
-
-    fn write_attribute(&mut self, attr: &Attribute) -> Result {
-        self.write_attribute_name(&attr.name)?;
-
-        // if the attribute is a shorthand, we don't need to write the colon, just the name
-        if !attr.can_be_shorthand() {
-            write!(self.out, ": ")?;
-            self.write_attribute_value(&attr.value)?;
-        }
-
-        Ok(())
-    }
-
-    fn write_attribute_name(&mut self, attr: &AttributeName) -> Result {
-        match attr {
-            AttributeName::BuiltIn(name) => {
-                write!(self.out, "{}", name)?;
-            }
-            AttributeName::Custom(name) => {
-                write!(self.out, "{}", name.to_token_stream())?;
-            }
-            AttributeName::Spread(_) => unreachable!(),
-        }
-
-        Ok(())
-    }
-
-    fn write_attribute_value(&mut self, value: &AttributeValue) -> Result {
-        match value {
-            AttributeValue::IfExpr(if_chain) => {
-                self.write_attribute_if_chain(if_chain)?;
-            }
-            AttributeValue::AttrLiteral(value) => {
-                write!(self.out, "{value}")?;
-            }
-            AttributeValue::Shorthand(value) => {
-                write!(self.out, "{value}")?;
-            }
-            AttributeValue::EventTokens(closure) => {
-                self.write_partial_closure(closure)?;
-            }
-
-            AttributeValue::AttrExpr(value) => {
-                let Ok(expr) = value.as_expr() else {
-                    return Err(fmt::Error);
-                };
-
-                let pretty_expr = self.retrieve_formatted_expr(&expr).to_string();
-                self.write_mulitiline_tokens(pretty_expr)?;
-            }
-        }
-
-        Ok(())
-    }
-
-    fn write_attribute_if_chain(&mut self, if_chain: &IfAttributeValue) -> Result {
-        write!(self.out, "if {} {{ ", unparse_expr(&if_chain.condition))?;
-        self.write_attribute_value(&if_chain.then_value)?;
-        write!(self.out, " }}")?;
-        match if_chain.else_value.as_deref() {
-            Some(AttributeValue::IfExpr(else_if_chain)) => {
-                write!(self.out, "else ")?;
-                self.write_attribute_if_chain(else_if_chain)?;
-            }
-            Some(other) => {
-                write!(self.out, "else {{")?;
-                self.write_attribute_value(other)?;
-                write!(self.out, " }}")?;
-            }
-            None => {}
-        }
-
-        Ok(())
-    }
-
-    fn write_mulitiline_tokens(&mut self, out: String) -> Result {
-        let mut lines = out.split('\n').peekable();
-        let first = lines.next().unwrap();
-
-        // a one-liner for whatever reason
-        // Does not need a new line
-        if lines.peek().is_none() {
-            write!(self.out, "{first}")?;
-        } else {
-            writeln!(self.out, "{first}")?;
-
-            while let Some(line) = lines.next() {
-                self.out.indented_tab()?;
-                write!(self.out, "{line}")?;
-                if lines.peek().is_none() {
-                    write!(self.out, "")?;
-                } else {
-                    writeln!(self.out)?;
-                }
-            }
-        }
-
-        Ok(())
-    }
-
-    /// Write out the special PartialClosure type from the rsx crate
-    /// Basically just write token by token until we hit the block and then try and format *that*
-    /// We can't just ToTokens
-    fn write_partial_closure(&mut self, closure: &PartialClosure) -> Result {
-        // Write the pretty version of the closure
-        if let Ok(expr) = closure.as_expr() {
-            let pretty_expr = self.retrieve_formatted_expr(&expr).to_string();
-            self.write_mulitiline_tokens(pretty_expr)?;
-            return Ok(());
-        }
-
-        // If we can't parse the closure, writing it is also a failure
-        // rustfmt won't be able to parse it either so no point in trying
-        Err(fmt::Error)
-    }
-
-    fn write_spread_attribute(&mut self, attr: &Expr) -> Result {
-        let formatted = unparse_expr(attr);
-
-        let mut lines = formatted.lines();
-
-        let first_line = lines.next().unwrap();
-
-        write!(self.out, "..{first_line}")?;
-        for line in lines {
-            self.out.indented_tabbed_line()?;
-            write!(self.out, "{line}")?;
-        }
-
-        Ok(())
-    }
-
-    // make sure the comments are actually relevant to this element.
-    // test by making sure this element is the primary element on this line
-    pub fn current_span_is_primary(&self, location: Span) -> bool {
-        let start = location.start();
-        let line_start = start.line - 1;
-
-        let beginning = self
-            .src
-            .get(line_start)
-            .filter(|this_line| this_line.len() > start.column)
-            .map(|this_line| this_line[..start.column].trim())
-            .unwrap_or_default();
-
-        beginning.is_empty()
-    }
-
-    // check if the children are short enough to be on the same line
-    // We don't have the notion of current line depth - each line tries to be < 80 total
-    // returns the total line length if it's short
-    // returns none if the length exceeds the limit
-    // I think this eventually becomes quadratic :(
-    pub fn is_short_children(&mut self, children: &[BodyNode]) -> Option<usize> {
-        if children.is_empty() {
-            // todo: allow elements with comments but no children
-            // like div { /* comment */ }
-            // or
-            // div {
-            //  // some helpful
-            // }
-            return Some(0);
-        }
-
-        // Any comments push us over the limit automatically
-        if self.children_have_comments(children) {
-            return None;
-        }
-
-        match children {
-            [BodyNode::Text(ref text)] => Some(text.input.to_string_with_quotes().len()),
-
-            // TODO: let rawexprs to be inlined
-            [BodyNode::RawExpr(ref expr)] => Some(get_expr_length(expr.span())),
-
-            // TODO: let rawexprs to be inlined
-            [BodyNode::Component(ref comp)] if comp.fields.is_empty() => Some(
-                comp.name
-                    .segments
-                    .iter()
-                    .map(|s| s.ident.to_string().len() + 2)
-                    .sum::<usize>(),
-            ),
-
-            // Feedback on discord indicates folks don't like combining multiple children on the same line
-            // We used to do a lot of math to figure out if we should expand out the line, but folks just
-            // don't like it.
-            _ => None,
-        }
-    }
-
-    fn children_have_comments(&self, children: &[BodyNode]) -> bool {
-        for child in children {
-            if self.current_span_is_primary(child.span()) {
-                'line: for line in self.src[..child.span().start().line - 1].iter().rev() {
-                    match (line.trim().starts_with("//"), line.is_empty()) {
-                        (true, _) => return true,
-                        (_, true) => continue 'line,
-                        _ => break 'line,
-                    }
-                }
-            }
-        }
-
-        false
-    }
-
-    /// empty everything except for some comments
-    fn write_todo_body(&mut self, brace: &Brace) -> fmt::Result {
-        let span = brace.span.span();
-        let start = span.start();
-        let end = span.end();
-
-        if start.line == end.line {
-            return Ok(());
-        }
-
-        writeln!(self.out)?;
-
-        for idx in start.line..end.line {
-            let line = &self.src[idx];
-            if line.trim().starts_with("//") {
-                for _ in 0..self.out.indent_level + 1 {
-                    write!(self.out, "    ")?
-                }
-                writeln!(self.out, "{}", line.trim()).unwrap();
-            }
-        }
-
-        for _ in 0..self.out.indent_level {
-            write!(self.out, "    ")?
-        }
-
-        Ok(())
-    }
-}
-
-fn get_expr_length(span: Span) -> usize {
-    let (start, end) = (span.start(), span.end());
-    if start.line == end.line {
-        end.column - start.column
-    } else {
-        10000
-    }
-}

+ 695 - 199
packages/autofmt/src/writer.rs

@@ -1,9 +1,5 @@
-use crate::buffer::Buffer;
-use crate::collect_macros::byte_offset;
-use dioxus_rsx::{
-    Attribute as AttributeType, AttributeName, AttributeValue as ElementAttrValue, BodyNode,
-    Component, Element, ForLoop, IfChain, Spread, TemplateBody,
-};
+use crate::{buffer::Buffer, IndentOptions};
+use dioxus_rsx::*;
 use proc_macro2::{LineColumn, Span};
 use quote::ToTokens;
 use std::{
@@ -17,19 +13,21 @@ pub struct Writer<'a> {
     pub raw_src: &'a str,
     pub src: Vec<&'a str>,
     pub cached_formats: HashMap<LineColumn, String>,
-    pub comments: VecDeque<usize>,
     pub out: Buffer,
+    pub invalid_exprs: Vec<Span>,
 }
 
 impl<'a> Writer<'a> {
-    pub fn new(raw_src: &'a str) -> Self {
-        let src = raw_src.lines().collect();
+    pub fn new(raw_src: &'a str, indent: IndentOptions) -> Self {
         Self {
+            src: raw_src.lines().collect(),
             raw_src,
-            src,
+            out: Buffer {
+                indent,
+                ..Default::default()
+            },
             cached_formats: HashMap::new(),
-            comments: VecDeque::new(),
-            out: Buffer::default(),
+            invalid_exprs: Vec::new(),
         }
     }
 
@@ -38,14 +36,17 @@ impl<'a> Writer<'a> {
     }
 
     pub fn write_rsx_call(&mut self, body: &TemplateBody) -> Result {
-        match body.roots.len() {
-            0 => {}
-            1 if matches!(body.roots[0], BodyNode::Text(_)) => {
-                write!(self.out, " ")?;
-                self.write_ident(&body.roots[0])?;
-                write!(self.out, " ")?;
-            }
-            _ => self.write_body_indented(&body.roots)?,
+        if body.roots.is_empty() {
+            return Ok(());
+        }
+
+        if Self::is_short_rsx_call(&body.roots) {
+            write!(self.out, " ")?;
+            self.write_ident(&body.roots[0])?;
+            write!(self.out, " ")?;
+        } else {
+            self.out.new_line()?;
+            self.write_body_indented(&body.roots)?
         }
 
         Ok(())
@@ -56,14 +57,29 @@ impl<'a> Writer<'a> {
         match node {
             BodyNode::Element(el) => self.write_element(el),
             BodyNode::Component(component) => self.write_component(component),
-            BodyNode::Text(text) => self.out.write_text(&text.input),
-            BodyNode::RawExpr(exp) => self.write_raw_expr(exp.span()),
+            BodyNode::Text(text) => self.write_text_node(text),
+            BodyNode::RawExpr(expr) => self.write_expr_node(expr),
             BodyNode::ForLoop(forloop) => self.write_for_loop(forloop),
             BodyNode::IfChain(ifchain) => self.write_if_chain(ifchain),
+        }?;
+
+        let span = Self::final_span_of_node(node);
+
+        self.write_inline_comments(span.end(), 0)?;
+
+        Ok(())
+    }
+
+    /// Check if the rsx call is short enough to be inlined
+    pub(crate) fn is_short_rsx_call(roots: &[BodyNode]) -> bool {
+        match roots.len() {
+            0 => true,
+            1 if matches!(roots[0], BodyNode::Text(_)) => true,
+            _ => false,
         }
     }
 
-    pub fn write_element(&mut self, el: &Element) -> Result {
+    fn write_element(&mut self, el: &Element) -> Result {
         let Element {
             name,
             raw_attributes: attributes,
@@ -73,24 +89,13 @@ impl<'a> Writer<'a> {
             ..
         } = el;
 
-        /*
-            1. Write the tag
-            2. Write the key
-            3. Write the attributes
-            4. Write the children
-        */
-
-        write!(self.out, "{name} {{")?;
-
-        let brace = brace.unwrap_or_default();
-        self.write_rsx_block(attributes, spreads, children, &brace)?;
-
-        write!(self.out, "}}")?;
+        write!(self.out, "{name} ")?;
+        self.write_rsx_block(attributes, spreads, children, &brace.unwrap_or_default())?;
 
         Ok(())
     }
 
-    pub fn write_component(
+    fn write_component(
         &mut self,
         Component {
             name,
@@ -114,66 +119,445 @@ impl<'a> Writer<'a> {
             write!(self.out, "{written}")?;
         }
 
-        write!(self.out, " {{")?;
+        write!(self.out, " ")?;
+        self.write_rsx_block(fields, spreads, &children.roots, &brace.unwrap_or_default())?;
+
+        Ok(())
+    }
+
+    fn write_text_node(&mut self, text: &TextNode) -> Result {
+        self.out.write_text(&text.input)
+    }
 
-        self.write_rsx_block(fields, spreads, &children.roots, brace)?;
+    fn write_expr_node(&mut self, expr: &ExprNode) -> Result {
+        self.write_partial_expr(expr.expr.as_expr(), expr.span())
+    }
 
+    fn write_for_loop(&mut self, forloop: &ForLoop) -> std::fmt::Result {
+        write!(
+            self.out,
+            "for {} in ",
+            forloop.pat.clone().into_token_stream(),
+        )?;
+
+        self.write_inline_expr(&forloop.expr)?;
+
+        if forloop.body.is_empty() {
+            write!(self.out, "}}")?;
+            return Ok(());
+        }
+
+        self.out.new_line()?;
+        self.write_body_indented(&forloop.body.roots)?;
+
+        self.out.tabbed_line()?;
         write!(self.out, "}}")?;
 
         Ok(())
     }
 
-    pub fn write_raw_expr(&mut self, placement: Span) -> Result {
-        /*
-        We want to normalize the expr to the appropriate indent level.
-        */
+    fn write_if_chain(&mut self, ifchain: &IfChain) -> std::fmt::Result {
+        // Recurse in place by setting the next chain
+        let mut branch = Some(ifchain);
 
-        let start = placement.start();
-        let end = placement.end();
+        while let Some(chain) = branch {
+            let IfChain {
+                if_token,
+                cond,
+                then_branch,
+                else_if_branch,
+                else_branch,
+                ..
+            } = chain;
 
-        // if the expr is on one line, just write it directly
-        if start.line == end.line {
-            // split counting utf8 chars
-            let start = byte_offset(self.raw_src, start);
-            let end = byte_offset(self.raw_src, end);
-            let row = self.raw_src[start..end].trim();
-            write!(self.out, "{row}")?;
-            return Ok(());
+            write!(self.out, "{} ", if_token.to_token_stream(),)?;
+
+            self.write_inline_expr(cond)?;
+
+            self.out.new_line()?;
+            self.write_body_indented(&then_branch.roots)?;
+
+            if let Some(else_if_branch) = else_if_branch {
+                // write the closing bracket and else
+                self.out.tabbed_line()?;
+                write!(self.out, "}} else ")?;
+
+                branch = Some(else_if_branch);
+            } else if let Some(else_branch) = else_branch {
+                self.out.tabbed_line()?;
+                write!(self.out, "}} else {{")?;
+
+                self.out.new_line()?;
+                self.write_body_indented(&else_branch.roots)?;
+                branch = None;
+            } else {
+                branch = None;
+            }
         }
 
-        // If the expr is multiline, we want to collect all of its lines together and write them out properly
-        // This involves unshifting the first line if it's aligned
-        let first_line = &self.src[start.line - 1];
-        write!(self.out, "{}", &first_line[start.column..].trim_start())?;
+        self.out.tabbed_line()?;
+        write!(self.out, "}}")?;
 
-        let prev_block_indent_level = self.out.indent.count_indents(first_line);
+        Ok(())
+    }
 
-        for (id, line) in self.src[start.line..end.line].iter().enumerate() {
-            writeln!(self.out)?;
+    /// An expression within a for or if block that might need to be spread out across several lines
+    fn write_inline_expr(&mut self, expr: &Expr) -> std::fmt::Result {
+        let unparsed = self.unparse_expr(expr);
+        let mut lines = unparsed.lines();
+        let first_line = lines.next().ok_or(std::fmt::Error)?;
 
-            // check if this is the last line
-            let line = {
-                if id == (end.line - start.line) - 1 {
-                    &line[..end.column]
-                } else {
-                    line
+        write!(self.out, "{first_line}")?;
+
+        let mut was_multiline = false;
+
+        for line in lines {
+            was_multiline = true;
+            self.out.tabbed_line()?;
+            write!(self.out, "{line}")?;
+        }
+
+        if was_multiline {
+            self.out.tabbed_line()?;
+            write!(self.out, "{{")?;
+        } else {
+            write!(self.out, " {{")?;
+        }
+
+        Ok(())
+    }
+
+    // Push out the indent level and write each component, line by line
+    fn write_body_indented(&mut self, children: &[BodyNode]) -> Result {
+        self.out.indent_level += 1;
+        self.write_body_nodes(children)?;
+        self.out.indent_level -= 1;
+        Ok(())
+    }
+
+    pub fn write_body_nodes(&mut self, children: &[BodyNode]) -> Result {
+        let mut iter = children.iter().peekable();
+
+        while let Some(child) = iter.next() {
+            if self.current_span_is_primary(child.span().start()) {
+                self.write_comments(child.span().start())?;
+            };
+            self.out.tab()?;
+            self.write_ident(child)?;
+            if iter.peek().is_some() {
+                self.out.new_line()?;
+            }
+        }
+
+        Ok(())
+    }
+
+    /// Basically elements and components are the same thing
+    ///
+    /// This writes the contents out for both in one function, centralizing the annoying logic like
+    /// key handling, breaks, closures, etc
+    fn write_rsx_block(
+        &mut self,
+        attributes: &[Attribute],
+        spreads: &[Spread],
+        children: &[BodyNode],
+        brace: &Brace,
+    ) -> Result {
+        #[derive(Debug)]
+        enum ShortOptimization {
+            /// Special because we want to print the closing bracket immediately
+            ///
+            /// IE
+            /// `div {}` instead of `div { }`
+            Empty,
+
+            /// Special optimization to put everything on the same line and add some buffer spaces
+            ///
+            /// IE
+            ///
+            /// `div { "asdasd" }` instead of a multiline variant
+            Oneliner,
+
+            /// Optimization where children flow but props remain fixed on top
+            PropsOnTop,
+
+            /// The noisiest optimization where everything flows
+            NoOpt,
+        }
+
+        // Write the opening brace
+        write!(self.out, "{{")?;
+
+        // decide if we have any special optimizations
+        // Default with none, opt the cases in one-by-one
+        let mut opt_level = ShortOptimization::NoOpt;
+
+        // check if we have a lot of attributes
+        let attr_len = self.is_short_attrs(attributes, spreads);
+        let is_short_attr_list = (attr_len + self.out.indent_level * 4) < 80;
+        let children_len = self
+            .is_short_children(children)
+            .map_err(|_| std::fmt::Error)?;
+        let is_small_children = children_len.is_some();
+
+        // if we have one long attribute and a lot of children, place the attrs on top
+        if is_short_attr_list && !is_small_children {
+            opt_level = ShortOptimization::PropsOnTop;
+        }
+
+        // even if the attr is long, it should be put on one line
+        // However if we have childrne we need to just spread them out for readability
+        if !is_short_attr_list && attributes.len() <= 1 && spreads.is_empty() {
+            if children.is_empty() {
+                opt_level = ShortOptimization::Oneliner;
+            } else {
+                opt_level = ShortOptimization::PropsOnTop;
+            }
+        }
+
+        // if we have few children and few attributes, make it a one-liner
+        if is_short_attr_list && is_small_children {
+            if children_len.unwrap() + attr_len + self.out.indent_level * 4 < 100 {
+                opt_level = ShortOptimization::Oneliner;
+            } else {
+                opt_level = ShortOptimization::PropsOnTop;
+            }
+        }
+
+        // If there's nothing at all, empty optimization
+        if attributes.is_empty() && children.is_empty() && spreads.is_empty() {
+            opt_level = ShortOptimization::Empty;
+
+            // Write comments if they exist
+            self.write_inline_comments(brace.span.span().start(), 1)?;
+            self.write_todo_body(brace)?;
+        }
+
+        // multiline handlers bump everything down
+        if attr_len > 1000 || self.out.indent.split_line_attributes() {
+            opt_level = ShortOptimization::NoOpt;
+        }
+
+        let has_children = !children.is_empty();
+
+        match opt_level {
+            ShortOptimization::Empty => {}
+            ShortOptimization::Oneliner => {
+                write!(self.out, " ")?;
+
+                self.write_attributes(attributes, spreads, true, brace, has_children)?;
+
+                if !children.is_empty() && !attributes.is_empty() {
+                    write!(self.out, " ")?;
+                }
+
+                let mut children_iter = children.iter().peekable();
+                while let Some(child) = children_iter.next() {
+                    self.write_ident(child)?;
+                    if children_iter.peek().is_some() {
+                        write!(self.out, " ")?;
+                    }
+                }
+
+                write!(self.out, " ")?;
+            }
+
+            ShortOptimization::PropsOnTop => {
+                if !attributes.is_empty() {
+                    write!(self.out, " ")?;
+                }
+
+                self.write_attributes(attributes, spreads, true, brace, has_children)?;
+
+                if !children.is_empty() {
+                    self.out.new_line()?;
+                    self.write_body_indented(children)?;
+                }
+
+                self.out.tabbed_line()?;
+            }
+
+            ShortOptimization::NoOpt => {
+                self.write_inline_comments(brace.span.span().start(), 1)?;
+                self.out.new_line()?;
+                self.write_attributes(attributes, spreads, false, brace, has_children)?;
+
+                if !children.is_empty() {
+                    self.out.new_line()?;
+                    self.write_body_indented(children)?;
                 }
+
+                self.out.tabbed_line()?;
+            }
+        }
+
+        // Write trailing comments
+        if matches!(
+            opt_level,
+            ShortOptimization::NoOpt | ShortOptimization::PropsOnTop
+        ) && self.leading_row_is_empty(brace.span.span().end())
+        {
+            let comments = self.accumulate_comments(brace.span.span().end());
+            if !comments.is_empty() {
+                self.apply_comments(comments)?;
+                self.out.tab()?;
+            }
+        }
+
+        write!(self.out, "}}")?;
+
+        Ok(())
+    }
+
+    fn write_attributes(
+        &mut self,
+        attributes: &[Attribute],
+        spreads: &[Spread],
+        props_same_line: bool,
+        brace: &Brace,
+        has_children: bool,
+    ) -> Result {
+        enum AttrType<'a> {
+            Attr(&'a Attribute),
+            Spread(&'a Spread),
+        }
+
+        let mut attr_iter = attributes
+            .iter()
+            .map(AttrType::Attr)
+            .chain(spreads.iter().map(AttrType::Spread))
+            .peekable();
+
+        let has_attributes = !attributes.is_empty() || !spreads.is_empty();
+
+        while let Some(attr) = attr_iter.next() {
+            self.out.indent_level += 1;
+
+            if !props_same_line {
+                self.write_attr_comments(
+                    brace,
+                    match attr {
+                        AttrType::Attr(attr) => attr.span(),
+                        AttrType::Spread(attr) => attr.expr.span(),
+                    },
+                )?;
+            }
+
+            self.out.indent_level -= 1;
+
+            if !props_same_line {
+                self.out.indented_tab()?;
+            }
+
+            match attr {
+                AttrType::Attr(attr) => self.write_attribute(attr)?,
+                AttrType::Spread(attr) => self.write_spread_attribute(&attr.expr)?,
+            }
+
+            let span = match attr {
+                AttrType::Attr(attr) => attr
+                    .comma
+                    .as_ref()
+                    .map(|c| c.span())
+                    .unwrap_or_else(|| self.final_span_of_attr(attr)),
+                AttrType::Spread(attr) => attr.span(),
             };
 
-            // trim the leading whitespace
-            let previous_indent = self.out.indent.count_indents(line);
-            let offset = previous_indent.saturating_sub(prev_block_indent_level);
-            let required_indent = self.out.indent_level + offset;
-            self.out.write_tabs(required_indent)?;
+            let has_more = attr_iter.peek().is_some();
+            let should_finish_comma = has_attributes && has_children || !props_same_line;
 
-            let line = line.trim_start();
-            write!(self.out, "{line}")?;
+            if has_more || should_finish_comma {
+                write!(self.out, ",")?;
+            }
+
+            if !props_same_line {
+                self.write_inline_comments(span.end(), 0)?;
+            }
+
+            if props_same_line && !has_more {
+                self.write_inline_comments(span.end(), 0)?;
+            }
+
+            if props_same_line && has_more {
+                write!(self.out, " ")?;
+            }
+
+            if !props_same_line && has_more {
+                self.out.new_line()?;
+            }
         }
 
         Ok(())
     }
 
-    pub fn write_attr_comments(&mut self, brace: &Brace, attr_span: Span) -> Result {
+    fn write_attribute(&mut self, attr: &Attribute) -> Result {
+        self.write_attribute_name(&attr.name)?;
+
+        // if the attribute is a shorthand, we don't need to write the colon, just the name
+        if !attr.can_be_shorthand() {
+            write!(self.out, ": ")?;
+            self.write_attribute_value(&attr.value)?;
+        }
+
+        Ok(())
+    }
+
+    fn write_attribute_name(&mut self, attr: &AttributeName) -> Result {
+        match attr {
+            AttributeName::BuiltIn(name) => write!(self.out, "{}", name),
+            AttributeName::Custom(name) => write!(self.out, "{}", name.to_token_stream()),
+            AttributeName::Spread(_) => unreachable!(),
+        }
+    }
+
+    fn write_attribute_value(&mut self, value: &AttributeValue) -> Result {
+        match value {
+            AttributeValue::IfExpr(if_chain) => {
+                self.write_attribute_if_chain(if_chain)?;
+            }
+            AttributeValue::AttrLiteral(value) => {
+                write!(self.out, "{value}")?;
+            }
+            AttributeValue::Shorthand(value) => {
+                write!(self.out, "{value}")?;
+            }
+            AttributeValue::EventTokens(closure) => {
+                self.out.indent_level += 1;
+                self.write_partial_expr(closure.as_expr(), closure.span())?;
+                self.out.indent_level -= 1;
+            }
+            AttributeValue::AttrExpr(value) => {
+                self.out.indent_level += 1;
+                self.write_partial_expr(value.as_expr(), value.span())?;
+                self.out.indent_level -= 1;
+            }
+        }
+
+        Ok(())
+    }
+
+    fn write_attribute_if_chain(&mut self, if_chain: &IfAttributeValue) -> Result {
+        let cond = self.unparse_expr(&if_chain.condition);
+        write!(self.out, "if {cond} {{ ")?;
+        self.write_attribute_value(&if_chain.then_value)?;
+        write!(self.out, " }}")?;
+        match if_chain.else_value.as_deref() {
+            Some(AttributeValue::IfExpr(else_if_chain)) => {
+                write!(self.out, " else ")?;
+                self.write_attribute_if_chain(else_if_chain)?;
+            }
+            Some(other) => {
+                write!(self.out, " else {{ ")?;
+                self.write_attribute_value(other)?;
+                write!(self.out, " }}")?;
+            }
+            None => {}
+        }
+
+        Ok(())
+    }
+
+    fn write_attr_comments(&mut self, brace: &Brace, attr_span: Span) -> Result {
         // There's a chance this line actually shares the same line as the previous
         // Only write comments if the comments actually belong to this line
         //
@@ -183,73 +567,80 @@ impl<'a> Writer<'a> {
         let attr_line = attr_span.start().line;
 
         if brace_line != attr_line {
-            self.write_comments(attr_span)?;
+            self.write_comments(attr_span.start())?;
         }
 
         Ok(())
     }
 
-    pub fn write_comments(&mut self, child: Span) -> Result {
+    fn write_inline_comments(&mut self, final_span: LineColumn, offset: usize) -> Result {
+        let line = final_span.line;
+        let column = final_span.column;
+        let Some(src_line) = self.src.get(line - 1) else {
+            return Ok(());
+        };
+
+        // the line might contain emoji or other unicode characters - this will cause issues
+        let Some(mut whitespace) = src_line.get(column..).map(|s| s.trim()) else {
+            return Ok(());
+        };
+
+        if whitespace.is_empty() {
+            return Ok(());
+        }
+
+        whitespace = whitespace[offset..].trim();
+
+        if whitespace.starts_with("//") {
+            write!(self.out, " {whitespace}")?;
+        }
+
+        Ok(())
+    }
+    fn accumulate_comments(&mut self, loc: LineColumn) -> VecDeque<usize> {
         // collect all comments upwards
         // make sure we don't collect the comments of the node that we're currently under.
-
-        let start = child.start();
+        let start = loc;
         let line_start = start.line - 1;
 
+        let mut comments = VecDeque::new();
+
         for (id, line) in self.src[..line_start].iter().enumerate().rev() {
-            if line.trim().starts_with("//") || line.is_empty() {
+            if line.trim().starts_with("//") || line.is_empty() && id != 0 {
                 if id != 0 {
-                    self.comments.push_front(id);
+                    comments.push_front(id);
                 }
             } else {
                 break;
             }
         }
 
-        let mut last_was_empty = false;
-        while let Some(comment_line) = self.comments.pop_front() {
-            let line = &self.src[comment_line];
+        comments
+    }
+    fn apply_comments(&mut self, mut comments: VecDeque<usize>) -> Result {
+        while let Some(comment_line) = comments.pop_front() {
+            let line = &self.src[comment_line].trim();
+
             if line.is_empty() {
-                if !last_was_empty {
-                    self.out.new_line()?;
-                }
-                last_was_empty = true;
+                self.out.new_line()?;
             } else {
-                last_was_empty = false;
-                self.out.tabbed_line()?;
-                write!(self.out, "{}", self.src[comment_line].trim())?;
+                self.out.tab()?;
+                writeln!(self.out, "{}", line.trim())?;
             }
         }
-
-        Ok(())
-    }
-
-    // Push out the indent level and write each component, line by line
-    pub fn write_body_indented(&mut self, children: &[BodyNode]) -> Result {
-        self.out.indent_level += 1;
-
-        self.write_body_no_indent(children)?;
-
-        self.out.indent_level -= 1;
         Ok(())
     }
 
-    pub fn write_body_no_indent(&mut self, children: &[BodyNode]) -> Result {
-        for child in children {
-            if self.current_span_is_primary(child.span()) {
-                self.write_comments(child.span())?;
-            };
-
-            self.out.tabbed_line()?;
-            self.write_ident(child)?;
-        }
+    fn write_comments(&mut self, loc: LineColumn) -> Result {
+        let comments = self.accumulate_comments(loc);
+        self.apply_comments(comments)?;
 
         Ok(())
     }
 
-    pub(crate) fn attr_value_len(&mut self, value: &ElementAttrValue) -> usize {
+    fn attr_value_len(&mut self, value: &AttributeValue) -> usize {
         match value {
-            ElementAttrValue::IfExpr(if_chain) => {
+            AttributeValue::IfExpr(if_chain) => {
                 let condition_len = self.retrieve_formatted_expr(&if_chain.condition).len();
                 let value_len = self.attr_value_len(&if_chain.then_value);
                 let if_len = 2;
@@ -262,13 +653,16 @@ impl<'a> Writer<'a> {
                     .unwrap_or_default();
                 condition_len + value_len + if_len + brace_len + space_len + else_len
             }
-            ElementAttrValue::AttrLiteral(lit) => lit.to_string().len(),
-            ElementAttrValue::Shorthand(expr) => expr.span().line_length(),
-            ElementAttrValue::AttrExpr(expr) => expr
+            AttributeValue::AttrLiteral(lit) => lit.to_string().len(),
+            AttributeValue::Shorthand(expr) => {
+                let span = &expr.span();
+                span.end().line - span.start().line
+            }
+            AttributeValue::AttrExpr(expr) => expr
                 .as_expr()
                 .map(|expr| self.attr_expr_len(&expr))
                 .unwrap_or(100000),
-            ElementAttrValue::EventTokens(closure) => closure
+            AttributeValue::EventTokens(closure) => closure
                 .as_expr()
                 .map(|expr| self.attr_expr_len(&expr))
                 .unwrap_or(100000),
@@ -284,11 +678,7 @@ impl<'a> Writer<'a> {
         }
     }
 
-    pub(crate) fn is_short_attrs(
-        &mut self,
-        attributes: &[AttributeType],
-        spreads: &[Spread],
-    ) -> usize {
+    fn is_short_attrs(&mut self, attributes: &[Attribute], spreads: &[Spread]) -> usize {
         let mut total = 0;
 
         // No more than 3 attributes before breaking the line
@@ -297,7 +687,7 @@ impl<'a> Writer<'a> {
         }
 
         for attr in attributes {
-            if self.current_span_is_primary(attr.span()) {
+            if self.current_span_is_primary(attr.span().start()) {
                 'line: for line in self.src[..attr.span().start().line - 1].iter().rev() {
                     match (line.trim().starts_with("//"), line.is_empty()) {
                         (true, _) => return 100000,
@@ -317,7 +707,6 @@ impl<'a> Writer<'a> {
             };
             total += name_len;
 
-            //
             if attr.can_be_shorthand() {
                 total += 2;
             } else {
@@ -335,114 +724,221 @@ impl<'a> Writer<'a> {
         total
     }
 
-    #[allow(clippy::map_entry)]
-    pub fn retrieve_formatted_expr(&mut self, expr: &Expr) -> &str {
-        let loc = expr.span().start();
+    fn write_todo_body(&mut self, brace: &Brace) -> std::fmt::Result {
+        let span = brace.span.span();
+        let start = span.start();
+        let end = span.end();
 
-        if !self.cached_formats.contains_key(&loc) {
-            let formatted = self.unparse_expr(expr);
-            self.cached_formats.insert(loc, formatted);
+        if start.line == end.line {
+            return Ok(());
+        }
+
+        writeln!(self.out)?;
+
+        for idx in start.line..end.line {
+            let line = &self.src[idx];
+            if line.trim().starts_with("//") {
+                for _ in 0..self.out.indent_level + 1 {
+                    write!(self.out, "    ")?
+                }
+                writeln!(self.out, "{}", line.trim())?;
+            }
+        }
+
+        for _ in 0..self.out.indent_level {
+            write!(self.out, "    ")?
         }
 
-        self.cached_formats.get(&loc).unwrap().as_str()
+        Ok(())
     }
 
-    fn write_for_loop(&mut self, forloop: &ForLoop) -> std::fmt::Result {
-        write!(
-            self.out,
-            "for {} in ",
-            forloop.pat.clone().into_token_stream(),
-        )?;
+    fn write_partial_expr(&mut self, expr: syn::Result<Expr>, src_span: Span) -> Result {
+        let Ok(expr) = expr else {
+            self.invalid_exprs.push(src_span);
+            return Err(std::fmt::Error);
+        };
 
-        self.write_inline_expr(&forloop.expr)?;
+        let pretty_expr = self.retrieve_formatted_expr(&expr).to_string();
+        self.write_mulitiline_tokens(pretty_expr)?;
 
-        if forloop.body.is_empty() {
-            write!(self.out, "}}")?;
-            return Ok(());
-        }
+        Ok(())
+    }
 
-        self.write_body_indented(&forloop.body.roots)?;
+    fn write_mulitiline_tokens(&mut self, out: String) -> Result {
+        let mut lines = out.split('\n').peekable();
+        let first = lines.next().unwrap();
 
-        self.out.tabbed_line()?;
-        write!(self.out, "}}")?;
+        // a one-liner for whatever reason
+        // Does not need a new line
+        if lines.peek().is_none() {
+            write!(self.out, "{first}")?;
+        } else {
+            writeln!(self.out, "{first}")?;
+
+            while let Some(line) = lines.next() {
+                self.out.tab()?;
+                write!(self.out, "{line}")?;
+                if lines.peek().is_none() {
+                    write!(self.out, "")?;
+                } else {
+                    writeln!(self.out)?;
+                }
+            }
+        }
 
         Ok(())
     }
 
-    fn write_if_chain(&mut self, ifchain: &IfChain) -> std::fmt::Result {
-        // Recurse in place by setting the next chain
-        let mut branch = Some(ifchain);
+    fn write_spread_attribute(&mut self, attr: &Expr) -> Result {
+        let formatted = self.unparse_expr(attr);
 
-        while let Some(chain) = branch {
-            let IfChain {
-                if_token,
-                cond,
-                then_branch,
-                else_if_branch,
-                else_branch,
-                ..
-            } = chain;
+        let mut lines = formatted.lines();
 
-            write!(self.out, "{} ", if_token.to_token_stream(),)?;
+        let first_line = lines.next().unwrap();
 
-            self.write_inline_expr(cond)?;
+        write!(self.out, "..{first_line}")?;
+        for line in lines {
+            self.out.indented_tabbed_line()?;
+            write!(self.out, "{line}")?;
+        }
 
-            self.write_body_indented(&then_branch.roots)?;
+        Ok(())
+    }
 
-            if let Some(else_if_branch) = else_if_branch {
-                // write the closing bracket and else
-                self.out.tabbed_line()?;
-                write!(self.out, "}} else ")?;
+    // check if the children are short enough to be on the same line
+    // We don't have the notion of current line depth - each line tries to be < 80 total
+    // returns the total line length if it's short
+    // returns none if the length exceeds the limit
+    // I think this eventually becomes quadratic :(
+    fn is_short_children(&mut self, children: &[BodyNode]) -> syn::Result<Option<usize>> {
+        if children.is_empty() {
+            return Ok(Some(0));
+        }
 
-                branch = Some(else_if_branch);
-            } else if let Some(else_branch) = else_branch {
-                self.out.tabbed_line()?;
-                write!(self.out, "}} else {{")?;
+        // Any comments push us over the limit automatically
+        if self.children_have_comments(children) {
+            return Ok(None);
+        }
 
-                self.write_body_indented(&else_branch.roots)?;
-                branch = None;
-            } else {
-                branch = None;
+        let res = match children {
+            [BodyNode::Text(ref text)] => Some(text.input.to_string_with_quotes().len()),
+
+            // TODO: let rawexprs to be inlined
+            [BodyNode::RawExpr(ref expr)] => {
+                let pretty = self.retrieve_formatted_expr(&expr.expr.as_expr()?);
+                if pretty.contains('\n') {
+                    None
+                } else {
+                    Some(pretty.len() + 2)
+                }
+            }
+
+            // TODO: let rawexprs to be inlined
+            [BodyNode::Component(ref comp)] if comp.fields.is_empty() => Some(
+                comp.name
+                    .segments
+                    .iter()
+                    .map(|s| s.ident.to_string().len() + 2)
+                    .sum::<usize>(),
+            ),
+
+            // Feedback on discord indicates folks don't like combining multiple children on the same line
+            // We used to do a lot of math to figure out if we should expand out the line, but folks just
+            // don't like it.
+            _ => None,
+        };
+
+        Ok(res)
+    }
+
+    fn children_have_comments(&self, children: &[BodyNode]) -> bool {
+        for child in children {
+            if self.current_span_is_primary(child.span().start()) {
+                'line: for line in self.src[..child.span().start().line - 1].iter().rev() {
+                    match (line.trim().starts_with("//"), line.is_empty()) {
+                        (true, _) => return true,
+                        (_, true) => continue 'line,
+                        _ => break 'line,
+                    }
+                }
             }
         }
 
-        self.out.tabbed_line()?;
-        write!(self.out, "}}")?;
+        false
+    }
 
-        Ok(())
+    // make sure the comments are actually relevant to this element.
+    // test by making sure this element is the primary element on this line (nothing else before it)
+    fn current_span_is_primary(&self, location: LineColumn) -> bool {
+        self.leading_row_is_empty(LineColumn {
+            line: location.line,
+            column: location.column + 1,
+        })
     }
 
-    /// An expression within a for or if block that might need to be spread out across several lines
-    fn write_inline_expr(&mut self, expr: &Expr) -> std::fmt::Result {
-        let unparsed = self.unparse_expr(expr);
-        let mut lines = unparsed.lines();
-        let first_line = lines.next().unwrap();
-        write!(self.out, "{first_line}")?;
+    fn leading_row_is_empty(&self, location: LineColumn) -> bool {
+        let Some(line) = self.src.get(location.line - 1) else {
+            return false;
+        };
 
-        let mut was_multiline = false;
+        let Some(sub) = line.get(..location.column - 1) else {
+            return false;
+        };
 
-        for line in lines {
-            was_multiline = true;
-            self.out.tabbed_line()?;
-            write!(self.out, "{line}")?;
-        }
+        sub.trim().is_empty()
+    }
 
-        if was_multiline {
-            self.out.tabbed_line()?;
-            write!(self.out, "{{")?;
-        } else {
-            write!(self.out, " {{")?;
+    #[allow(clippy::map_entry)]
+    fn retrieve_formatted_expr(&mut self, expr: &Expr) -> &str {
+        let loc = expr.span().start();
+
+        if !self.cached_formats.contains_key(&loc) {
+            let formatted = self.unparse_expr(expr);
+            self.cached_formats.insert(loc, formatted);
         }
 
-        Ok(())
+        self.cached_formats
+            .get(&loc)
+            .expect("Just inserted the parsed expr, so it should be in the cache")
+            .as_str()
     }
-}
 
-pub(crate) trait SpanLength {
-    fn line_length(&self) -> usize;
-}
-impl SpanLength for Span {
-    fn line_length(&self) -> usize {
-        self.end().line - self.start().line
+    fn final_span_of_node(node: &BodyNode) -> Span {
+        // Write the trailing comments if there are any
+        // Get the ending span of the node
+        let span = match node {
+            BodyNode::Element(el) => el
+                .brace
+                .as_ref()
+                .map(|b| b.span.span())
+                .unwrap_or_else(|| el.name.span()),
+            BodyNode::Component(el) => el
+                .brace
+                .as_ref()
+                .map(|b| b.span.span())
+                .unwrap_or_else(|| el.name.span()),
+            BodyNode::Text(txt) => txt.input.span(),
+            BodyNode::RawExpr(exp) => exp.span(),
+            BodyNode::ForLoop(f) => f.brace.span.span(),
+            BodyNode::IfChain(i) => match i.else_brace {
+                Some(b) => b.span.span(),
+                None => i.then_brace.span.span(),
+            },
+        };
+        span
+    }
+
+    fn final_span_of_attr(&self, attr: &Attribute) -> Span {
+        match &attr.value {
+            AttributeValue::Shorthand(s) => s.span(),
+            AttributeValue::AttrLiteral(l) => l.span(),
+            AttributeValue::EventTokens(closure) => closure.body.span(),
+            AttributeValue::AttrExpr(exp) => exp.span(),
+            AttributeValue::IfExpr(ex) => ex
+                .else_value
+                .as_ref()
+                .map(|v| v.span())
+                .unwrap_or_else(|| ex.then_value.span()),
+        }
     }
 }

+ 21 - 0
packages/autofmt/tests/error_handling.rs

@@ -0,0 +1,21 @@
+#[test]
+fn no_parse() {
+    let src = include_str!("./partials/no_parse.rsx");
+    assert!(syn::parse_file(src).is_err());
+}
+
+#[test]
+fn parses_but_fmt_fails() {
+    let src = include_str!("./partials/wrong.rsx");
+    let file = syn::parse_file(src).unwrap();
+    let formatted = dioxus_autofmt::try_fmt_file(src, &file, Default::default());
+    assert!(&formatted.is_err());
+}
+
+#[test]
+fn parses_and_is_okay() {
+    let src = include_str!("./partials/okay.rsx");
+    let file = syn::parse_file(src).unwrap();
+    let formatted = dioxus_autofmt::try_fmt_file(src, &file, Default::default()).unwrap();
+    assert_ne!(formatted.len(), 0);
+}

+ 8 - 0
packages/autofmt/tests/partials/no_parse.rsx

@@ -0,0 +1,8 @@
+#[component]
+fn SidebarSection() -> Element {
+    rsx! {
+        div {
+            { .doesnt_work) }
+        }
+    }
+}

+ 10 - 0
packages/autofmt/tests/partials/okay.rsx

@@ -0,0 +1,10 @@
+#[component]
+fn SidebarSection() -> Element {
+    rsx! {
+        div {
+            onclick: move |_| {
+                works()
+            }
+        }
+    }
+}

+ 10 - 0
packages/autofmt/tests/partials/wrong.rsx

@@ -0,0 +1,10 @@
+#[component]
+fn SidebarSection() -> Element {
+    rsx! {
+        div {
+            onclick: move |_| {
+                .doesnt_work()
+            }
+        }
+    }
+}

+ 14 - 9
packages/autofmt/tests/samples.rs

@@ -1,3 +1,5 @@
+#![allow(deprecated)]
+
 macro_rules! twoway {
     (
         $(
@@ -22,35 +24,38 @@ macro_rules! twoway {
         )*
     };
 }
-
 twoway![
     attributes,
+    basic_expr,
     collapse_expr,
     comments,
     commentshard,
     complex,
+    docsite,
     emoji,
+    fat_exprs,
     ifchain_forloop,
     immediate_expr,
     key,
+    letsome,
     long_exprs,
     long,
     manual_props,
+    many_exprs,
     messy_indent,
+    misplaced,
     multirsx,
+    nested,
     raw_strings,
     reallylong,
+    shorthand,
     simple,
+    skip,
+    spaces,
+    staged,
     t2,
     tiny,
     tinynoopt,
     trailing_expr,
-    many_exprs,
-    shorthand,
-    docsite,
-    letsome,
-    fat_exprs,
-    nested,
-    staged,
-    misplaced
+    oneline
 ];

+ 2 - 2
packages/autofmt/tests/samples/attributes.rsx

@@ -30,7 +30,7 @@ rsx! {
         a: "123",
         a: "123",
         a: "123",
-        a: "123"
+        a: "123",
     }
 
     div {
@@ -42,6 +42,6 @@ rsx! {
         a: "123",
         a: "123",
         a: "123",
-        a: "123"
+        a: "123",
     }
 }

+ 8 - 0
packages/autofmt/tests/samples/basic_expr.rsx

@@ -0,0 +1,8 @@
+fn itworks() {
+    rsx! {
+        div {
+            "hi"
+            {children}
+        }
+    }
+}

+ 26 - 0
packages/autofmt/tests/samples/comments.rsx

@@ -33,4 +33,30 @@ rsx! {
         class: "asd",
         "Jon"
     }
+
+    // comments inline
+    div { // inline
+        // Collapse
+        class: "asd", // super inline
+        class: "asd", // super inline
+        "Jon" // all the inline
+        // Comments at the end too
+    }
+
+    // please dont eat me 1
+    div { // please dont eat me 2
+        // please dont eat me 3
+    }
+
+    // please dont eat me 1
+    div { // please dont eat me 2
+        // please dont eat me 3
+        abc: 123,
+    }
+
+    // please dont eat me 1
+    div {
+        // please dont eat me 3
+        abc: 123,
+    }
 }

+ 1 - 1
packages/autofmt/tests/samples/commentshard.rsx

@@ -36,7 +36,7 @@ rsx! {
             class: "hello world",
 
             // todo some work in here
-            class: "hello world"
+            class: "hello world",
         }
 
         div {

+ 7 - 4
packages/autofmt/tests/samples/complex.rsx

@@ -22,14 +22,17 @@ rsx! {
                 span {
                     class: "inline-block ml-auto hover:bg-gray-500",
                     onclick: move |evt| {
-                        // open.set(!open.get());
                         evt.cancel_bubble();
                     },
                     icons::icon_8 {}
                 }
             })}
         }
-        div { class: "px-4", {is_current.then(|| rsx!{ children })} }
+        div { class: "px-4",
+            {is_current.then(|| rsx! {
+                {children}
+            })}
+        }
     }
 
     // No nesting
@@ -37,7 +40,7 @@ rsx! {
         adsasd: "asd",
         onclick: move |_| {
             let blah = 120;
-        }
+        },
     }
 
     // Component path
@@ -45,7 +48,7 @@ rsx! {
         adsasd: "asd",
         onclick: move |_| {
             let blah = 120;
-        }
+        },
     }
 
     for i in 0..10 {

+ 2 - 2
packages/autofmt/tests/samples/docsite.rsx

@@ -15,7 +15,7 @@ pub(crate) fn Nav() -> Element {
                     MaterialIcon {
                         name: "menu",
                         size: 24,
-                        color: MaterialIconColor::Dark
+                        color: MaterialIconColor::Dark,
                     }
                 }
                 div { class: "flex z-50 md:flex-1 px-2", LinkList {} }
@@ -59,7 +59,7 @@ pub(crate) fn Nav() -> Element {
                                 Link { to: Route::Homepage {},
                                     img {
                                         src: "https://avatars.githubusercontent.com/u/10237910?s=40&v=4",
-                                        class: "ml-4 h-10 rounded-full w-auto"
+                                        class: "ml-4 h-10 rounded-full w-auto",
                                     }
                                 }
                             }

+ 11 - 0
packages/autofmt/tests/samples/ifchain_forloop.rsx

@@ -24,4 +24,15 @@ rsx! {
     } else {
         h3 {}
     }
+
+    div {
+        class: "asdasd",
+        class: if expr { "asdasd" } else { "asdasd" },
+        class: if expr { "asdasd" },
+        class: if expr { "asdasd" } else if expr { "asdasd" } else { "asdasd" },
+
+        // comments?
+        class: if expr { "asdasd" } else if expr { "asdasd" } else { "asdasd" }, // comments!!?
+        // comments?
+    }
 }

+ 3 - 1
packages/autofmt/tests/samples/long_exprs.rsx

@@ -6,7 +6,9 @@ rsx! {
                     section { class: "body-font overflow-hidden dark:bg-ideblack",
                         div { class: "container px-6 mx-auto",
                             div { class: "-my-8 divide-y-2 divide-gray-100",
-                                {POSTS.iter().enumerate().map(|(id, post)| rsx! { BlogPostItem { post: post, id: id } })}
+                                {POSTS.iter().enumerate().map(|(id, post)| rsx! {
+                                    BlogPostItem { post, id }
+                                })}
                             }
                         }
                     }

+ 1 - 1
packages/autofmt/tests/samples/manual_props.rsx

@@ -10,7 +10,7 @@ rsx! {
         Component {
             asdasd: "asdasd",
             asdasd: "asdasdasdasdasdasdasdasdasdasd",
-            ..Props { a: 10, b: 20 }
+            ..Props { a: 10, b: 20 },
         }
         Component {
             asdasd: "asdasd",

+ 39 - 15
packages/autofmt/tests/samples/many_exprs.rsx

@@ -103,12 +103,21 @@ fn app() -> Element {
     rsx! {
         div {
             {
-                let millis = timer.with(|t| t.duration().saturating_sub(t.started_at.map(|x| x.elapsed()).unwrap_or(Duration::ZERO)).as_millis());
-                format!("{:02}:{:02}:{:02}.{:01}",
-                        millis / 1000 / 3600 % 3600,
-                        millis / 1000 / 60 % 60,
-                        millis / 1000 % 60,
-                        millis / 100 % 10)
+                let millis = timer
+                    .with(|t| {
+                        t.duration()
+                            .saturating_sub(
+                                t.started_at.map(|x| x.elapsed()).unwrap_or(Duration::ZERO),
+                            )
+                            .as_millis()
+                    });
+                format!(
+                    "{:02}:{:02}:{:02}.{:01}",
+                    millis / 1000 / 3600 % 3600,
+                    millis / 1000 / 60 % 60,
+                    millis / 1000 % 60,
+                    millis / 100 % 10,
+                )
             }
         }
         div {
@@ -119,7 +128,7 @@ fn app() -> Element {
                 value: format!("{:02}", timer.read().hours),
                 oninput: move |e| {
                     timer.write().hours = e.value().parse().unwrap_or(0);
-                }
+                },
             }
 
             input {
@@ -129,7 +138,7 @@ fn app() -> Element {
                 value: format!("{:02}", timer.read().minutes),
                 oninput: move |e| {
                     timer.write().minutes = e.value().parse().unwrap_or(0);
-                }
+                },
             }
 
             input {
@@ -139,7 +148,7 @@ fn app() -> Element {
                 value: format!("{:02}", timer.read().seconds),
                 oninput: move |e| {
                     timer.write().seconds = e.value().parse().unwrap_or(0);
-                }
+                },
             }
         }
 
@@ -155,7 +164,7 @@ fn app() -> Element {
                         }
                     })
             },
-            { timer.with(|t| if t.started_at.is_none() { "Start" } else { "Stop" }) }
+            {timer.with(|t| if t.started_at.is_none() { "Start" } else { "Stop" })}
         }
         div { id: "app",
             button {
@@ -165,7 +174,10 @@ fn app() -> Element {
                     window_preferences.write().with_decorations = !decorations;
                 },
                 {
-                    format!("with decorations{}", if window_preferences.read().with_decorations { " ✓" } else { "" }).to_string()
+                    format!(
+                        "with decorations{}",
+                        if window_preferences.read().with_decorations { " ✓" } else { "" },
+                    )
                 }
             }
             button {
@@ -178,16 +190,28 @@ fn app() -> Element {
                 },
                 width: 100,
                 {
-                    format!("always on top{}", if window_preferences.read().always_on_top { " ✓" } else { "" })
+                    format!(
+                        "always on top{}",
+                        if window_preferences.read().always_on_top { " ✓" } else { "" },
+                    )
                 }
             }
         }
         {
             exit_button(
                 Duration::from_secs(3),
-                |trigger, delay| rsx! {
-                    {format!("{:0.1?}", trigger.read().map(|inst| (delay.as_secs_f32() - inst.elapsed().as_secs_f32()))) }
-                }
+                |trigger, delay| {
+                    rsx! {
+                        {
+                            format!(
+                                "{:0.1?}",
+                                trigger
+                                    .read()
+                                    .map(|inst| (delay.as_secs_f32() - inst.elapsed().as_secs_f32())),
+                            )
+                        }
+                    }
+                },
             )
         }
     }

+ 1 - 1
packages/autofmt/tests/samples/misplaced.rsx

@@ -15,7 +15,7 @@ pub(crate) fn Nav() -> Element {
                     MaterialIcon {
                         name: "menu",
                         size: 24,
-                        color: MaterialIconColor::Dark
+                        color: MaterialIconColor::Dark,
                     }
                 }
                 div { class: "flex z-50 md:flex-1 px-2", LinkList {} }

+ 13 - 11
packages/autofmt/tests/samples/nested.rsx

@@ -36,7 +36,7 @@ fn App() -> Element {
                             "hi!!"
                             "womh"
                         }
-                    };
+                    }
                     println!("hi")
                 },
                 "hi"
@@ -83,7 +83,7 @@ fn App() -> Element {
                                                                 "so22mething nested?"
                                                             }
                                                         }
-                                                    }
+                                                    },
                                                 }
                                             }
                                         };
@@ -93,11 +93,11 @@ fn App() -> Element {
                                                 "something nested?"
                                             }
                                         }
-                                    }
+                                    },
                                 }
-                            }
+                            },
                         }
-                    }
+                    },
                 }
             },
             onrender: move |_| {
@@ -121,12 +121,14 @@ fn App() -> Element {
                                 }
                             }
                         },
-                        {rsx! {
-                            div2 {
-                                h12 { "hi" }
-                                "so22mething nested?"
+                        {
+                            rsx! {
+                                div2 {
+                                    h12 { "hi" }
+                                    "so22mething nested?"
+                                }
                             }
-                        }}
+                        }
                     }
                 }
             },
@@ -138,7 +140,7 @@ fn App() -> Element {
                             "something nested?"
                         }
                     };
-                }
+                },
             }
         }
     }

+ 1 - 0
packages/autofmt/tests/samples/oneline.rsx

@@ -0,0 +1 @@
+rsx! { "hello world" }

+ 1 - 1
packages/autofmt/tests/samples/raw_strings.rsx

@@ -12,6 +12,6 @@ rsx! {
         width: r#"10px"#,
         height: r##"{10}px"##,
         "raw-attr": r###"raw-attr"###,
-        "raw-attr2": r###"{100}"###
+        "raw-attr2": r###"{100}"###,
     }
 }

+ 1 - 1
packages/autofmt/tests/samples/simple.rsx

@@ -16,7 +16,7 @@ rsx! {
         id: "{a}",
         class: "ban",
         style: "color: red",
-        value: "{b}"
+        value: "{b}",
     }
 
     // Nested one level

+ 36 - 0
packages/autofmt/tests/samples/skip.rsx

@@ -0,0 +1,36 @@
+/// dont format this component
+#[rustfmt::skip]
+#[component]
+fn SidebarSection() -> Element {
+    rsx! {
+        div {
+            "hi" div {} div {}
+        }
+    }
+}
+
+/// dont format this component
+#[component]
+fn SidebarSection() -> Element {
+    // format this
+    rsx! {
+        div { "hi" }
+    }
+
+    // and this
+    rsx! {
+        div {
+            "hi"
+            div {}
+            div {}
+        }
+    }
+
+    // but not this
+    #[rustfmt::skip]
+    rsx! {
+        div {
+            "hi" div {} div {}
+        }
+    }
+}

+ 14 - 0
packages/autofmt/tests/samples/spaces.rsx

@@ -0,0 +1,14 @@
+rsx! {
+    if let Some(Some(record)) = &*records.read_unchecked() {
+        {
+            let (label, value): (Vec<String>, Vec<f64>) = record
+                .iter()
+                .rev()
+                .map(|d| (d.model.clone().expect("work"), d.row_total))
+                .collect();
+            rsx! {
+                BarChart { id: "bar-plot".to_string(), x: value, y: label }
+            }
+        }
+    }
+}

+ 9 - 7
packages/autofmt/tests/samples/staged.rsx

@@ -9,18 +9,20 @@ rsx! {
 
     div { {some_expr} }
     div {
-        {
-            POSTS.iter().enumerate().map(|(id, post)| rsx! {
-                BlogPostItem { post, id }
-            })
-        }
+        {POSTS.iter().enumerate().map(|(id, post)| rsx! {
+            BlogPostItem { post, id }
+        })}
     }
 
     div { class: "123123123123123123123123123123123123",
-        {some_really_long_expr_some_really_long_expr_some_really_long_expr_some_really_long_expr_}
+        {
+            some_really_long_expr_some_really_long_expr_some_really_long_expr_some_really_long_expr_
+        }
     }
 
     div { class: "-my-8 divide-y-2 divide-gray-100",
-        {POSTS.iter().enumerate().map(|(id, post)| rsx! { BlogPostItem { post: post, id: id } })}
+        {POSTS.iter().enumerate().map(|(id, post)| rsx! {
+            BlogPostItem { post, id }
+        })}
     }
 }

+ 9 - 1
packages/autofmt/tests/wrong.rs

@@ -1,3 +1,5 @@
+#![allow(deprecated)]
+
 use dioxus_autofmt::{IndentOptions, IndentType};
 
 macro_rules! twoway {
@@ -6,7 +8,12 @@ macro_rules! twoway {
         fn $name() {
             let src_right = include_str!(concat!("./wrong/", $val, ".rsx"));
             let src_wrong = include_str!(concat!("./wrong/", $val, ".wrong.rsx"));
-            let formatted = dioxus_autofmt::fmt_file(src_wrong, $indent);
+
+            let parsed = syn::parse_file(src_wrong)
+                .expect("fmt_file should only be called on valid syn::File files");
+
+            let formatted =
+                dioxus_autofmt::try_fmt_file(src_wrong, &parsed, $indent).unwrap_or_default();
             let out = dioxus_autofmt::apply_formats(src_wrong, formatted);
 
             // normalize line endings
@@ -31,3 +38,4 @@ twoway!("simple-combo-expr" => simple_combo_expr (IndentOptions::new(IndentType:
 twoway!("oneline-expand" => online_expand (IndentOptions::new(IndentType::Spaces, 4, false)));
 twoway!("shortened" => shortened (IndentOptions::new(IndentType::Spaces, 4, false)));
 twoway!("syntax_error" => syntax_error (IndentOptions::new(IndentType::Spaces, 4, false)));
+twoway!("skipfail" => skipfail (IndentOptions::new(IndentType::Spaces, 4, false)));

+ 38 - 14
packages/autofmt/tests/wrong/multiexpr-many.rsx

@@ -103,12 +103,22 @@ fn app() -> Element {
     rsx! {
         div {
             {
-                let millis = timer.with(|t| t.duration().saturating_sub(t.started_at.map(|x| x.elapsed()).unwrap_or(Duration::ZERO)).as_millis());
-                format!("{:02}:{:02}:{:02}.{:01}",
-                        millis / 1000 / 3600 % 3600,
-                        millis / 1000 / 60 % 60,
-                        millis / 1000 % 60,
-                        millis / 100 % 10)
+                let millis = timer
+                    .with(|t| {
+                        t
+                            .duration()
+                            .saturating_sub(
+                                t.started_at.map(|x| x.elapsed()).unwrap_or(Duration::ZERO),
+                            )
+                            .as_millis()
+                    });
+                format!(
+                    "{:02}:{:02}:{:02}.{:01}",
+                    millis / 1000 / 3600 % 3600,
+                    millis / 1000 / 60 % 60,
+                    millis / 1000 % 60,
+                    millis / 100 % 10,
+                )
             }
         }
         div {
@@ -119,7 +129,7 @@ fn app() -> Element {
                 value: format!("{:02}", timer.read().hours),
                 oninput: move |e| {
                     timer.write().hours = e.value().parse().unwrap_or(0);
-                }
+                },
             }
 
             input {
@@ -129,7 +139,7 @@ fn app() -> Element {
                 value: format!("{:02}", timer.read().minutes),
                 oninput: move |e| {
                     timer.write().minutes = e.value().parse().unwrap_or(0);
-                }
+                },
             }
 
             input {
@@ -139,7 +149,7 @@ fn app() -> Element {
                 value: format!("{:02}", timer.read().seconds),
                 oninput: move |e| {
                     timer.write().seconds = e.value().parse().unwrap_or(0);
-                }
+                },
             }
         }
 
@@ -155,7 +165,7 @@ fn app() -> Element {
                         };
                     })
             },
-            { timer.with(|t| if t.started_at.is_none() { "Start" } else { "Stop" }) }
+            {timer.with(|t| if t.started_at.is_none() { "Start" } else { "Stop" })}
         }
         div { id: "app",
             button {
@@ -165,7 +175,11 @@ fn app() -> Element {
                     window_preferences.write().with_decorations = !decorations;
                 },
                 {
-                    format!("with decorations{}", if window_preferences.read().with_decorations { " ✓" } else { "" }).to_string()
+                    format!(
+                        "with decorations{}",
+                        if window_preferences.read().with_decorations { " ✓" } else { "" },
+                    )
+                        .to_string()
                 }
             }
             button {
@@ -178,7 +192,10 @@ fn app() -> Element {
                 },
                 width: 100,
                 {
-                    format!("always on top{}", if window_preferences.read().always_on_top { " ✓" } else { "" })
+                    format!(
+                        "always on top{}",
+                        if window_preferences.read().always_on_top { " ✓" } else { "" },
+                    )
                 }
             }
         }
@@ -186,8 +203,15 @@ fn app() -> Element {
             exit_button(
                 Duration::from_secs(3),
                 |trigger, delay| rsx! {
-                    {format!("{:0.1?}", trigger.read().map(|inst| (delay.as_secs_f32() - inst.elapsed().as_secs_f32()))) }
-                }
+                    {
+                        format!(
+                            "{:0.1?}",
+                            trigger
+                                .read()
+                                .map(|inst| (delay.as_secs_f32() - inst.elapsed().as_secs_f32())),
+                        )
+                    }
+                },
             )
         }
     }

+ 1 - 1
packages/autofmt/tests/wrong/oneline-expand.rsx

@@ -11,7 +11,7 @@ fn main() {
                             None
                         };
                     })
-            }
+            },
         }
     }
 }

+ 18 - 8
packages/autofmt/tests/wrong/simple-combo-expr.rsx

@@ -2,12 +2,22 @@ fn main() {
     rsx! {
         div {
             {
-                let millis = timer.with(|t| t.duration().saturating_sub(t.started_at.map(|x| x.elapsed()).unwrap_or(Duration::ZERO)).as_millis());
-                format!("{:02}:{:02}:{:02}.{:01}",
-                        millis / 1000 / 3600 % 3600,
-                        millis / 1000 / 60 % 60,
-                        millis / 1000 % 60,
-                        millis / 100 % 10)
+                let millis = timer
+                    .with(|t| {
+                        t
+                            .duration()
+                            .saturating_sub(
+                                t.started_at.map(|x| x.elapsed()).unwrap_or(Duration::ZERO),
+                            )
+                            .as_millis()
+                    });
+                format!(
+                    "{:02}:{:02}:{:02}.{:01}",
+                    millis / 1000 / 3600 % 3600,
+                    millis / 1000 / 60 % 60,
+                    millis / 1000 % 60,
+                    millis / 100 % 10,
+                )
             }
         }
         div {
@@ -18,7 +28,7 @@ fn main() {
                 value: format!("{:02}", timer.read().hours),
                 oninput: move |e| {
                     timer.write().hours = e.value().parse().unwrap_or(0);
-                }
+                },
             }
             // some comment
             input {
@@ -28,7 +38,7 @@ fn main() {
                 value: format!("{:02}", timer.read().hours),
                 oninput: move |e| {
                     timer.write().hours = e.value().parse().unwrap_or(0);
-                }
+                },
             }
         }
     }

+ 36 - 0
packages/autofmt/tests/wrong/skipfail.rsx

@@ -0,0 +1,36 @@
+/// dont format this component
+#[rustfmt::skip]
+#[component]
+fn SidebarSection() -> Element {
+    rsx! {
+        div {
+            "hi" div {} div {}
+        }
+    }
+}
+
+/// dont format this component
+#[component]
+fn SidebarSection() -> Element {
+    // format this
+    rsx! {
+        div { "hi" }
+    }
+
+    // and this
+    rsx! {
+        div {
+            "hi"
+            div {}
+            div {}
+        }
+    }
+
+    // but not this
+    #[rustfmt::skip]
+    rsx! {
+        div {
+            "hi" div {} div {}
+        }
+    }
+}

+ 32 - 0
packages/autofmt/tests/wrong/skipfail.wrong.rsx

@@ -0,0 +1,32 @@
+/// dont format this component
+#[rustfmt::skip]
+#[component]
+fn SidebarSection() -> Element {
+    rsx! {
+        div {
+            "hi" div {} div {}
+        }
+    }
+}
+
+/// dont format this component
+#[component]
+fn SidebarSection() -> Element {
+    // format this
+    rsx! {
+        div { "hi" }
+    }
+
+    // and this
+    rsx! {
+        div { "hi" div {} div {} }
+    }
+
+    // but not this
+    #[rustfmt::skip]
+    rsx! {
+        div {
+            "hi" div {} div {}
+        }
+    }
+}

+ 11 - 28
packages/cli/src/cli/autoformat.rs

@@ -115,7 +115,13 @@ fn refactor_file(
         s = format_rust(&s)?;
     }
 
-    let edits = dioxus_autofmt::fmt_file(&s, indent);
+    let Ok(Ok(edits)) =
+        syn::parse_file(&s).map(|file| dioxus_autofmt::try_fmt_file(&s, &file, indent))
+    else {
+        eprintln!("failed to format file: {}", s);
+        exit(1);
+    };
+
     let out = dioxus_autofmt::apply_formats(&s, edits);
 
     if file == "-" {
@@ -159,7 +165,10 @@ fn format_file(
         }
     }
 
-    let edits = dioxus_autofmt::fmt_file(&contents, indent);
+    let parsed = syn::parse_file(&contents)
+        .map_err(|err| Error::ParseError(format!("Failed to parse file: {}", err)))?;
+    let edits = dioxus_autofmt::try_fmt_file(&contents, &parsed, indent)
+        .map_err(|err| Error::ParseError(format!("Failed to format file: {}", err)))?;
     let len = edits.len();
 
     if !edits.is_empty() {
@@ -313,29 +322,3 @@ async fn test_auto_fmt() {
 
     fmt.autoformat().unwrap();
 }
-
-/*#[test]
-fn spawn_properly() {
-    let out = Command::new("dioxus")
-        .args([
-            "fmt",
-            "-f",
-            r#"
-//
-
-rsx! {
-
-    div {}
-}
-
-//
-//
-//
-
-        "#,
-        ])
-        .output()
-        .expect("failed to execute process");
-
-    dbg!(out);
-}*/

+ 1 - 0
packages/extension/Cargo.toml

@@ -11,6 +11,7 @@ wasm-bindgen = { workspace = true }
 dioxus-autofmt = { workspace = true }
 rsx-rosetta = { workspace = true }
 html_parser = { workspace = true }
+syn ={ workspace = true }
 
 [lib]
 crate-type = ["cdylib", "rlib"]

+ 19 - 11
packages/extension/src/lib.rs

@@ -65,18 +65,26 @@ impl FormatBlockInstance {
 
 #[wasm_bindgen]
 pub fn format_file(contents: String, use_tabs: bool, indent_size: usize) -> FormatBlockInstance {
-    let _edits = dioxus_autofmt::fmt_file(
-        &contents,
-        IndentOptions::new(
-            if use_tabs {
-                IndentType::Tabs
-            } else {
-                IndentType::Spaces
-            },
-            indent_size,
-            false,
-        ),
+    // todo: use rustfmt for this instead
+    let options = IndentOptions::new(
+        if use_tabs {
+            IndentType::Tabs
+        } else {
+            IndentType::Spaces
+        },
+        indent_size,
+        false,
     );
+
+    let Ok(Ok(_edits)) = syn::parse_file(&contents)
+        .map(|file| dioxus_autofmt::try_fmt_file(&contents, &file, options))
+    else {
+        return FormatBlockInstance {
+            new: contents,
+            _edits: Vec::new(),
+        };
+    };
+
     let out = dioxus_autofmt::apply_formats(&contents, _edits.clone());
     FormatBlockInstance { new: out, _edits }
 }

+ 9 - 9
packages/extension/src/main.ts

@@ -65,9 +65,9 @@ function fmtSelection() {
 
 	// Select full lines of selection
 	let selection_range = new vscode.Range(
-		editor.selection.start.line, 
-		0, 
-		end_line, 
+		editor.selection.start.line,
+		0,
+		end_line,
 		editor.document.lineAt(end_line).range.end.character
 	);
 
@@ -83,9 +83,9 @@ function fmtSelection() {
 		end_line += 1;
 
 		selection_range = new vscode.Range(
-			editor.selection.start.line, 
-			0, 
-			end_line, 
+			editor.selection.start.line,
+			0,
+			end_line,
 			editor.document.lineAt(end_line).range.end.character
 		);
 
@@ -103,8 +103,8 @@ function fmtSelection() {
 
 	let lines_above = editor.document.getText(
 		new vscode.Range(
-			0, 
-			0, 
+			0,
+			0,
 			end_above,
 			editor.document.lineAt(end_above).range.end.character
 		)
@@ -115,7 +115,7 @@ function fmtSelection() {
 
 	try {
 		let formatted = dioxus.format_selection(unformatted, !editor.options.insertSpaces, tabSize, base_indentation);
-		for(let i = 0; i <= base_indentation; i++) {
+		for (let i = 0; i <= base_indentation; i++) {
 			formatted = (editor.options.insertSpaces ? " ".repeat(tabSize) : "\t") + formatted;
 		}
 		if (formatted.length > 0) {

+ 1 - 1
packages/rsx-rosetta/src/lib.rs

@@ -129,7 +129,7 @@ pub fn collect_svgs(children: &mut [BodyNode], out: &mut Vec<BodyNode>) {
                     diagnostics: Default::default(),
                     fields: vec![],
                     children: TemplateBody::new(vec![]),
-                    brace: Default::default(),
+                    brace: Some(Default::default()),
                     dyn_idx: Default::default(),
                     component_literal_dyn_idx: vec![],
                 });

+ 3 - 3
packages/rsx/src/component.rs

@@ -34,7 +34,7 @@ pub struct Component {
     pub fields: Vec<Attribute>,
     pub component_literal_dyn_idx: Vec<DynIdx>,
     pub spreads: Vec<Spread>,
-    pub brace: token::Brace,
+    pub brace: Option<token::Brace>,
     pub children: TemplateBody,
     pub dyn_idx: DynIdx,
     pub diagnostics: Diagnostics,
@@ -69,8 +69,8 @@ impl Parse for Component {
             name,
             generics,
             fields,
+            brace: Some(brace),
             component_literal_dyn_idx,
-            brace,
             spreads,
             diagnostics,
         };
@@ -308,7 +308,7 @@ impl Component {
         Component {
             name,
             generics,
-            brace: token::Brace::default(),
+            brace: None,
             fields: vec![],
             spreads: vec![],
             children: TemplateBody::new(vec![]),

+ 4 - 2
packages/rsx/src/forloop.rs

@@ -1,7 +1,7 @@
 use super::*;
 use location::DynIdx;
 use proc_macro2::TokenStream as TokenStream2;
-use syn::{braced, Expr, Pat};
+use syn::{braced, token::Brace, Expr, Pat};
 
 #[non_exhaustive]
 #[derive(PartialEq, Eq, Clone, Debug)]
@@ -10,6 +10,7 @@ pub struct ForLoop {
     pub pat: Pat,
     pub in_token: Token![in],
     pub expr: Box<Expr>,
+    pub brace: Brace,
     pub body: TemplateBody,
     pub dyn_idx: DynIdx,
 }
@@ -24,13 +25,14 @@ impl Parse for ForLoop {
         let expr = input.call(Expr::parse_without_eager_brace)?;
 
         let content;
-        let _brace = braced!(content in input);
+        let brace = braced!(content in input);
         let body = content.parse()?;
 
         Ok(Self {
             for_token,
             pat,
             in_token,
+            brace,
             expr: Box::new(expr),
             body,
             dyn_idx: DynIdx::default(),

+ 8 - 2
packages/rsx/src/ifchain.rs

@@ -4,6 +4,7 @@ use quote::quote;
 use quote::{ToTokens, TokenStreamExt};
 use syn::{
     parse::{Parse, ParseStream},
+    token::Brace,
     Expr, Result, Token,
 };
 
@@ -14,8 +15,10 @@ use crate::TemplateBody;
 pub struct IfChain {
     pub if_token: Token![if],
     pub cond: Box<Expr>,
+    pub then_brace: Brace,
     pub then_branch: TemplateBody,
     pub else_if_branch: Option<Box<IfChain>>,
+    pub else_brace: Option<Brace>,
     pub else_branch: Option<TemplateBody>,
     pub dyn_idx: DynIdx,
 }
@@ -42,10 +45,11 @@ impl Parse for IfChain {
         let cond = Box::new(input.call(Expr::parse_without_eager_brace)?);
 
         let content;
-        syn::braced!(content in input);
+        let then_brace = syn::braced!(content in input);
 
         let then_branch = content.parse()?;
 
+        let mut else_brace = None;
         let mut else_branch = None;
         let mut else_if_branch = None;
 
@@ -56,7 +60,7 @@ impl Parse for IfChain {
                 else_if_branch = Some(Box::new(input.parse::<IfChain>()?));
             } else {
                 let content;
-                syn::braced!(content in input);
+                else_brace = Some(syn::braced!(content in input));
                 else_branch = Some(content.parse()?);
             }
         }
@@ -67,6 +71,8 @@ impl Parse for IfChain {
             then_branch,
             else_if_branch,
             else_branch,
+            then_brace,
+            else_brace,
             dyn_idx: DynIdx::default(),
         })
     }

+ 1 - 7
packages/rsx/src/ifmt.rs

@@ -17,12 +17,6 @@ pub struct IfmtInput {
     pub segments: Vec<Segment>,
 }
 
-impl Default for IfmtInput {
-    fn default() -> Self {
-        Self::new(Span::call_site())
-    }
-}
-
 impl IfmtInput {
     pub fn new(span: Span) -> Self {
         Self {
@@ -213,7 +207,7 @@ impl ToTokens for IfmtInput {
     fn to_tokens(&self, tokens: &mut TokenStream) {
         // If the input is a string literal, we can just return it
         if let Some(static_str) = self.to_static() {
-            return static_str.to_tokens(tokens);
+            return quote_spanned! { self.span() => #static_str }.to_tokens(tokens);
         }
 
         // Try to turn it into a single _.to_string() call

+ 8 - 0
packages/rsx/src/literal.rs

@@ -151,6 +151,14 @@ pub struct HotReloadFormattedSegment {
     pub dynamic_node_indexes: Vec<DynIdx>,
 }
 
+impl HotReloadFormattedSegment {
+    /// This method is very important!
+    /// Deref + Spanned + .span() methods leads to name collisions
+    pub fn span(&self) -> Span {
+        self.formatted_input.span()
+    }
+}
+
 impl Deref for HotReloadFormattedSegment {
     type Target = IfmtInput;