Vectornaut
  • Joined on 2019-09-28
Vectornaut pushed to main at Vectornaut/dyna3 2025-12-02 08:19:31 +00:00
d7587cb57f chore: wrap at 80 characters (#128)
Vectornaut pushed to main at StudioInfinity/dyna3 2025-12-02 08:13:30 +00:00
d7587cb57f chore: wrap at 80 characters (#128)
Vectornaut merged pull request StudioInfinity/dyna3#128 2025-12-02 08:13:28 +00:00
chore: wrap at 80 characters
Vectornaut commented on pull request StudioInfinity/dyna3#128 2025-12-02 08:01:34 +00:00
chore: wrap at 80 characters

This seems acceptable to me. It leaves the code in an inconsistently formatted state, and one that neither of us seems entirely happy with, but I think we'll need Husht to resolve some of the…

Vectornaut commented on pull request StudioInfinity/dyna3#128 2025-11-27 00:12:24 +00:00
chore: wrap at 80 characters

Our current convention is to put spaces after the commas between arguments.

Vectornaut suggested changes for StudioInfinity/dyna3#128 2025-11-27 00:12:24 +00:00
chore: wrap at 80 characters

Looks like there's just two small tweaks left.

Vectornaut commented on pull request StudioInfinity/dyna3#128 2025-11-27 00:09:59 +00:00
chore: wrap at 80 characters

Please confirm, thanks.

Confirmed.

Vectornaut commented on pull request StudioInfinity/dyna3#128 2025-11-26 23:56:50 +00:00
chore: wrap at 80 characters

It's just an abbreviation.

Oh, I see: you're now trying to find a name short enough to jam that expression into one line. Personally, I think the extra clarity is worth an extra line.…

Vectornaut commented on pull request StudioInfinity/dyna3#128 2025-11-26 23:46:19 +00:00
chore: wrap at 80 characters

I'll admit I am looking for a two-line solution here.

All of the reasonable two-line solutions I can think of seem equally hard to read to me, and equally in violation of the [Rust](https://do

Vectornaut commented on pull request StudioInfinity/dyna3#128 2025-11-26 23:33:47 +00:00
chore: wrap at 80 characters

To me, turning a variable called value into a variable called spec_val seems more understandable than turning a variable called value into a variable called val. In the latter case, the…

Vectornaut commented on pull request StudioInfinity/dyna3#128 2025-11-23 08:18:13 +00:00
chore: wrap at 80 characters

I think the main obstacle here is my preferred way to format nesting in language with C-like syntax, [described above](StudioInfinity/dyna3#128 (comment)

Vectornaut commented on pull request StudioInfinity/dyna3#128 2025-11-23 08:06:47 +00:00
chore: wrap at 80 characters

I am also a strong advocate for laying out code as closely as one would lay out other forms of text, equations in articles, etc.

This is helpful guideline, and we should keep it in mind when…

Vectornaut commented on pull request StudioInfinity/dyna3#128 2025-11-23 07:53:46 +00:00
chore: wrap at 80 characters

The main thing I find confusing about the current formatting is that the -> DVector<f64> is part of the function declaration, but no indentation is used to show this. If you find bracket on next…

Vectornaut commented on pull request StudioInfinity/dyna3#128 2025-11-23 07:07:37 +00:00
chore: wrap at 80 characters

Well that's how it was, but i was trying to accommodate the preference for breaking at a method call instead of within the arguments.

Oh, I see. Like I said [above](https://code.studioinfinity

Vectornaut commented on pull request StudioInfinity/dyna3#128 2025-11-23 07:04:23 +00:00
chore: wrap at 80 characters

I find (2) above most readable when more than one dot item is chained, so I guess I'd be fine with switching back to Python-style hanging indent for single method calls. I realize that this is…

Vectornaut commented on pull request StudioInfinity/dyna3#128 2025-11-23 06:53:49 +00:00
chore: wrap at 80 characters

My current attempt was motivated by your apparent preference to split before a ., rather than just after a (. Is that not the case?

My preferences are:

  1. In languages with C-like…
Vectornaut commented on pull request StudioInfinity/dyna3#128 2025-11-23 06:35:21 +00:00
chore: wrap at 80 characters

How about depth_if_hit? To me, this communicates that the value will be Some(depth) if we hit elt and None if we missed.

Vectornaut commented on pull request StudioInfinity/dyna3#128 2025-11-22 08:45:06 +00:00
chore: wrap at 80 characters

Your alternate tweak looks fine to me.

Vectornaut commented on pull request StudioInfinity/dyna3#128 2025-11-22 08:42:16 +00:00
chore: wrap at 80 characters

Tried to go through and reformat accordingly.

Thanks! I've flagged a few missed occurrences in the next review.

Vectornaut commented on pull request StudioInfinity/dyna3#128 2025-11-22 08:39:39 +00:00
chore: wrap at 80 characters

There were several others as well.

Yes, looks like you got them all!