StudioInfinity/dyna3
2
0
Fork 2
Code Issues 57 Pull requests 1 Projects 3 Releases 4 Wiki Activity Actions

Add trailing commas and clean up formatting #108

Merged
glen merged 5 commits from Vectornaut/dyna3:trailing-commas into main 2025-08-04 23:34:35 +00:00
Conversation 3 Commits 5 Files changed 12 +14 -14
4 changed files with 14 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit bfd5d8e35f - Show all commits

8
app-proto/src/assembly.rs
View file

@ -717,14 +717,14 @@ impl Assembly {
// save the tangent space
self.tangent.set_silent(tangent);
}
},
Err(message) => {
// report the realization status. the `Err(message)` we're
// setting the status to has a different type than the
// `Err(message)` we received from the match: we're changing the
// `Ok` type from `Realization` to `()`
self.realization_status.set(Err(message))
}
},
}
}
@ -804,10 +804,10 @@ impl Assembly {
// restore its normalization
*rep += motion_proj.column(column_index);
elt.project_to_normalized(rep);
}
},
None => {
console_log!("No velocity to unpack for fresh element \"{}\"", elt.id())
}
},
};
});
}

8
app-proto/src/components/display.rs
View file

@ -910,17 +910,17 @@ pub fn Display() -> View {
if depth < best_depth {
clicked = Some((elt, depth))
}
}
},
None => clicked = Some((elt, depth)),
}
None => ()
},
None => (),
};
}
// if we clicked something, select it
match clicked {
Some((elt, _)) => state.select(&elt, event.shift_key()),
None => state.selection.update(|sel| sel.clear())
None => state.selection.update(|sel| sel.clear()),
};
},
)
glen marked this conversation as resolved Outdated
glen commented 2025-08-01 22:11:56 +00:00
Outdated
Copy link

this function that just got comma-terminated is the value of a keyword argument onclick:move=fn ... (unchanged in this PR). While we are doing formatting, is it legal in keyword arguments to have whitespace: onclick:move = fn ... ? If so, please also adopt such a convention, as it is far more readable.

If not, I think the syntax design here really exposes the hypocrisy of the sanctimonious "oh whitespace-sensitive languages are so terrible and dangerous etc." because here we have a whitespace sensitivity that makes the programs less easily visually processed (and violate usual rules of punctuation, which include whitespace around =).

I assume the state of affairs is the latter, maybe because a keyword argument needs to be distinguished from an in-line assignment, but using the lack of whitespace to do so when (I think) in other contexts an assignment could elide the whitespace (or if not, why can you write a+1 but not a=1)? But these fine points of whitespace-based semantics, at the cost of readability, seem nutty to me. Hopefully we can do better in Husht...

this function that just got comma-terminated is the value of a keyword argument `onclick:move=fn ...` (unchanged in this PR). While we are doing formatting, is it legal in keyword arguments to have whitespace: `onclick:move = fn ...` ? If so, please also adopt such a convention, as it is far more readable. If not, I think the syntax design here really exposes the hypocrisy of the sanctimonious "oh whitespace-sensitive languages are so terrible and dangerous etc." because here we have a whitespace sensitivity that makes the programs less easily visually processed (and violate usual rules of punctuation, which include whitespace around =). I assume the state of affairs is the latter, maybe because a keyword argument needs to be distinguished from an in-line assignment, but using the lack of whitespace to do so when (I think) in other contexts an assignment _could_ elide the whitespace (or if not, why can you write `a+1` but not `a=1`)? But these fine points of whitespace-based semantics, at the cost of readability, seem nutty to me. Hopefully we can do better in Husht...
Vectornaut commented 2025-08-02 07:18:29 +00:00
Outdated
Copy link

is it legal in keyword arguments to have whitespace

For clarity: Rust does not currently have keyword arguments. The code you're looking at is inside a Sycamore view! macro, which provides its own special synatx for laying out DOM nodes.

Sycamore props (as these node keyword arguments are called) can indeed have whitespace around the =. As you noted, this is consistent with Rust's general insensitivity to whitespace.

Sycamore's lead developer consistently writes props without space around the =, and the Sycamore community seems to broadly agree on this style, which is why I adopted it. This style seems widely used in languages that do have keyword arguments: for example, it's part of the Python style guide. I'm fine with using a different style, though, so I've added space around the = in commit b02e682.

> is it legal in keyword arguments to have whitespace For clarity: Rust does not [currently](https://github.com/rust-lang/rfcs/issues/323) have keyword arguments. The code you're looking at is inside a Sycamore [`view!`](https://sycamore.dev/book/introduction/your-first-app#creating-views) macro, which provides its own special synatx for laying out DOM nodes. Sycamore [*props*](https://sycamore.dev/book/introduction/your-first-app#component-props) (as these node keyword arguments are called) can indeed have whitespace around the `=`. As you noted, this is consistent with Rust's general insensitivity to whitespace. Sycamore's lead developer consistently writes props without space around the `=`, and the Sycamore community seems to broadly agree on this style, which is why I adopted it. This style seems widely used in languages that do have keyword arguments: for example, it's part of the [Python style guide](https://peps.python.org/pep-0008/#other-recommendations). I'm fine with using a different style, though, so I've added space around the `=` in commit b02e682.
glen commented 2025-08-02 23:51:52 +00:00
Outdated
Copy link

Thanks for the clarification, and sounds good. I actually agree that in brief single-line calls,

foo(bar, key1=baz, key2=quux)

does actually look/read a bit better than

foo(bar, key1 = baz, key2 = quux)

but that when the value for the key is multi-line, the readability switches very strongly the other way. So I am fine with either always spacing them (if you prefer that consistency) or squeezing the spaces on single-line calls and leaving them in on multi-line calls -- whichever of these options you prefer.

Thanks for the clarification, and sounds good. I actually agree that in brief single-line calls, ``` foo(bar, key1=baz, key2=quux) ``` does actually look/read a bit better than ``` foo(bar, key1 = baz, key2 = quux) ``` but that when the value for the key is multi-line, the readability switches very strongly the other way. So I am fine with either always spacing them (if you prefer that consistency) or squeezing the spaces on single-line calls and leaving them in on multi-line calls -- whichever of these options you prefer.
Vectornaut commented 2025-08-04 03:19:43 +00:00
Outdated
Copy link

Spacing around the = in single-line calls looks okay to me, and it's consistent with typical Rust style for things like the cfg attribute, so let's stick with the formatting from commit b02e682.

Spacing around the `=` in single-line calls looks okay to me, and it's consistent with typical Rust style for things like the [`cfg` attribute](https://doc.rust-lang.org/rust-by-example/attribute/cfg.html), so let's stick with the formatting from commit b02e682.

10
app-proto/src/components/outline.rs
View file

@ -67,8 +67,8 @@ fn RegulatorInput(regulator: Rc<dyn Regulator>) -> View {
Ok(set_pt) => {
set_point.set(set_pt);
true
}
Err(_) => false
},
Err(_) => false,
}
)
},
@ -162,19 +162,19 @@ fn ElementOutlineItem(element: Rc<dyn Element>) -> View {
"Enter" => {
state.select(&element_for_handler, event.shift_key());
event.prevent_default();
}
},
"ArrowRight" if regulated.get() => {
let _ = details_node
.get()
.unchecked_into::<web_sys::Element>()
.set_attribute("open", "");
}
},
"ArrowLeft" => {
let _ = details_node
.get()
.unchecked_into::<web_sys::Element>()
.remove_attribute("open");
}
},
_ => (),
glen marked this conversation as resolved
glen commented 2025-08-01 22:15:39 +00:00
Copy link

this new comma looks particularly nutty when all of the other commas are being removed. Can we just leave it out, especially given that a null default return will presumably be forever the final branch here? Indeed, isn't the case that there could never be subsequent code here that would have any runtime effect, because the _ branch would always fire, cutting off any chance that subsequent branches would? This latter consideration obviates the need for the comma to avoid the "spurious git diff" phenomenon, since no useful code could ever be added subsequent to this branch. So please leave out this comma, thanks.

this new comma looks particularly nutty when all of the other commas are being removed. Can we just leave it out, especially given that a null default return will presumably be forever the final branch here? Indeed, isn't the case that there could never be subsequent code here that would have any runtime effect, because the `_` branch would always fire, cutting off any chance that subsequent branches would? This latter consideration obviates the need for the comma to avoid the "spurious git diff" phenomenon, since no useful code could ever be added subsequent to this branch. So please leave out this comma, thanks.
Vectornaut commented 2025-08-02 06:17:47 +00:00
Copy link

Now that we've switched to using commas after match arms wrapped in blocks, should I still remove the commas after catch-all match arms? That would make them the only match arms without commas, which makes them look weird to me.

Now that we've [switched](https://code.studioinfinity.org/StudioInfinity/dyna3/pulls/108#issuecomment-3152) to using commas after match arms wrapped in blocks, should I still remove the commas after catch-all match arms? That would make them the only match arms without commas, which makes them look weird to me.
glen commented 2025-08-02 23:52:41 +00:00
Copy link

Yes, agreed, if we are always using commas, they should be here too. Resolving.

Yes, agreed, if we are always using commas, they should be here too. Resolving.
}
}

2
app-proto/src/engine.rs
View file

@ -474,7 +474,7 @@ pub fn realize_gram(
None => return Realization {
result: Err("Cholesky decomposition failed".to_string()),
history,
}
},
};
let base_step_stacked = hess_cholesky.solve(&neg_grad_stacked);
let base_step = base_step_stacked.reshape_generic(Dyn(element_dim), Dyn(assembly_dim));