Misc cleanup (#194)

* Rename `watcher` module to `file_watcher`

The name change itself is largely benign. The sole motivation is that
`cargo test` defaults to alphabetical order for running tests and the
`watcher::tests::the_guantlet` is the longest test by far while also
being the last in the alphabet.

This change makes the test suite go from taking 0.92 seconds to 0.64
seconds. `file_watcher::tests::the_gauntlet` also happened to take 0.64
seconds according to `cargo nextest run`

* Mostly remove bespoke cruft from keybindings sanity test

* Mostly standardized import style

Not actually an important change, but some of the imports were
sprawling, so I figured it was worth doing a little cleanup

* Fix where cache is created on macos arm CI job

* Make yaml snapshot test slightly more succinct

* Also run CI tests on `beta` to catch breakages semi-early

* Placate `clippy`

* Add `clippy` and `rustfmt` checks to CI

* `#![deny]` some useful clippy lints

.github/workflows/CI.yml

@@ -1,4 +1,3 @@
-
 name: CI
 
 on: [push, pull_request]
@@ -7,37 +6,48 @@ env:
   CARGO_TERM_COLOR: always
   RUSTFLAGS: "-C debuginfo=0"
 
-# TODO: build/test on stable and beta?
-
 jobs:
   build:
     strategy:
       matrix:
         os: [windows-latest, macos-latest, ubuntu-latest]
+        toolchain: [stable, beta]
 
     runs-on: ${{ matrix.os }}
 
     steps:
       - uses: actions/checkout@v3
 
+      - name: Install toolchain
+        run: |
+          rustup update --no-self-update ${{ matrix.toolchain }}
+          rustup default ${{ matrix.toolchain }}
+          rustup component add clippy rustfmt
+
       - name: Cache
         uses: Swatinem/rust-cache@v2
-
-      - name: Stable
+        with:
+          prefix-key: "v0-rust-2"
+
+      # TODO: In the future clippy and rustfmt dont need to be run on all the
+      # `os`', but it should be fast enough to be fine still for now
+      - name: Formatting
+        run: cargo fmt --check
+      - name: Linting
+        run: cargo clippy -- --deny warnings
+      - name: Run test suite
         run: cargo test --workspace
 
   check-macos-arm:
     runs-on: macos-11
     steps:
       - uses: actions/checkout@v3
 
-      # FIXME: This is in the wrong area. It should be below setting up the
-      # toolchain
-      - name: Cache
-        uses: Swatinem/rust-cache@v2
-
       - name: Install target
         run: rustup update && rustup target add aarch64-apple-darwin
 
+      - name: Cache
+        uses: Swatinem/rust-cache@v2
+
       - name: Build
         run: cargo build --target=aarch64-apple-darwin

src/color.rs

@@ -1,4 +1,6 @@
-use std::{fs::File, io::BufReader, path::PathBuf};
+use std::fs::File;
+use std::io::BufReader;
+use std::path::PathBuf;
 
 use anyhow::Context;
 use serde::Deserialize;

src/debug_impls.rs

@@ -4,7 +4,8 @@
 
 use std::fmt;
 
-use crate::{positioner::Spacer, text::Text};
+use crate::positioner::Spacer;
+use crate::text::Text;
 
 use glyphon::FamilyOwned;
 

src/watcher/mod.rs → src/file_watcher/mod.rs

@@ -1,20 +1,16 @@
-use std::{
-    path::{Path, PathBuf},
-    sync::mpsc,
-    time::Duration,
-};
+#[cfg(test)]
+mod tests;
+
+use std::path::{Path, PathBuf};
+use std::sync::mpsc;
+use std::time::Duration;
 
 use crate::InlyneEvent;
 
-use notify::{
-    event::{EventKind, ModifyKind},
-    Event, EventHandler, RecommendedWatcher, RecursiveMode, Watcher as _,
-};
+use notify::event::{EventKind, ModifyKind};
+use notify::{Event, EventHandler, RecommendedWatcher, RecursiveMode, Watcher as _};
 use winit::event_loop::EventLoopProxy;
 
-#[cfg(test)]
-mod tests;
-
 trait Callback: Send + 'static {
     fn file_reload(&self);
     fn file_change(&self, contents: String);

src/watcher/tests.rs → src/file_watcher/tests.rs

@@ -1,4 +1,7 @@
-use std::{fs, path::Path, sync::mpsc, time::Duration};
+use std::fs;
+use std::path::Path;
+use std::sync::mpsc;
+use std::time::Duration;
 
 use super::{Callback, Watcher};
 use crate::test_utils::init_test_log;

src/image/decode.rs

@@ -1,15 +1,15 @@
-use crate::utils::usize_in_mib;
-use image::{
-    codecs::{
-        gif::GifDecoder, jpeg::JpegDecoder, png::PngDecoder, tiff::TiffDecoder, webp::WebPDecoder,
-    },
-    ColorType, GenericImageView, ImageDecoder, ImageFormat,
-};
-use lz4_flex::frame::{BlockSize, FrameDecoder, FrameEncoder, FrameInfo};
 use std::cmp;
 use std::io;
 use std::time::Instant;
 
+use crate::utils::usize_in_mib;
+
+use image::codecs::{
+    gif::GifDecoder, jpeg::JpegDecoder, png::PngDecoder, tiff::TiffDecoder, webp::WebPDecoder,
+};
+use image::{ColorType, GenericImageView, ImageDecoder, ImageFormat};
+use lz4_flex::frame::{BlockSize, FrameDecoder, FrameEncoder, FrameInfo};
+
 pub fn lz4_compress<R: io::Read>(reader: &mut R) -> anyhow::Result<Vec<u8>> {
     let mut frame_info = FrameInfo::new();
     frame_info.block_size = BlockSize::Max256KB;

src/image/mod.rs

@@ -1,26 +1,26 @@
+mod decode;
+#[cfg(test)]
+mod tests;
+
+use std::borrow::Cow;
+use std::path::PathBuf;
+use std::sync::{Arc, Mutex};
+use std::time::Instant;
+use std::{fs, io};
+
 use crate::debug_impls::{DebugBytesPrefix, DebugInline};
 use crate::interpreter::ImageCallback;
 use crate::positioner::DEFAULT_MARGIN;
 use crate::utils::{usize_in_mib, Align, Point, Size};
+
 use anyhow::Context;
 use bytemuck::{Pod, Zeroable};
 use image::{ImageBuffer, RgbaImage};
 use smart_debug::SmartDebug;
-use std::fs;
-use std::io;
-use std::path::PathBuf;
-use std::sync::{Arc, Mutex};
-use std::time::Instant;
 use usvg::{TreeParsing, TreeTextToPath};
 use wgpu::util::DeviceExt;
 use wgpu::{BindGroup, Device, TextureFormat};
 
-use std::borrow::Cow;
-
-mod decode;
-#[cfg(test)]
-mod tests;
-
 #[derive(Debug, Clone)]
 pub enum ImageSize {
     PxWidth(u32),

src/image/tests.rs

@@ -1,4 +1,5 @@
-use std::{fmt, fs, path::Path};
+use std::path::Path;
+use std::{fmt, fs};
 
 use super::ImageData;
 use crate::test_utils::init_test_log;

src/interpreter/html.rs

@@ -1,8 +1,11 @@
 use std::slice;
 
-use html5ever::{local_name, Attribute};
+use crate::positioner::Section;
+use crate::table::Table;
+use crate::text::TextBox;
+use crate::utils::Align;
 
-use crate::{positioner::Section, table::Table, text::TextBox, utils::Align};
+use html5ever::{local_name, Attribute};
 
 pub fn find_align(attrs: &[Attribute]) -> Option<Align> {
     AttrIter::new(attrs).find_map(|attr| {
@@ -39,7 +42,7 @@ impl<'attrs> Iterator for AttrIter<'attrs> {
         loop {
             let Attribute { name, value } = self.0.next()?;
             let attr = match name.local {
-                local_name!("align") => Align::new(&value).map(Attr::Align),
+                local_name!("align") => Align::new(value).map(Attr::Align),
                 local_name!("href") => Some(Attr::Href(value.to_string())),
                 local_name!("id") => Some(Attr::Anchor(format!("#{value}"))),
                 local_name!("width") => value.parse().ok().map(Attr::Width),

src/interpreter/mod.rs

@@ -1,47 +1,32 @@
-use crate::color::native_color;
-use crate::image::Image;
-use crate::image::ImageData;
-use crate::image::ImageSize;
-use crate::positioner::Positioned;
-use crate::positioner::Row;
-use crate::positioner::Section;
-use crate::positioner::Spacer;
-use crate::positioner::DEFAULT_MARGIN;
-use crate::table::Table;
-use crate::ImageCache;
-use crate::InlyneEvent;
+mod html;
+#[cfg(test)]
+mod tests;
 
-use crate::color::Theme;
+use std::collections::VecDeque;
+use std::path::PathBuf;
+use std::str::FromStr;
+use std::sync::atomic::AtomicBool;
+use std::sync::{mpsc, Arc, Mutex};
+
+use crate::color::{native_color, Theme};
+use crate::image::{Image, ImageData, ImageSize};
+use crate::positioner::{Positioned, Row, Section, Spacer, DEFAULT_MARGIN};
+use crate::table::Table;
 use crate::text::{Text, TextBox};
 use crate::utils::{markdown_to_html, Align};
-use crate::Element;
+use crate::{Element, ImageCache, InlyneEvent};
+use html::{Attr, AttrIter, FontStyle, FontWeight, Style, StyleIter};
 
 use glyphon::FamilyOwned;
 use html5ever::tendril::*;
-use html5ever::tokenizer::BufferQueue;
-use html5ever::tokenizer::TagKind;
-use html5ever::tokenizer::TagToken;
-use html5ever::tokenizer::{Token, TokenSink, TokenSinkResult};
-use html5ever::tokenizer::{Tokenizer, TokenizerOpts};
+use html5ever::tokenizer::{
+    BufferQueue, TagKind, TagToken, Token, TokenSink, TokenSinkResult, Tokenizer, TokenizerOpts,
+};
 use wgpu::TextureFormat;
 use winit::event_loop::EventLoopProxy;
 use winit::window::Window;
 use Token::{CharacterTokens, EOFToken};
 
-use std::collections::VecDeque;
-use std::path::PathBuf;
-use std::str::FromStr;
-use std::sync::atomic::AtomicBool;
-use std::sync::mpsc;
-use std::sync::Arc;
-use std::sync::Mutex;
-
-mod html;
-#[cfg(test)]
-mod tests;
-
-use html::{Attr, AttrIter, FontStyle, FontWeight, Style, StyleIter};
-
 #[derive(Default)]
 struct State {
     global_indent: f32,
@@ -117,6 +102,9 @@ pub struct HtmlInterpreter {
 }
 
 impl HtmlInterpreter {
+    // FIXME: clippy is probably right here, but I didn't want to hold up setting up clippy for the
+    // rest of the repo just because of here
+    #[allow(clippy::too_many_arguments)]
     pub fn new(
         window: Arc<Window>,
         element_queue: Arc<Mutex<VecDeque<Element>>>,

src/interpreter/snapshots/inlyne__interpreter__tests__yaml_frontmatter.snap

@@ -1,6 +1,6 @@
 ---
 source: src/interpreter/tests.rs
-description: " --- md\n\n---\ntitle: Title\ndate: 2018-05-01\ntags:\n  - another tag\n---\n# Markdown h1 header\n\n\n --- html\n\n<table>\n<thead>\n<tr>\n<th align=\"center\">title</th>\n<th align=\"center\">date</th>\n<th align=\"center\">tags</th>\n</tr>\n</thead>\n<tbody>\n<tr>\n<td align=\"center\">Title</td>\n<td align=\"center\">2018-05-01</td>\n<td align=\"center\">{Skipped nested table}</td>\n</tr>\n</tbody>\n</table>\n<h1>Markdown h1 header</h1>\n"
+description: " --- md\n\n---\ndate: 2018-05-01\ntags:\n  - another tag\n---\n# Markdown h1 header\n\n\n --- html\n\n<table>\n<thead>\n<tr>\n<th align=\"center\">date</th>\n<th align=\"center\">tags</th>\n</tr>\n</thead>\n<tbody>\n<tr>\n<td align=\"center\">2018-05-01</td>\n<td align=\"center\">{Skipped nested table}</td>\n</tr>\n</tbody>\n</table>\n<h1>Markdown h1 header</h1>\n"
 expression: interpret_md(text)
 ---
 [
@@ -10,18 +10,6 @@ expression: interpret_md(text)
     Table(
         Table {
             headers: [
-                TextBox {
-                    align: Center,
-                    texts: [
-                        Text {
-                            text: "title",
-                            default_color: Color(BLACK),
-                            style: BOLD ,
-                            ..
-                        },
-                    ],
-                    ..
-                },
                 TextBox {
                     align: Center,
                     texts: [
@@ -49,17 +37,6 @@ expression: interpret_md(text)
             ],
             rows: [
                 [
-                    TextBox {
-                        align: Center,
-                        texts: [
-                            Text {
-                                text: "Title",
-                                default_color: Color(BLACK),
-                                ..
-                            },
-                        ],
-                        ..
-                    },
                     TextBox {
                         align: Center,
                         texts: [

src/interpreter/tests.rs

@@ -1,16 +1,17 @@
-use std::{
-    collections::VecDeque,
-    path::PathBuf,
-    sync::{
-        atomic::{AtomicU32, Ordering},
-        mpsc, Arc, Mutex,
-    },
-    thread,
-    time::{Duration, Instant},
+use std::collections::VecDeque;
+use std::path::PathBuf;
+use std::sync::{
+    atomic::{AtomicU32, Ordering},
+    mpsc, Arc, Mutex,
 };
+use std::thread;
+use std::time::{Duration, Instant};
 
 use super::{HtmlInterpreter, ImageCallback, WindowInteractor};
-use crate::{color::Theme, image::ImageData, test_utils::init_test_log, Element, ImageCache};
+use crate::color::Theme;
+use crate::image::ImageData;
+use crate::test_utils::init_test_log;
+use crate::{Element, ImageCache};
 
 use wgpu::TextureFormat;
 use wiremock::{matchers, Mock, MockServer, ResponseTemplate};
@@ -213,7 +214,6 @@ const CODE_IN_ORDERED_LIST: &str = "\
 
 const YAML_FRONTMATTER: &str = "\
 ---
-title: Title
 date: 2018-05-01
 tags:
   - another tag
@@ -286,7 +286,6 @@ fn image_loading_fails_gracefully() {
 
     let text = format!("![This actually returns JSON 😈]({json_url})");
 
-    // TODO: Need to have some better way of either signaling or detecting errors here
     insta::with_settings!({
         // The port for the URL here is non-deterministic, but the description changing doesn't
         // invalidate the snapshot, so that's okay

src/keybindings/defaults.rs

@@ -1,7 +1,5 @@
-use super::{
-    action::{Action, VertDirection, Zoom},
-    Key, KeyCombo, ModifiedKey,
-};
+use super::action::{Action, VertDirection, Zoom};
+use super::{Key, KeyCombo, ModifiedKey};
 
 use winit::event::{ModifiersState, VirtualKeyCode as VirtKey};