Another round of polishing (#101)

* Implement basic refactorings

* Simplify some match statements

* Rename `ix` to `index`

If we're not going with a single letter name,
then a full word makes more sense.

* Rename `as_hex` to `to_hex`

`as_` implies lossless reinterpretation
while the function does a conversion that loses information

* Replace `for_each` with for loops

for loops are a lot easier to read and maintain.

* factor out x and y coords in Line::render

this is arguably more ergonomic

* Remove redundant `format!(format_args!())`

client/web/wasm/src/document.rs

@@ -7,6 +7,7 @@ use wasm_bindgen::prelude::*;
 fn convert_error(err: editor_core::EditorError) -> JsValue {
 	Error::new(&err.to_string()).into()
 }
+
 mod mouse_state {
 	pub(super) type MouseKeys = u8;
 	use editor_core::events::{self, Event, MouseState, ViewportPosition};
@@ -35,6 +36,7 @@ mod mouse_state {
 		}
 	}
 }
+
 /// Modify the currently selected tool in the document state store
 #[wasm_bindgen]
 pub fn select_tool(tool: String) -> Result<(), JsValue> {

client/web/wasm/src/utils.rs

@@ -38,7 +38,7 @@ impl log::Log for WasmLog {
 			log::Level::Info => (info, "info", "color:#1b8"),
 			log::Level::Error => (error, "error", "color:red"),
 		};
-		let msg = &format!("{}", format_args!("%c{}\t{}", name, record.args()));
+		let msg = &format!("%c{}\t{}", name, record.args());
 		log(msg, color)
 	}
 	fn flush(&self) {}

client/web/wasm/src/wrappers.rs

@@ -86,30 +86,30 @@ pub fn translate_append_mode(name: &str) -> Option<SelectAppendMode> {
 }
 
 pub fn translate_key(name: &str) -> events::Key {
-	use events::Key as K;
+	use events::Key::*;
 	match name {
-		"e" => K::KeyE,
+		"e" => KeyE,
-		"v" => K::KeyV,
+		"v" => KeyV,
-		"l" => K::KeyL,
+		"l" => KeyL,
-		"p" => K::KeyP,
+		"p" => KeyP,
-		"r" => K::KeyR,
+		"r" => KeyR,
-		"m" => K::KeyM,
+		"m" => KeyM,
-		"x" => K::KeyX,
+		"x" => KeyX,
-		"z" => K::KeyZ,
+		"z" => KeyZ,
-		"y" => K::KeyY,
+		"y" => KeyY,
-		"0" => K::Key0,
+		"0" => Key0,
-		"1" => K::Key1,
+		"1" => Key1,
-		"2" => K::Key2,
+		"2" => Key2,
-		"3" => K::Key3,
+		"3" => Key3,
-		"4" => K::Key4,
+		"4" => Key4,
-		"5" => K::Key5,
+		"5" => Key5,
-		"6" => K::Key6,
+		"6" => Key6,
-		"7" => K::Key7,
+		"7" => Key7,
-		"8" => K::Key8,
+		"8" => Key8,
-		"9" => K::Key9,
+		"9" => Key9,
-		"Enter" => K::KeyEnter,
+		"Enter" => KeyEnter,
-		"Shift" => K::KeyShift,
+		"Shift" => KeyShift,
-		"Alt" => K::KeyAlt,
+		"Alt" => KeyAlt,
-		_ => K::UnknownKey,
+		_ => UnknownKey,
 	}
 }

core/document/src/color.rs

@@ -55,7 +55,7 @@ impl Color {
 	pub fn components(&self) -> (f32, f32, f32, f32) {
 		(self.red, self.green, self.blue, self.alpha)
 	}
-	pub fn as_hex(&self) -> String {
+	pub fn to_hex(&self) -> String {
 		format!(
 			"{:02X?}{:02X?}{:02X?}{:02X?}",
 			(self.r() * 255.) as u8,

core/document/src/document.rs

@@ -1,7 +1,5 @@
-use layers::PolyLine;
-
 use crate::{
-	layers::{self, Folder, Layer, LayerData, LayerDataTypes, Line, Rect, Shape},
+	layers::{self, Folder, Layer, LayerData, LayerDataTypes, Line, PolyLine, Rect, Shape},
 	response::{LayerPanelEntry, LayerType},
 	DocumentError, DocumentResponse, LayerId, Operation,
 };

core/document/src/layers/folder.rs

@@ -13,11 +13,12 @@ pub struct Folder {
 
 impl LayerData for Folder {
 	fn render(&mut self, svg: &mut String) {
-		self.layers.iter_mut().for_each(|layer| {
+		for layer in &mut self.layers {
 			let _ = writeln!(svg, "{}", layer.render());
-		});
+		}
 	}
 }
+
 impl Folder {
 	pub fn add_layer(&mut self, layer: Layer, insert_index: isize) -> Option<LayerId> {
 		let mut insert_index = insert_index as i128;

core/document/src/layers/line.rs

@@ -20,14 +20,9 @@ impl Line {
 
 impl LayerData for Line {
 	fn render(&mut self, svg: &mut String) {
-		let _ = write!(
+		let kurbo::Point { x: x1, y: y1 } = self.shape.p0;
-			svg,
+		let kurbo::Point { x: x2, y: y2 } = self.shape.p1;
-			r#"<line x1="{}" y1="{}" x2="{}" y2="{}" {} />"#,
+
-			self.shape.p0.x,
+		let _ = write!(svg, r#"<line x1="{}" y1="{}" x2="{}" y2="{}" {} />"#, x1, y1, x2, y2, self.style.render(),);
-			self.shape.p0.y,
-			self.shape.p1.x,
-			self.shape.p1.y,
-			self.style.render(),
-		);
 	}
 }

core/document/src/layers/mod.rs

@@ -36,16 +36,26 @@ pub enum LayerDataTypes {
 	Shape(Shape),
 }
 
+macro_rules! call_render {
+    ($self:ident.render($svg:ident) { $($variant:ident),* }) => {
+		match $self {
+			$(Self::$variant(x) => x.render($svg)),*
+		}
+	};
+}
+
 impl LayerDataTypes {
 	pub fn render(&mut self, svg: &mut String) {
-		match self {
+		call_render! {
-			Self::Folder(f) => f.render(svg),
+			self.render(svg) {
-			Self::Circle(c) => c.render(svg),
+				Folder,
-			Self::Ellipse(e) => e.render(svg),
+				Circle,
-			Self::Rect(r) => r.render(svg),
+				Ellipse,
-			Self::Line(l) => l.render(svg),
+				Rect,
-			Self::PolyLine(pl) => pl.render(svg),
+				Line,
-			Self::Shape(s) => s.render(svg),
+				PolyLine,
+				Shape
+			}
 		}
 	}
 }

core/document/src/layers/polyline.rs

@@ -24,13 +24,14 @@ impl LayerData for PolyLine {
 			return;
 		}
 		let _ = write!(svg, r#"<polyline points=""#);
-		self.points.iter().for_each(|p| {
+		for p in &self.points {
 			let _ = write!(svg, " {:.3} {:.3}", p.x, p.y);
-		});
+		}
 		let _ = write!(svg, r#"" {}/>"#, self.style.render());
 	}
 }
 
+#[cfg(test)]
 #[test]
 fn polyline_should_render() {
 	let mut polyline = PolyLine {

core/document/src/layers/style/mod.rs

@@ -15,8 +15,8 @@ impl Fill {
 	}
 	pub fn render(&self) -> String {
 		match self.color {
-			Some(c) => format!("fill: #{};", c.as_hex()),
+			Some(c) => format!("fill: #{};", c.to_hex()),
-			None => format!("fill: none;"),
+			None => "fill: none;".to_string(),
 		}
 	}
 }
@@ -33,7 +33,7 @@ impl Stroke {
 		Self { color, width }
 	}
 	pub fn render(&self) -> String {
-		format!("stroke: #{};stroke-width:{};", self.color.as_hex(), self.width)
+		format!("stroke: #{};stroke-width:{};", self.color.to_hex(), self.width)
 	}
 }
 

core/document/src/shape_points.rs

@@ -62,7 +62,7 @@ impl std::fmt::Display for ShapePoints {
 #[doc(hidden)]
 pub struct ShapePathIter {
 	shape: ShapePoints,
-	ix: usize,
+	index: usize,
 }
 
 impl Iterator for ShapePathIter {
@@ -74,11 +74,11 @@ impl Iterator for ShapePathIter {
 			let sine = theta.sin();
 			Vec2::new(v.x * cosine - v.y * sine, v.x * sine + v.y * cosine)
 		}
-		self.ix += 1;
+		self.index += 1;
-		match self.ix {
+		match self.index {
 			1 => Some(PathEl::MoveTo(self.shape.center + self.shape.extent)),
 			_ => {
-				let radians = self.shape.apothem_offset_angle() * ((self.ix * 2 + (self.shape.sides % 2) as usize) as f64);
+				let radians = self.shape.apothem_offset_angle() * ((self.index * 2 + (self.shape.sides % 2) as usize) as f64);
 				let offset = rotate(&self.shape.extent, radians);
 				let point = self.shape.center + offset;
 				Some(PathEl::LineTo(point))

core/editor/src/dispatcher/mod.rs

@@ -30,22 +30,7 @@ impl Dispatcher {
 				editor_state.tool_state.document_tool_data.primary_color = Color::BLACK;
 				editor_state.tool_state.document_tool_data.secondary_color = Color::WHITE;
 			}
-			Event::LmbDown(mouse_state) => {
+			Event::LmbDown(mouse_state) | Event::RmbDown(mouse_state) | Event::MmbDown(mouse_state) | Event::LmbUp(mouse_state) | Event::RmbUp(mouse_state) | Event::MmbUp(mouse_state) => {
-				editor_state.tool_state.document_tool_data.mouse_state = *mouse_state;
-			}
-			Event::RmbDown(mouse_state) => {
-				editor_state.tool_state.document_tool_data.mouse_state = *mouse_state;
-			}
-			Event::MmbDown(mouse_state) => {
-				editor_state.tool_state.document_tool_data.mouse_state = *mouse_state;
-			}
-			Event::LmbUp(mouse_state) => {
-				editor_state.tool_state.document_tool_data.mouse_state = *mouse_state;
-			}
-			Event::RmbUp(mouse_state) => {
-				editor_state.tool_state.document_tool_data.mouse_state = *mouse_state;
-			}
-			Event::MmbUp(mouse_state) => {
 				editor_state.tool_state.document_tool_data.mouse_state = *mouse_state;
 			}
 			Event::MouseMove(pos) => {