Vectornaut
  • Joined on 2019-09-28
Vectornaut commented on pull request StudioInfinity/dyna3#118 2025-10-10 23:05:52 +00:00
feat: Point coordinate regulators

I can revert all these changes, and do a PR after the whitespace one fixing all the 80-character violations in the whole code base. Ok?

Making a later pull request to impose a consistent line…

Vectornaut commented on pull request StudioInfinity/dyna3#118 2025-10-10 22:49:25 +00:00
feat: Point coordinate regulators

A much stronger convention, universal among all projects I have ever worked on, is no trailing whitespace (on any line).

This is the convention the [Rust style guide](https://doc.rust-lang.org

Vectornaut commented on pull request StudioInfinity/dyna3#118 2025-10-10 22:43:51 +00:00
feat: Point coordinate regulators

To me, the placement of the new helper is kind of weird to me. It's right in the middle of the Sphere definition and implementation section, which makes it seem associated with Sphere, when in…

Vectornaut commented on pull request StudioInfinity/dyna3#118 2025-10-10 22:40:11 +00:00
feat: Point coordinate regulators

The helper is useful, and I like the more informative error messages! To help make it clear how the helper is supposed to be used, I might make some or all of the following changes.

  • Add more…
Vectornaut commented on pull request StudioInfinity/dyna3#118 2025-10-10 22:24:32 +00:00
feat: Point coordinate regulators

I'll trust your experience with installing Cargo tools at custom locations, since I've never tried it!

Vectornaut commented on pull request StudioInfinity/dyna3#118 2025-10-10 22:24:32 +00:00
feat: Point coordinate regulators

The information here looks accurate to me, although I've never installed a global tool with the --locked flag, and I don't know whether it's recommended.

Vectornaut suggested changes for StudioInfinity/dyna3#118 2025-10-10 22:24:32 +00:00
feat: Point coordinate regulators
Vectornaut commented on pull request StudioInfinity/dyna3#118 2025-10-10 22:15:44 +00:00
feat: Point coordinate regulators

@glen wrote in StudioInfinity/dyna3#118 (comment):

It took me a while to figure out how to deal with it, so I made a couple small changes in the…

Vectornaut commented on pull request StudioInfinity/dyna3#118 2025-10-10 22:07:16 +00:00
feat: Point coordinate regulators

Thanks for reflowing and rewording the comment!

Actually there are many 80-character line violations; again, I didn't realize. I will fix the ones in this source file, and we can fix other…

Vectornaut commented on pull request StudioInfinity/dyna3#118 2025-10-10 06:41:34 +00:00
feat: Point coordinate regulators

Whoops, here's a formatting mistake from the revision I made. I should've just written

Vectornaut suggested changes for StudioInfinity/dyna3#118 2025-10-10 06:41:34 +00:00
feat: Point coordinate regulators
Vectornaut commented on pull request StudioInfinity/dyna3#118 2025-10-10 06:28:05 +00:00
feat: Point coordinate regulators

I'd be fine with leaving it out of this pull request. I made it because this pull request implements Display for Axis using the terser fmt::Result convention, and I wanted the previous…

Vectornaut commented on pull request StudioInfinity/dyna3#118 2025-10-10 06:21:26 +00:00
feat: Point coordinate regulators

Our current convention is that all the lines in a block, including blank lines, should have the block indentation. Under that convention, this blank line would need to be indented by four spaces, just like the lines above and below it and the other blank lines within the same impl block.

Vectornaut commented on pull request StudioInfinity/dyna3#118 2025-10-10 06:21:26 +00:00
feat: Point coordinate regulators

I've been wrapping comments at 80 characters throughout the code. Under that convention, this comment would need to be reflowed.

Vectornaut commented on pull request StudioInfinity/dyna3#118 2025-10-10 06:21:26 +00:00
feat: Point coordinate regulators

For consistency with InversiveDistanceRegulator, I'd change the panic message to:

Vectornaut suggested changes for StudioInfinity/dyna3#118 2025-10-10 06:21:26 +00:00
feat: Point coordinate regulators

Here are the error message and formatting changes that I'd most strongly like to apply from my reverted "spruce up" commit.

Vectornaut commented on pull request StudioInfinity/dyna3#118 2025-10-10 06:01:39 +00:00
feat: Point coordinate regulators

This spacing convention is widely used in Rust code, and it's recommended in the Rust style guide:

For a single-line…

Vectornaut pushed to pointCoordRegulators at glen/dyna3 2025-10-10 05:45:04 +00:00
c081f1a809 Revert "Spruce up formatting and error messages"
Vectornaut commented on pull request StudioInfinity/dyna3#118 2025-10-10 05:44:49 +00:00
feat: Point coordinate regulators

Any chance we could just unwind that commit, and then if there are a handful of individual formatting things that you think are particularly problematic, you enter them as review comments for me…

Vectornaut commented on pull request StudioInfinity/dyna3#118 2025-10-09 05:28:07 +00:00
feat: Point coordinate regulators

since the PR didn't create the bug, just gave us a way for the first time to trigger it, I have no problem with your filing an issue and merging this without fixing the bug.

Great. I just…