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 +1 -3
1 changed files with 1 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit ebad512a03 - Show all commits

4
app-proto/src/components/diagnostics.rs
View file

@ -16,9 +16,7 @@ struct DiagnosticsState {
impl DiagnosticsState {
fn new(initial_tab: String) -> DiagnosticsState {
DiagnosticsState {
active_tab: create_signal(initial_tab),
}
DiagnosticsState { active_tab: create_signal(initial_tab) }
glen marked this conversation as resolved Outdated
glen commented 2025-08-01 21:45:51 +00:00
Outdated
Copy link

As a mostly uninformed question about Rust syntax, why is/must DiagnosticsState mentioned four times in quick succession here? The first is to declare a new data type, so is of course unavoidable. I think the second is to tie some methods to that datatype; is there no more compact syntax for doing so when you immediately want to tie some methods, as is very common? Is the third a type annotation and the fourth a constructor? If so, is there no way for Rust to simply infer the return type from the explicit type of the return value in the body of new? (If I recall, new has no special status in Rust, so we can't hope Rust will just know its return type should be the struct being impled. But the type is patent from the only exit from the function body...)

It would be great to reduce this redundancy; of course we may ultimately just have to introduce some mechanisms for it in Husht, if there's little to be done within Rust...

As a mostly uninformed question about Rust syntax, why is/must DiagnosticsState mentioned _four_ times in quick succession here? The first is to declare a new data type, so is of course unavoidable. I think the second is to tie some methods to that datatype; is there no more compact syntax for doing so when you immediately want to tie some methods, as is very common? Is the third a type annotation and the fourth a constructor? If so, is there no way for Rust to simply infer the return type from the explicit type of the return value in the body of `new`? (If I recall, `new` has no special status in Rust, so we can't hope Rust will just know its return type should be the struct being `impl`ed. But the type is patent from the only exit from the function body...) It would be great to reduce this redundancy; of course we may ultimately just have to introduce some mechanisms for it in Husht, if there's little to be done within Rust...
Vectornaut commented 2025-08-02 09:23:08 +00:00
Outdated
Copy link

I think the second is to tie some methods to that datatype; is there no more compact syntax for doing so when you immediately want to tie some methods, as is very common?

I don't know of any more compact syntax. If there were one, it would probably be mentioned here. I think the current syntax is quite usable and readable. I also think there's an advantage to having only one syntax for an implementation block: you can find all of the implementation blocks for a non-parametric structure Sandwich by skimming or searching for blocks of the forms shown here:

impl Sandwich
impl Sandwich</* concrete type */>
impl</* generic type: perhaps with bounds */> Sandwich</* generic type */>

Is the third a type annotation and the fourth a constructor?

Yes.

If so, is there no way for Rust to simply infer the return type from the explicit type of the return value in the body of new?

I don't think so. The thing that comes before the braces in a struct expression is something called a path in expression, which looks like it's supposed to be a specific type specifier like Sandwich or snack::Sandwich or super::snack::Sandwich or Sandwich<Cucumber>.

The code below shows that Rust can infer type parameters in a struct expression, but I don't think there's syntax to request inference for the whole type.

struct NamedValue<T> {
    name: String,
    value: T,
}

fn the_answer() -> NamedValue<i32> {
    NamedValue {
        name: "The answer".to_string(),
        value: 42
    }
}

fn main() {
    let NamedValue { name, value } = the_answer();
    print!("{name} is {value}");
}

I guess it could be reasonable to use _ as a placeholder for inferred types:

let _ { name, value } = the_answer();

After all, it can already be used as a placeholder for inferred type parameters in expressions like this:

let list: Vec<_> = [4, 6, 8, 12, 20]
    .into_iter()
    .map(|n| format!("{n}-hedron"))
    .collect();

(In this example, the compiler can't infer what type of collection we want, because collect<B> is a generic function that can return collections of various types. Once we tell the compiler that we want a Vec, though, we can ask it to infer the type parameter that specifies the element type.)

> I think the second is to tie some methods to that datatype; is there no more compact syntax for doing so when you immediately want to tie some methods, as is very common? I don't know of any more compact syntax. If there were one, it would probably be mentioned [here](https://doc.rust-lang.org/stable/book/ch05-03-method-syntax.html). I think the current syntax is quite usable and readable. I also think there's an advantage to having only one syntax for an implementation block: you can find all of the implementation blocks for a non-parametric structure `Sandwich` by skimming or searching for blocks of the forms shown [here](https://doc.rust-lang.org/rust-by-example/generics/impl.html): ```rust impl Sandwich impl Sandwich</* concrete type */> impl</* generic type: perhaps with bounds */> Sandwich</* generic type */> ``` > Is the third a type annotation and the fourth a constructor? Yes. > If so, is there no way for Rust to simply infer the return type from the explicit type of the return value in the body of new? I don't think so. The thing that comes before the braces in a [struct expression](https://doc.rust-lang.org/reference/expressions/struct-expr.html) is something called a [*path in expression*](https://doc.rust-lang.org/reference/paths.html#paths-in-expressions), which looks like it's supposed to be a specific type specifier like `Sandwich` or `snack::Sandwich` or `super::snack::Sandwich` or `Sandwich<Cucumber>`. The code below shows that Rust can infer type parameters in a struct expression, but I don't think there's syntax to request inference for the whole type. ```rust struct NamedValue<T> { name: String, value: T, } fn the_answer() -> NamedValue<i32> { NamedValue { name: "The answer".to_string(), value: 42 } } fn main() { let NamedValue { name, value } = the_answer(); print!("{name} is {value}"); } ``` I guess it could be reasonable to use `_` as a placeholder for inferred types: ```rust let _ { name, value } = the_answer(); ``` After all, it can already be used as a placeholder for inferred type parameters in expressions like this: ```rust let list: Vec<_> = [4, 6, 8, 12, 20] .into_iter() .map(|n| format!("{n}-hedron")) .collect(); ``` (In this example, the compiler can't infer what type of collection we want, because [`collect<B>`](https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.collect) is a generic function that can return collections of various types. Once we tell the compiler that we want a `Vec`, though, we can ask it to infer the type parameter that specifies the element type.)
glen commented 2025-08-02 16:39:48 +00:00
Outdated
Copy link

I guess it could be reasonable to use _ as a placeholder for inferred types:

let _ { name, value } = the_answer();

This isn't actually allowed syntax, correct? Just something one might plausibly implement in Husht, say?

On the page you link, it does mention that in an impl block, Self is an abbreviation for the type being impled. So that suggests we could write

struct DiagnosticsState {
    active_tab: Signal<String>,
}

impl DiagnosticsState {
    fn new(initial_tab: String) -> Self {
        Self { active_tab: create_signal(initial_tab) }
    }
}

What do you think? I'm just struggling with the virtue of design decisions on the part of Rust that would lead to the type name being written out four time in such a short bit of code. I don't see anything misleading or ambiguous about syntax like:

class DiagnosticsState  // transform to a struct declaration
   active_tab: Signal<String>

   fn new(initial_tab: String)  // fns in a class automatically get put in a impl block; return type inferred
      DiagnosticsState { active_tab: create_signal(initial_tab) } // or Self { ... }

as a possible Husht rendering of this. Anyhow, seems like the only action item to consider here is whether to use Self as an abbreviation, presuming it works fine. Seems pretty sensible for a "new" function, at least.

> I guess it could be reasonable to use _ as a placeholder for inferred types: > `let _ { name, value } = the_answer();` This isn't actually allowed syntax, correct? Just something one might plausibly implement in Husht, say? On the page you link, it does mention that in an `impl` block, `Self` is an abbreviation for the type being impled. So that suggests we could write ``` struct DiagnosticsState { active_tab: Signal<String>, } impl DiagnosticsState { fn new(initial_tab: String) -> Self { Self { active_tab: create_signal(initial_tab) } } } ``` What do you think? I'm just struggling with the virtue of design decisions on the part of Rust that would lead to the type name being written out four time in such a short bit of code. I don't see anything misleading or ambiguous about syntax like: ``` class DiagnosticsState // transform to a struct declaration active_tab: Signal<String> fn new(initial_tab: String) // fns in a class automatically get put in a impl block; return type inferred DiagnosticsState { active_tab: create_signal(initial_tab) } // or Self { ... } ``` as a possible Husht rendering of this. Anyhow, seems like the only action item to consider here is whether to use Self as an abbreviation, presuming it works fine. Seems pretty sensible for a "new" function, at least.
Vectornaut commented 2025-08-04 03:04:28 +00:00
Outdated
Copy link

On the page you link, it does mention that in an impl block, Self is an abbreviation for the type being impled. So that suggests we could write […]. What do you think?

Oh, yes, we could do that. I mostly see Self used in implementations of traits, like Clone and From, but nothing would stop us from using it in the basic implementation of a structure. This style seems to be used to some extent in the source code for the Rust standard library and for nalgebra. For example:

  • It's used for the return type, but not the returned structure, in Vec::new.
  • It's used for the return type, but not the returned structure, in ColPivQR::new from nalgebra.
  • It's not used at all in BTreeSet::new_in—which is newly added code, and therefore might reflect current conventions.

The main downside I see to this style is that it can make the source harder to navigate, because it's context-dependent.

If we use this style, I think we should use it consistently for every structure. Let me know if you'd like me to proceed with reformatting all our structure implementations in this way.

I guess it could be reasonable to use _ as a placeholder for inferred types:

This isn't actually allowed syntax, correct? Just something one might plausibly implement in Husht, say?

Correct.

> On the page you link, it does mention that in an impl block, Self is an abbreviation for the type being impled. So that suggests we could write […]. What do you think? Oh, yes, we could do that. I mostly see `Self` used in implementations of traits, like `Clone` and `From`, but nothing would stop us from using it in the basic implementation of a structure. This style seems to be used to some extent in the source code for the Rust standard library and for nalgebra. For example: - It's used for the return type, but not the returned structure, in [`Vec::new`](https://doc.rust-lang.org/src/alloc/vec/mod.rs.html#434). - It's used for the return type, but not the returned structure, in [`ColPivQR::new`](https://docs.rs/nalgebra/latest/src/nalgebra/linalg/col_piv_qr.rs.html#59-92`) from nalgebra. - It's not used at all in [`BTreeSet::new_in`](https://doc.rust-lang.org/src/alloc/collections/btree/set.rs.html#364)—which is newly added code, and therefore might reflect current conventions. The main downside I see to this style is that it can make the source harder to navigate, because it's context-dependent. If we use this style, I think we should use it consistently for every structure. Let me know if you'd like me to proceed with reformatting all our structure implementations in this way. >> I guess it could be reasonable to use _ as a placeholder for inferred types: > > This isn't actually allowed syntax, correct? Just something one might plausibly implement in Husht, say? Correct.
glen commented 2025-08-04 09:03:33 +00:00
Outdated
Copy link

Well, I like the reduced rendundancy of Self, especially for long struct names. So if you're comfortable with it, yes please let's use that abbreviation, either in this PR or the next as seems better to you.

Well, I like the reduced rendundancy of Self, especially for long struct names. So if you're comfortable with it, yes please let's use that abbreviation, either in this PR or the next as seems better to you.
Vectornaut commented 2025-08-04 17:27:11 +00:00
Outdated
Copy link

Let's do it in the next PR, along with renaming the test assembly loader functions. I think those two mass name changes are more similar to each other than to the changes this PR was intended for.

Let's do it in the next PR, along with renaming the test assembly loader functions. I think those two mass name changes are more similar to each other than to the changes this PR was intended for.
glen commented 2025-08-04 21:25:09 +00:00
Outdated
Copy link

OK, again, please put it in whatever new issue you made/are making for the function name changes. Resolving, and will merge this PR as soon as I have a moment to run-test it.

OK, again, please put it in whatever new issue you made/are making for the function name changes. Resolving, and will merge this PR as soon as I have a moment to run-test it.
Vectornaut commented 2025-08-04 21:55:06 +00:00
Outdated
Copy link

This is now issue #110. (To help with organization, I opened a separate issue instead of mashing it together with #109).

This is now issue #110. (To help with organization, I opened a separate issue instead of mashing it together with #109).
}
}
glen marked this conversation as resolved
glen commented 2025-08-01 21:48:24 +00:00
Copy link

As a mild convention, should we prefer things like

DiagnosticsState {active_tab: create_signal(initial_tab)}

when they easily fit on one line? As you may recall, I am averse to typographical conventions that burn vertical lines of code with no content, because I find allowing more code semantics to fit on the screen at once to be an aid to code comprehension, and it is vertical real estate that is at a premium...

p.s. I see now that in fact you have re-single-lined several such constructors in other files, so it seems to me this one should be as well, right?

As a mild convention, should we prefer things like ``` DiagnosticsState {active_tab: create_signal(initial_tab)} ``` when they easily fit on one line? As you may recall, I am averse to typographical conventions that burn vertical lines of code with no content, because I find allowing more code semantics to fit on the screen at once to be an aid to code comprehension, and it is vertical real estate that is at a premium... p.s. I see now that in fact you have re-single-lined several such constructors in other files, so it seems to me this one should be as well, right?
Vectornaut commented 2025-08-02 05:42:17 +00:00
Copy link

As you may recall, I am averse to typographical conventions that burn vertical lines of code with no content, because I find allowing more code semantics to fit on the screen at once to be an aid to code comprehension, and it is vertical real estate that is at a premium...

Oh, yeah, I remember you mentioning that. I've collapsed the structure expression onto one line in commit ebad512. I had put active_tab: /* ... */ on its own line to make the create_signal call more readable, but it's fine if that's not worth the cost in vertical space.

> As you may recall, I am averse to typographical conventions that burn vertical lines of code with no content, because I find allowing more code semantics to fit on the screen at once to be an aid to code comprehension, and it is vertical real estate that is at a premium... Oh, yeah, I remember you mentioning that. I've collapsed the structure expression onto one line in commit ebad512. I had put `active_tab: /* ... */` on its own line to make the `create_signal` call more readable, but it's fine if that's not worth the cost in vertical space.