dioxus icon indicating copy to clipboard operation
dioxus copied to clipboard

dx fmt removes comments in expressions

Open Coder2195 opened this issue 1 year ago • 3 comments

Problem

In

Steps To Reproduce

Steps to reproduce the behavior:

Make some rsx with non 4 tabs where your rustfmt.toml is set to ie 2 spaces below

div {
  class: "w-1/4",
  onclick: move |_| {
    todo!();
    todo!();
    todo!();
    // some comment
  },
  "Name"
}

You will get this

div {
  class: "w-1/4",
  onclick: move |_| {
      todo!();
      todo!();
      todo!();
  },
  "Name"
}

Expected behavior

Consistent tab spacing, and the retaining of comments.

Environment:

  • Dioxus version: 0.5.4
  • Rust version: 1.79.0
  • OS info: Ubuntu 24.04
  • App platform: any

Questionnaire

  • [x] I'm interested in fixing this myself but don't know where to start
  • [ ] I would like to fix and I have a solution
  • [ ] I don't have time to fix this right now, but maybe later

Coder2195 avatar Jun 15 '24 15:06 Coder2195

Well I'm gonna blame my not seeing this issue on the fact that you didn't mention indent/indentation anywhere in your issue so my single search didn't hit it 😛.

I dug into this issue in a bit more detail in #2537, the problem basically boils down to the fact that prettyplease forces 4 space formatting and dtolnay is uninterested in supporting alternative indentation styles.

I see 2 potential solutions, either have Dioxus or someone else maintain a fork of prettyplease which adds support for alternative indentation styles, or some relatively ugly hacks around unparse_expr, specifically with regards to handling multiline strings and potentially other indentation edge cases.

I'm not sure what direction Dioxus would prefer to go with this, I'd be willing to take a look at writing some hacky conversion code which takes the output from prettyplease and tries to reformat it correctly, I think it would be possible to have accurate results with enough tests. From a relatively quick look, prettyplease shouldn't be too bad to adjust, but it would require some API changes and the maintenance of that fork (which I am personally uninterested in).

Enduriel avatar Jun 17 '24 15:06 Enduriel

There is an issue about dx fmt eating comments here: https://github.com/DioxusLabs/dioxus/issues/2341 And an older issue about using the rustfmt config here: https://github.com/DioxusLabs/dioxus/issues/1492

Dioxus doesn't use rustfmt, so the rustfmt settings don't apply to dx format. We use prettyplease which doesn't support rustfmt compatibility. If we want rustfmt compatibility, using rustfmt directly is probably the easiest way to support that. Otherwise, we can just choose an indention style and apply it consistently

I think most people will be using rustfmt for most of their code so we should use it for dx fmt as well. Ideally we could create/use a library binding for rustfmt. If we can't, I still think it would be better to use rustfmt through the CLI even if it is slightly more fragile and slower

ealmloff avatar Jun 17 '24 16:06 ealmloff

@ealmloff I actually agree, I do think going with rustfmt is the most hollistic solution, I just wasn't that was feasible without going through the CLI which I didn't think dioxus would be interested in. Maybe all the other issues should be closed and the original #1492 should link to the relevant code so if someone is interested in fixing it they would know where to start.

Enduriel avatar Jun 17 '24 16:06 Enduriel