Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dx fmt deletes comments #2341

Open
2 of 3 tasks
magikmw opened this issue Apr 19, 2024 · 8 comments
Open
2 of 3 tasks

dx fmt deletes comments #2341

magikmw opened this issue Apr 19, 2024 · 8 comments
Labels
autoformatting Related to the autofmt crate bug Something isn't working

Comments

@magikmw
Copy link

magikmw commented Apr 19, 2024

Problem

Using dx fmt on source files is deleting comments, both in rsx macros and standard rust when using --all-code.
This is especially egregious because standard rustfmt formatter is really messing up any files with rsx in them, so dx fmt is the only viable alternative.

Steps To Reproduce

Steps to reproduce the behavior:

  • Put any comments into a file - either inside rsx or elsewhere
  • Run dx fmt -f yourfile.rs
  • Comments deleted

Expected behavior

Comments should be left alone.
I'm not entirely sure how rustfmt does it, but with my limited experience with it, it seems to keep comments intact and relatively in the relevelant place after formatting.

Screenshots

n/a

Environment:

  • Dioxus version: 0.5.1
  • Rust version: rustc 1.77.2 (25ef9e3d8 2024-04-09)
  • OS info: Fedora 40 beta
  • App platform: n/a (but I'm making a fullstack app)

Questionnaire

  • 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
@DogeDark
Copy link
Member

I am unable to reproduce this using the git version of the CLI. Can you test the git version of the CLI on your end?

You can install the git version with:

cargo install --git https://github.com/DioxusLabs/dioxus dioxus-cli

@DogeDark DogeDark added bug Something isn't working autoformatting Related to the autofmt crate labels Apr 20, 2024
@magikmw
Copy link
Author

magikmw commented Apr 22, 2024

I did run the command to install git version and checked again:

-                // TODO: Implement event handling here -> listen to eve
nts in child
                 let accept_keys = [Key::Enter, Key::Character(" ".to_string())];
-                if accept_keys.contains(&evt.key()){
+                if accept_keys.contains(&evt.key()) {
                     log::info!("{} pressed in {}", evt.key(), "{props.labelText}");
                     log::info!("{} pressed in {}", evt.key(), "{props.labelText}");
                 }
             },
             class: "flex flex-1 h-full w-3/4 items-center justify-center btn btn-outline my-1 py-1",
-            div { class: "flex-none py-1", // TODO: Refactor into separate component
+            div { class: "flex-none py-1",
                 Icon { width: 24, height: 24, icon: props.icon, title: "{props.iconTitle}" }
-            },
+            }
             "{props.labelText}"
-            input { // TODO: Refactor into separate component
+            input {

dx -V says:

dioxus 0.5.2

@DogeDark
Copy link
Member

I can reproduce it now. The formatter doesn't like inline comments and removes them completely:

rsx! {
    div {   // this comment will be removed
        "hi",
    }
}

@spookyvision
Copy link
Contributor

this also seems to affect the VSCode Dioxus extension. Block-level comments (/* .. */) are also affected; doc comments (///) however are not.

@DogeDark
Copy link
Member

DogeDark commented May 2, 2024

Adding to the list.

All comments are removed in event handlers (except doc comments):

rsx! {
    div {
        // also occurs in move || async move
        onclick: move |_| {
            // is removed
            /* is removed */
        }
    }
}

@OlivierLemoine
Copy link

I think it comes from here:

fn format_rust(input: &str) -> Result<String> {
let syntax_tree = syn::parse_file(input).map_err(format_syn_error)?;
let output = prettyplease::unparse(&syntax_tree);
Ok(output)
}

syn does not keep comments.

https://crates.io/crates/rustfmt-wrapper (or spawning rustfmt directly) could be used instead.
This won't help in non-rust code, but I think it's a start.

I don't know if Dioxus developers would agree with this solution, but if they do, I would be happy to do it.

@jkelleyrtp
Copy link
Member

We try our best to preserve comments by writing out whitepace on rsx indents, but there's no way to preserve whitespace with prettyplease. I'd be interested in supporting the rustfmt wrapper approach, but it is convenient for autoformatting to not rely on rustfmt so we can use it in things like wasm.

@OlivierLemoine
Copy link

What if it was behind a target cfg? Wasi support spawning new tasks, so rustfmt would work.
But it's true it's not an ideal solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autoformatting Related to the autofmt crate bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants