feat: Point coordinate regulators #118
|
|
@ -127,7 +127,7 @@ pub trait Element: Serial + ProblemPoser + DisplayItem {
|
|||
}
|
||||
|
||||
impl Debug for dyn Element {
|
||||
fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), fmt::Error> {
|
||||
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
|
||||
|
glen marked this conversation as resolved
Outdated
|
||||
self.id().fmt(f)
|
||||
}
|
||||
}
|
||||
|
|
@ -511,10 +511,18 @@ impl ProblemPoser for HalfCurvatureRegulator {
|
|||
}
|
||||
|
||||
#[derive(Clone, Copy, Sequence)]
|
||||
pub enum Axis {X = 0, Y = 1, Z = 2}
|
||||
pub enum Axis { X = 0, Y = 1, Z = 2 }
|
||||
|
||||
impl Axis {
|
||||
|
glen marked this conversation as resolved
Outdated
glen
commented
I generally prefer to punctuate code as one would text, when possible. Can we remove the two extraneous spaces you just added here? They don't seem to improve readability (as we are all used to no space between brackets and content). I generally prefer to punctuate code as one would text, when possible. Can we remove the two extraneous spaces you just added here? They don't seem to improve readability (as we are all used to no space between brackets and content).
Vectornaut
commented
This spacing convention is widely used in Rust code, and it's recommended in the Rust style guide:
I like it a lot, and I've used it in hundreds of places in the dyna3 source code. If you're adamant about changing it, I'd prefer to do it throughout the source code in a formatting-only pull request. This spacing convention is widely used in Rust code, and it's [recommended](https://doc.rust-lang.org/beta/style-guide/expressions.html#blocks) in the Rust style guide:
> For a single-line block, put spaces after the opening brace and before the closing brace.
I like it a lot, and I've used it in hundreds of places in the dyna3 source code. If you're adamant about changing it, I'd prefer to do it throughout the source code in a formatting-only pull request.
glen
commented
We don't have to fix it now. I don't care at all about the Rust style guide, it's an assembly of weird rigidities from my point of view (what business does a language have semi-enforcing snake_case identifiers? I thought typography reiterating semantic category went into the trashbin with Perl a decade or more ago...). I very much like the convention of punctuating code as close as possible to regular text. I get it that you are used to these spaces that look very extra to me. We'll figure out how to resolve this at some point. No need to spin our wheels on it at the moment. We don't have to fix it now. I don't care at all about the Rust style guide, it's an assembly of weird rigidities from my point of view (what business does a _language_ have semi-enforcing snake_case identifiers? I thought typography reiterating semantic category went into the trashbin with Perl a decade or more ago...). I very much like the convention of punctuating code as close as possible to regular text. I get it that you are used to these spaces that look very extra to me. We'll figure out how to resolve this at some point. No need to spin our wheels on it at the moment.
|
||||
pub const NAME: [&str; Axis::CARDINALITY] = ["X", "Y", "Z"];
|
||||
fn name(&self) -> &'static str {
|
||||
|
Vectornaut marked this conversation as resolved
Outdated
Vectornaut
commented
The associated constant The associated constant `Axis::N_AXIS`, which is defined by hand here, has the same value as the associated constant [`Axis::CARDINALITY`](https://docs.rs/enum-iterator/latest/enum_iterator/trait.Sequence.html#associatedconstant.CARDINALITY), which is defined automatically when we derive the `Sequence` trait. I'd recommend removing `N_AXIS` and using `CARDINALITY` in its place. I'm confident that `CARDINALITY` is currently 3, because I get a compilation error if I replace it with a value different from 3 in the definition of `Axis::NAME`.
|
||||
match self { Axis::X => "X", Axis::Y => "Y", Axis::Z => "Z" }
|
||||
}
|
||||
}
|
||||
|
||||
impl fmt::Display for Axis {
|
||||
|
Vectornaut marked this conversation as resolved
Outdated
Vectornaut
commented
Whoops, here's a formatting mistake from the revision I made. I should've just written and changed the relevant Would you mind including that change in your next round of revisions? Whoops, here's a formatting mistake from the revision I made. I should've just written
```rust
impl Display for Axis
```
and changed the relevant `use` expression as follows:
```diff
- fmt::{Debug, Formatter}
+ fmt::{Debug, Display, Formatter}
```
Would you mind including that change in your next round of revisions?
glen
commented
Yes of course, and this makes sense. Yes of course, and this makes sense.
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
f.write_str(self.name())
|
||||
}
|
||||
}
|
||||
|
||||
pub struct PointCoordinateRegulator {
|
||||
|
|
|
|||
|
|
@ -6,7 +6,6 @@ use web_sys::{KeyboardEvent, MouseEvent, wasm_bindgen::JsCast};
|
|||
use crate::{
|
||||
AppState,
|
||||
assembly::{
|
||||
Axis,
|
||||
Element,
|
||||
HalfCurvatureRegulator,
|
||||
InversiveDistanceRegulator,
|
||||
|
|
@ -123,7 +122,7 @@ impl OutlineItem for HalfCurvatureRegulator {
|
|||
|
||||
impl OutlineItem for PointCoordinateRegulator {
|
||||
fn outline_item(self: Rc<Self>, _element: &Rc<dyn Element>) -> View {
|
||||
let name = format!("{} coordinate", Axis::NAME[self.axis as usize]);
|
||||
let name = format!("{} coordinate", self.axis);
|
||||
view! {
|
||||
li(class = "regulator") {
|
||||
div(class = "regulator-label") // for spacing
|
||||
|
|
|
|||
This change is irrelevant to this PR so we really oughtnt do it but no big deal I guess.
I'd be fine with leaving it out of this pull request. I made it because this pull request implements
DisplayforAxisusing the terserfmt::Resultconvention, and I wanted the previous implementation ofDebugfordyn Elementto look consistent.like I said, no big deal either way, resolving.