Vectornaut
  • Joined on 2019-09-28
Vectornaut commented on pull request StudioInfinity/dyna3#108 2025-08-04 21:45:42 +00:00
Add trailing commas and clean up formatting

This is now issue #109.

Vectornaut opened issue StudioInfinity/dyna3#109 2025-08-04 21:45:19 +00:00
Drop the _assemb suffixes from test assembly loaders
Vectornaut commented on pull request StudioInfinity/dyna3#108 2025-08-04 17:27:11 +00:00
Add trailing commas and clean up formatting

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.

Vectornaut commented on pull request StudioInfinity/dyna3#108 2025-08-04 03:20:45 +00:00
Add trailing commas and clean up formatting

Okay, let's do that in the next pull request.

Vectornaut commented on pull request StudioInfinity/dyna3#108 2025-08-04 03:19:43 +00:00
Add trailing commas and clean up formatting

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/c

Vectornaut commented on pull request StudioInfinity/dyna3#108 2025-08-04 03:11:51 +00:00
Add trailing commas and clean up formatting

Done in commit e67a658.

Vectornaut pushed to trailing-commas at Vectornaut/dyna3 2025-08-04 03:09:58 +00:00
e67a658e00 Group the parameters of the scene push methods
Vectornaut commented on pull request StudioInfinity/dyna3#108 2025-08-04 03:04:28 +00:00
Add trailing commas and clean up formatting

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…

Vectornaut commented on pull request StudioInfinity/dyna3#108 2025-08-02 09:23:08 +00:00
Add trailing commas and clean up formatting

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…

Vectornaut commented on pull request StudioInfinity/dyna3#108 2025-08-02 08:18:47 +00:00
Add trailing commas and clean up formatting

That sounds fine. I've suggested some groupings for SceneSpheres::push and ScenePoints::push. Are there any…

Vectornaut commented on pull request StudioInfinity/dyna3#108 2025-08-02 08:14:46 +00:00
Add trailing commas and clean up formatting

why the semicolon now

The semicolon helps communicate the intent of the code, as explained below. I'm adding it now because I didn't notice it was missing until now, when I ran Rustfmt in…

Vectornaut commented on pull request StudioInfinity/dyna3#108 2025-08-02 07:25:45 +00:00
Add trailing commas and clean up formatting

What do you think?

I think that commas on every arm (as we now have) are much more readable than braces on…

Vectornaut commented on pull request StudioInfinity/dyna3#108 2025-08-02 07:22:44 +00:00
Add trailing commas and clean up formatting

elide the _assemb part, and quite possibly all the common load_ parts, of these function names altogether

I agree that dropping the _assemb suffix makes sense now that we've split the…

Vectornaut commented on pull request StudioInfinity/dyna3#108 2025-08-02 07:18:29 +00:00
Add trailing commas and clean up formatting

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…

Vectornaut pushed to trailing-commas at Vectornaut/dyna3 2025-08-02 07:16:07 +00:00
b02e682e15 Add space around = for Sycamore props
Vectornaut commented on pull request StudioInfinity/dyna3#108 2025-08-02 06:28:02 +00:00
Add trailing commas and clean up formatting

but also see a later comment that another way to go at the moment might be to go ahead and group some of these option arguments onto a single line

Sure. Does either of the following groupings…

Vectornaut commented on pull request StudioInfinity/dyna3#108 2025-08-02 06:20:31 +00:00
Add trailing commas and clean up formatting

Yes, I think that revising the SceneSpheres and ScenePoints structures at some point would be a good idea.

But perhaps this PR is not the time to do it?

I agree. Like I said in the…

Vectornaut commented on pull request StudioInfinity/dyna3#108 2025-08-02 06:17:47 +00:00
Add trailing commas and clean up formatting

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,…

Vectornaut commented on pull request StudioInfinity/dyna3#108 2025-08-02 06:04:53 +00:00
Add trailing commas and clean up formatting

If the point is that a comma is required when the branch is an expression without braces and optional when the branch is enclosed in braces […]

Yes, that's the point.

[…] then I…

Vectornaut pushed to trailing-commas at Vectornaut/dyna3 2025-08-02 06:03:33 +00:00
bfd5d8e35f Add commas after match arms wrapped in blocks