Fix document reloading with delete-then-write editors and redo BottomMessage updating to work better

* Relicense to GPLv3 since poppler is GPL and we must not violate that license

* Add note to README that contributions will be licensed as MPL-2.0

.github/workflows/rust.yml

@@ -0,0 +1,56 @@
+name: Rust
+
+on:
+  push:
+    branches: [ "main" ]
+  pull_request:
+    branches: [ "main" ]
+
+env:
+  CARGO_TERM_COLOR: always
+
+jobs:
+  build:
+
+    runs-on: ubuntu-latest
+
+    steps:
+    - name: Setup sccache
+      if: github.event_name != 'release' && github.event_name != 'workflow_dispatch'
+      uses: mozilla-actions/sccache-action@v0.0.6
+    - name: Configure sccache
+      if: github.event_name != 'release' && github.event_name != 'workflow_dispatch'
+      run: |
+        echo "SCCACHE_GHA_ENABLED=true" >> $GITHUB_ENV
+        echo "RUSTC_WRAPPER=sccache" >> $GITHUB_ENV
+    - name: Install build dependencies
+      run: |
+        sudo apt-get update
+        sudo apt-get install -y cmake libjpeg-dev libfontconfig1-dev libopenjp2-7-dev libopenjpip7 libopenjp2-7 libglib2.0-dev libnss3-dev libunwind-dev libgoogle-perftools-dev libboost-dev
+    - name: Build newer poppler
+      run: |
+        wget https://poppler.freedesktop.org/poppler-23.10.0.tar.xz
+        tar xf poppler-23.10.0.tar.xz
+        cd poppler-23.10.0
+        mkdir build && cd build
+        cmake .. -DCMAKE_BUILD_TYPE=Release \
+             -DCMAKE_C_COMPILER_LAUNCHER=sccache \
+             -DCMAKE_CXX_COMPILER_LAUNCHER=sccache \
+             -DENABLE_UNSTABLE_API_ABI_HEADERS=ON \
+             -DENABLE_GPGME=OFF \
+             -DENABLE_QT5=OFF \
+             -DENABLE_QT6=OFF \
+             -DENABLE_SPLASH=OFF \
+             -DENABLE_LIBCURL=OFF
+        make -j$(nproc)
+        sudo make install
+        sudo ldconfig
+    - uses: actions/checkout@v4
+    - name: Clippy
+      run: cargo clippy -- -D warnings
+    - name: Check fmt
+      run: cargo fmt -- --check
+    - name: Run tests
+      run: cargo test --benches -- adobe_example
+    - name: Build
+      run: cargo build

CHANGELOG.md

@@ -0,0 +1,16 @@
+# Unreleased
+
+- Add `--r-to-l` flag to support displaying pdfs that read from right to left
+- Add `--max-wide` flag to restrict amount of pages that can appear on the screen at a time
+- Small internal changes to accomodate a few more clippy lints
+- Update `ratatui` and `ratatui-image` git dependencies to latest upstream
+- Move `ratatui-image/vb64` support under `nightly` feature, enabled by default
+- Fixed a bug where jumping to a page out of range could result in weird `esc` key behavior
+- Added CI ([#31](https://github.com/itsjunetime/tdf/pull/31), thank you [@Kriejstal](https://github.com/Kreijstal))
+- Changed global allocator to [`mimalloc`](https://github.com/purpleprotocol/mimalloc_rust) for slightly improved performance
+- Fixed issue with document reloading not working when files are intermedially deleted
+- Fixed a lot of weirdness with bottom message layering/updating
+
+# v0.1.0
+
+Initial tag :)

Cargo.toml

@@ -1,8 +1,15 @@
 [package]
 name = "tdf"
 version = "0.1.0"
+authors = ["June Welker <junewelker@gmail.com>"]
 edition = "2021"
-license = "MPL-2.0"
+description = "A terminal viewer for PDFs"
+readme = "README.md"
+homepage = "https://github.com/itsjunetime/tdf"
+repository = "https://github.com/itsjunetime/tdf"
+license = "GPL-3.0-or-later"
+keywords = ["pdf", "tui", "cli", "terminal"]
+categories = ["command-line-utilities", "text-processing", "visualization"]
 default-run = "tdf"
 
 [[bin]]
@@ -17,10 +24,10 @@ poppler-rs = { version = "0.24.1", default-features = false, features = ["v23_7"
 cairo-rs = { version = "0.20.0", default-features = false, features = ["png"] }
 # we're using this branch because it has significant performance fixes that I'm waiting on responses from the upstream devs to get upstreamed. See https://github.com/ratatui-org/ratatui/issues/1116
 ratatui = { git = "https://github.com/itsjunetime/ratatui.git" }
-# ratatui = { path = "./ratatui" }
+# ratatui = { path = "./ratatui/ratatui" }
 # We're using this to have the vb64 feature (for faster base64 encoding, since that does take up a good bit of time when converting images to the Box<dyn ratatui_image::Protocol>. It also just includes a few more features that I'm waiting on main to upstream
-ratatui-image = { git = "https://github.com/itsjunetime/ratatui-image.git", branch = "vb64_on_personal", features = ["rustix", "vb64"], default-features = false }
-# ratatui-image = { path = "./ratatui-image", features = ["rustix", "vb64"], default-features = false }
+ratatui-image = { git = "https://github.com/itsjunetime/ratatui-image.git", branch = "vb64_on_personal", default-features = false }
+# ratatui-image = { path = "./ratatui-image", features = ["vb64"], default-features = false }
 crossterm = { version = "0.28.1", features = ["event-stream"] }
 image = { version = "0.25.1", features = ["png", "rayon"], default-features = false }
 notify = { version = "7.0.0", features = ["crossbeam-channel"] }
@@ -29,6 +36,8 @@ futures-util = { version = "0.3.30", default-features = false }
 glib = "0.20.0"
 itertools = "*"
 flume = { version = "0.11.0", default-features = false, features = ["async"] }
+xflags = "0.4.0-pre.2"
+mimalloc = "0.1.43"
 
 # for tracing with tokio-console
 console-subscriber = { version = "0.4.0", optional = true }
@@ -38,7 +47,8 @@ inherits = "release"
 lto = "fat"
 
 [features]
-default = []
+default = ["nightly"]
+nightly = ["ratatui-image/vb64"]
 tracing = ["tokio/tracing", "dep:console-subscriber"]
 
 [dev-dependencies]
@@ -52,3 +62,93 @@ harness = false
 [[bin]]
 name = "for_profiling"
 path = "./benches/for_profiling.rs"
+
+[lints.clippy]
+uninlined_format_args = "warn"
+redundant_closure_for_method_calls = "warn"
+cast_lossless = "warn"
+single_char_pattern = "warn"
+manual_let_else = "warn"
+ignored_unit_patterns = "warn"
+range_plus_one = "warn"
+unreadable_literal = "warn"
+redundant_else = "warn"
+assigning_clones = "warn"
+bool_to_int_with_if = "warn"
+borrow_as_ptr = "warn"
+cast_ptr_alignment = "warn"
+checked_conversions = "warn"
+copy_iterator = "warn"
+default_trait_access = "warn"
+doc_link_with_quotes = "warn"
+empty_enum = "warn"
+explicit_into_iter_loop = "warn"
+explicit_iter_loop = "warn"
+filter_map_next = "warn"
+flat_map_option = "warn"
+fn_params_excessive_bools = "warn"
+from_iter_instead_of_collect = "warn"
+implicit_clone = "warn"
+index_refutable_slice = "warn"
+inefficient_to_string = "warn"
+invalid_upcast_comparisons = "warn"
+iter_filter_is_ok = "warn"
+iter_filter_is_some = "warn"
+iter_not_returning_iterator = "warn"
+large_futures = "warn"
+large_stack_arrays = "warn"
+large_types_passed_by_value = "warn"
+linkedlist = "warn"
+macro_use_imports = "warn"
+manual_assert = "warn"
+manual_instant_elapsed = "warn"
+manual_is_power_of_two = "warn"
+manual_is_variant_and = "warn"
+manual_ok_or = "warn"
+manual_string_new = "warn"
+many_single_char_names = "warn"
+manual_unwrap_or = "warn"
+match_on_vec_items = "warn"
+match_same_arms = "warn"
+match_wildcard_for_single_variants = "warn"
+maybe_infinite_iter = "warn"
+mismatching_type_param_order = "warn"
+missing_fields_in_debug = "warn"
+mut_mut = "warn"
+needless_bitwise_bool = "warn"
+needless_continue = "warn"
+needless_for_each = "warn"
+needless_pass_by_value = "warn"
+needless_raw_string_hashes = "warn"
+no_effect_underscore_binding = "warn"
+no_mangle_with_rust_abi = "warn"
+option_as_ref_cloned = "warn"
+option_option = "warn"
+ptr_as_ptr = "warn"
+ptr_cast_constness = "warn"
+range_minus_one = "warn"
+ref_as_ptr = "warn"
+ref_binding_to_reference = "warn"
+ref_option = "warn"
+ref_option_ref = "warn"
+return_self_not_must_use = "warn"
+same_functions_in_if_condition = "warn"
+should_panic_without_expect = "warn"
+similar_names = "warn"
+stable_sort_primitive = "warn"
+str_split_at_newline = "warn"
+struct_excessive_bools = "warn"
+struct_field_names = "warn"
+transmute_ptr_to_ptr = "warn"
+trivially_copy_pass_by_ref = "warn"
+unicode_not_nfc = "warn"
+unnecessary_box_returns = "warn"
+unnecessary_join = "warn"
+unnecessary_literal_bound = "warn"
+unnecessary_wraps = "warn"
+unnested_or_patterns = "warn"
+unused_async = "warn"
+unused_self = "warn"
+used_underscore_binding = "warn"
+used_underscore_items = "warn"
+zero_sized_map_values = "warn"

README.md

@@ -29,3 +29,5 @@ I dunno. Just for fun, mostly.
 ## Can I contribute?
 
 Yeah, sure. Please do.
+
+Please note, though, that all contributions will be treated as licensed under MPL-2.0. This is so that we can relicense to MPL-2.0 at some point in the future if we manage to move away from poppler as a backend (since that is the only dependency, at time of writing, which requires the GPLv3 license).

benches/rendering.rs

@@ -71,7 +71,7 @@ pub async fn render_first_page(path: impl AsRef<Path>) {
 	} = start_all_rendering(path);
 
 	// we only want to render until the first page is ready to be printed
-	while pages.iter().all(|p| p.is_none()) {
+	while pages.iter().all(Option::is_none) {
 		tokio::select! {
 			Some(renderer_msg) = from_render_rx.next() => {
 				handle_renderer_msg(renderer_msg, &mut pages, &mut to_converter_tx);
@@ -94,6 +94,7 @@ async fn render_all_files(path: &'static str) -> Vec<PageInfo> {
 
 	while let Some(info) = from_render_rx.next().await {
 		match info.expect("Renderer ran into an error while rendering") {
+			RenderInfo::Reloaded => (),
 			RenderInfo::NumPages(num) => fill_default(&mut pages, num),
 			RenderInfo::Page(page) => {
 				let num = page.page;
@@ -101,7 +102,7 @@ async fn render_all_files(path: &'static str) -> Vec<PageInfo> {
 			}
 		};
 
-		if pages.iter().all(|p| p.is_some()) {
+		if pages.iter().all(Option::is_some) {
 			break;
 		}
 	}
@@ -136,7 +137,7 @@ async fn convert_all_files(files: Vec<PageInfo>) {
 		}
 	}
 
-	while converted.iter().any(|p| p.is_none()) {
+	while converted.iter().any(Option::is_none) {
 		let page = from_converter_rx
 			.next()
 			.await
@@ -157,7 +158,7 @@ impl Profiler for CpuProfiler {
 	fn start_profiling(&mut self, benchmark_id: &str, _: &std::path::Path) {
 		let file = format!(
 			"./{}-{}.profile",
-			benchmark_id.replace("/", "-"),
+			benchmark_id.replace('/', "-"),
 			SystemTime::now()
 				.duration_since(UNIX_EPOCH)
 				.unwrap()

benches/utils.rs

@@ -21,6 +21,8 @@ pub fn handle_renderer_msg(
 			to_converter_tx.send(ConverterMsg::NumPages(num)).unwrap();
 		}
 		Ok(RenderInfo::Page(info)) => to_converter_tx.send(ConverterMsg::AddImg(info)).unwrap(),
+		// We can ignore the `Reloaded` variant 'cause that's only used to send info to the TUI
+		Ok(RenderInfo::Reloaded) => (),
 		Err(e) => panic!("Got error from renderer: {e:?}")
 	}
 }
@@ -75,7 +77,7 @@ pub fn start_rendering_loop(
 		width: columns * FONT_SIZE.0
 	};
 
-	std::thread::spawn(move || start_rendering(str_path, to_main_tx, from_main_rx, size));
+	std::thread::spawn(move || start_rendering(&str_path, to_main_tx, from_main_rx, size));
 
 	let main_area = Rect {
 		x: 0,
@@ -98,8 +100,8 @@ pub fn start_converting_loop(
 	let (to_converter_tx, from_main_rx) = unbounded();
 	let (to_main_tx, from_converter_rx) = unbounded();
 
-	let mut picker = Picker::new(FONT_SIZE);
-	picker.protocol_type = ProtocolType::Kitty;
+	let mut picker = Picker::from_fontsize(FONT_SIZE);
+	picker.set_protocol_type(ProtocolType::Kitty);
 
 	tokio::spawn(run_conversion_loop(
 		to_main_tx,
@@ -136,7 +138,7 @@ pub async fn render_doc(path: impl AsRef<Path>) {
 		to_render_tx
 	} = start_all_rendering(path);
 
-	while pages.is_empty() || pages.iter().any(|p| p.is_none()) {
+	while pages.is_empty() || pages.iter().any(Option::is_none) {
 		tokio::select! {
 			Some(renderer_msg) = from_render_rx.next() => {
 				handle_renderer_msg(renderer_msg, &mut pages, &mut to_converter_tx);

src/converter.rs

@@ -7,7 +7,7 @@ use ratatui_image::{picker::Picker, protocol::Protocol, Resize};
 use crate::renderer::{fill_default, PageInfo, RenderError};
 
 pub struct ConvertedPage {
-	pub page: Box<dyn Protocol>,
+	pub page: Protocol,
 	pub num: usize,
 	pub num_results: usize
 }

src/lib.rs

@@ -1,4 +1,5 @@
-#![feature(if_let_guard)]
+#[global_allocator]
+static ALLOC: mimalloc::MiMalloc = mimalloc::MiMalloc;
 
 pub mod converter;
 pub mod renderer;

src/main.rs

@@ -1,12 +1,10 @@
-#![feature(if_let_guard)]
-
 use std::{
+	ffi::OsString,
 	io::{stdout, Read, Write},
-	path::PathBuf,
-	str::FromStr
+	num::NonZeroUsize,
+	path::PathBuf
 };
 
-use converter::{run_conversion_loop, ConvertedPage, ConverterMsg};
 use crossterm::{
 	execute,
 	terminal::{
@@ -19,13 +17,11 @@ use glib::{LogField, LogLevel, LogWriterOutput};
 use notify::{Event, EventKind, RecursiveMode, Watcher};
 use ratatui::{backend::CrosstermBackend, Terminal};
 use ratatui_image::picker::Picker;
-use renderer::{RenderError, RenderInfo, RenderNotif};
-use tui::{InputAction, Tui};
-
-mod converter;
-mod renderer;
-mod skip;
-mod tui;
+use tdf::{
+	converter::{run_conversion_loop, ConvertedPage, ConverterMsg},
+	renderer::{self, RenderError, RenderInfo, RenderNotif},
+	tui::{BottomMessage, InputAction, MessageSetting, Tui}
+};
 
 // Dummy struct for easy errors in main
 #[derive(Debug)]
@@ -44,42 +40,47 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 	#[cfg(feature = "tracing")]
 	console_subscriber::init();
 
-	let file = std::env::args()
-		.nth(1)
-		.ok_or("Program requires a file to process")?;
-	let path = PathBuf::from_str(&file)?.canonicalize()?;
+	let flags = xflags::parse_or_exit! {
+		/// Display the pdf with the pages starting at the right hand size and moving left and
+		/// adjust input keys to match
+		optional -r,--r-to-l r_to_l: bool
+		/// The maximum number of pages to display together, horizontally, at a time
+		optional -m,--max-wide max_wide: NonZeroUsize
+		/// PDF file to read
+		required file: PathBuf
+	};
 
-	let (watch_tx, render_rx) = flume::unbounded();
-	let tui_tx = watch_tx.clone();
+	let path = flags.file.canonicalize()?;
+
+	let (watch_to_render_tx, render_rx) = flume::unbounded();
+	let tui_tx = watch_to_render_tx.clone();
 
 	let (render_tx, tui_rx) = flume::unbounded();
 	let watch_to_tui_tx = render_tx.clone();
 
-	// we need to call this outside the recommended_watcher call because if we call it inside, that
-	// will be calling it from a thread not owned by the tokio runtime (since it's created by
-	// calling thread::spawn) and that will cause a panic
-	let mut watcher = notify::recommended_watcher(move |res: notify::Result<Event>| match res {
-		// If we get an error here, and then an error sending, everything's going wrong. Just give
-		// up lol.
-		Err(e) => watch_to_tui_tx.send(Err(RenderError::Notify(e))).unwrap(),
-		// TODO: Should we match EventKind::Rename and propogate that so that the other parts of the
-		// process know that too? Or should that be
-		Ok(ev) => match ev.kind {
-			EventKind::Access(_) => (),
-			EventKind::Remove(_) =>
-				drop(watch_to_tui_tx.send(Err(RenderError::Render("File was deleted".into())))),
-			// This shouldn't fail to send unless the receiver gets disconnected. If that's
-			// happened, then like the main thread has panicked or something, so it doesn't matter
-			// we don't handle the error here.
-			EventKind::Other | EventKind::Any | EventKind::Create(_) | EventKind::Modify(_) =>
-				drop(watch_tx.send(renderer::RenderNotif::Reload)),
-		}
-	})?;
+	let mut watcher = notify::recommended_watcher(on_notify_ev(
+		watch_to_tui_tx,
+		watch_to_render_tx,
+		path.file_name()
+			.ok_or("Path does not have a last component??")?
+			.to_owned()
+	))?;
 
-	// We're making this nonrecursive 'cause we're just watching a single file, so there's nothing
-	// to recurse into
-	watcher.watch(&path, RecursiveMode::NonRecursive)?;
+	// So we have to watch the parent directory of the file that we are interested in because the
+	// `notify` library works on inodes, and if the file is deleted, that inode is gone as well,
+	// and then the notify library just gives up on trying to watch for the file reappearing. Imo
+	// they should start watching the parent directory if the file is deleted, and then wait for it
+	// to reappear and then begin watching it again, but whatever. It seems they've made their
+	// opinion on this clear
+	// (https://github.com/notify-rs/notify/issues/113#issuecomment-281836995) so whatever, guess
+	// we have to do this annoying workaround.
+	watcher.watch(
+		path.parent().expect("The root directory is not a PDF"),
+		RecursiveMode::NonRecursive
+	)?;
 
+	// TODO: Handle non-utf8 file names? Maybe by constructing a CString and passing that in to the
+	// poppler stuff instead of a rust string?
 	let file_path = format!("file://{}", path.clone().into_os_string().to_string_lossy());
 
 	let mut window_size = window_size()?;
@@ -96,7 +97,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 		// read in the returned size until we hit a 't' (which indicates to us it's done)
 		let input_vec = std::io::stdin()
 			.bytes()
-			.flat_map(|b| b.ok())
+			.filter_map(Result::ok)
 			.take_while(|b| *b != b't')
 			.collect::<Vec<_>>();
 
@@ -134,17 +135,13 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 
 	// We need to create `picker` on this thread because if we create it on the `renderer` thread,
 	// it messes up something with user input. Input never makes it to the crossterm thing
-	let mut picker = Picker::new((
-		window_size.width / window_size.columns,
-		window_size.height / window_size.rows
-	));
-	picker.guess_protocol();
+	let picker = Picker::from_query_stdio()?;
 
 	// then we want to spawn off the rendering task
 	// We need to use the thread::spawn API so that this exists in a thread not owned by tokio,
 	// since the methods we call in `start_rendering` will panic if called in an async context
 	std::thread::spawn(move || {
-		renderer::start_rendering(file_path, render_tx, render_rx, window_size)
+		renderer::start_rendering(&file_path, render_tx, render_rx, window_size)
 	});
 
 	let mut ev_stream = crossterm::event::EventStream::new();
@@ -154,12 +151,11 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 
 	tokio::spawn(run_conversion_loop(to_main, from_main, picker, 20));
 
-	let file_name = path
-		.file_name()
-		.map(|n| n.to_string_lossy())
-		.unwrap_or_else(|| "Unknown file".into())
-		.to_string();
-	let mut tui = tui::Tui::new(file_name);
+	let file_name = path.file_name().map_or_else(
+		|| "Unknown file".into(),
+		|n| n.to_string_lossy().to_string()
+	);
+	let mut tui = Tui::new(file_name, flags.max_wide, flags.r_to_l.unwrap_or_default());
 
 	let backend = CrosstermBackend::new(std::io::stdout());
 	let mut term = Terminal::new(backend)?;
@@ -177,23 +173,23 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 	)?;
 	enable_raw_mode()?;
 
-	let mut main_area = tui::Tui::main_layout(&term.get_frame());
+	let mut main_area = Tui::main_layout(&term.get_frame());
 	tui_tx.send(RenderNotif::Area(main_area[1]))?;
 
 	let mut tui_rx = tui_rx.into_stream();
 	let mut from_converter = from_converter.into_stream();
 
 	loop {
-		let mut needs_redraw = tokio::select! {
+		let mut needs_redraw = true;
+		tokio::select! {
 			// First we check if we have any keystrokes
 			Some(ev) = ev_stream.next().fuse() => {
 				// If we can't get user input, just crash.
 				let ev = ev.expect("Couldn't get any user input");
 
-				match tui.handle_event(ev) {
-					None => false,
-					Some(action) => {
-						match action {
+				match tui.handle_event(&ev) {
+					None => needs_redraw = false,
+					Some(action) => match action {
 						InputAction::Redraw => (),
 						InputAction::QuitApp => break,
 						InputAction::JumpingToPage(page) => {
@@ -201,17 +197,12 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 							to_converter.send(ConverterMsg::GoToPage(page))?;
 						},
 						InputAction::Search(term) => tui_tx.send(RenderNotif::Search(term))?,
-						};
-						true
 					}
 				}
 			},
 			Some(renderer_msg) = tui_rx.next() => {
 				match renderer_msg {
-					// if an Ok comes through, we know the error has been resolved ('cause it kinda
-					// bails whenever we run into an error) so just clear it
-					Ok(render_info) => {
-						match render_info {
+					Ok(render_info) => match render_info {
 						RenderInfo::NumPages(num) => {
 							tui.set_n_pages(num);
 							to_converter.send(ConverterMsg::NumPages(num))?;
@@ -220,19 +211,16 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 							tui.got_num_results_on_page(info.page, info.search_results);
 							to_converter.send(ConverterMsg::AddImg(info))?;
 						},
-						}
-						tui.set_bottom_msg(None);
+						RenderInfo::Reloaded => tui.set_msg(MessageSetting::Some(BottomMessage::Reloaded)),
 					},
 					Err(e) => tui.show_error(e),
 				}
-				true
 			}
 			Some(img_res) = from_converter.next() => {
 				match img_res {
 					Ok(ConvertedPage { page, num, num_results }) => tui.page_ready(page, num, num_results),
 					Err(e) => tui.show_error(e),
 				}
-				true
 			},
 		};
 
@@ -261,6 +249,44 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 	Ok(())
 }
 
+fn on_notify_ev(
+	to_tui_tx: flume::Sender<Result<RenderInfo, RenderError>>,
+	to_render_tx: flume::Sender<RenderNotif>,
+	file_name: OsString
+) -> impl Fn(notify::Result<Event>) {
+	move |res| match res {
+		// If we get an error here, and then an error sending, everything's going wrong. Just give
+		// up lol.
+		Err(e) => to_tui_tx.send(Err(RenderError::Notify(e))).unwrap(),
+		// TODO: Should we match EventKind::Rename and propogate that so that the other parts of the
+		// process know that too? Or should that be
+		Ok(ev) => {
+			// We only watch the parent directory (see the comment above `watcher.watch` in `fn
+			// main`) so we need to filter out events to only ones that pertain to the single file
+			// we care about
+			if !ev
+				.paths
+				.iter()
+				.any(|path| path.file_name().is_some_and(|f| f == file_name))
+			{
+				return;
+			}
+
+			match ev.kind {
+				EventKind::Access(_) => (),
+				EventKind::Remove(_) => to_tui_tx
+					.send(Err(RenderError::Render("File was deleted".into())))
+					.unwrap(),
+				// This shouldn't fail to send unless the receiver gets disconnected. If that's
+				// happened, then like the main thread has panicked or something, so it doesn't matter
+				// we don't handle the error here.
+				EventKind::Other | EventKind::Any | EventKind::Create(_) | EventKind::Modify(_) =>
+					to_render_tx.send(RenderNotif::Reload).unwrap(),
+			}
+		}
+	}
+}
+
 fn noop(_: LogLevel, _: &[LogField<'_>]) -> LogWriterOutput {
 	LogWriterOutput::Handled
 }

src/renderer.rs

@@ -25,7 +25,8 @@ pub enum RenderError {
 
 pub enum RenderInfo {
 	NumPages(usize),
-	Page(PageInfo)
+	Page(PageInfo),
+	Reloaded
 }
 
 #[derive(Clone)]
@@ -64,21 +65,23 @@ pub fn fill_default<T: Default>(vec: &mut Vec<T>, size: usize) {
 // Also we just kinda 'unwrap' all of the send/recv calls here 'cause if they return an error, that
 // means the other side's disconnected, which means that the main thread has panicked, which means
 // we're done.
+// We're allowing passing by value here because this is only called once, at the beginning of the
+// program, and the arguments that 'should' be passed by value (`receiver` and `size`) would
+// probably be more performant if accessed by-value instead of through a reference. Probably.
+#[allow(clippy::needless_pass_by_value)]
 pub fn start_rendering(
-	path: String,
+	path: &str,
 	mut sender: Sender<Result<RenderInfo, RenderError>>,
 	receiver: Receiver<RenderNotif>,
 	size: WindowSize
 ) -> Result<(), SendError<Result<RenderInfo, RenderError>>> {
 	// first, wait 'til we get told what the current starting area is so that we can set it to
 	// know what to render to
-	let mut area;
-	loop {
+	let mut area = loop {
 		if let RenderNotif::Area(r) = receiver.recv().unwrap() {
-			area = r;
-			break;
-		}
+			break r;
 		}
+	};
 
 	// We want this outside of 'reload so that if the doc reloads, the search term that somebody
 	// set will still get highlighted in the reloaded doc
@@ -89,11 +92,17 @@ pub fn start_rendering(
 	let col_w = size.width / size.columns;
 	let col_h = size.height / size.rows;
 
+	let mut stored_doc = None;
+
 	'reload: loop {
-		let doc = match Document::from_file(&path, None) {
+		let doc = match Document::from_file(path, None) {
 			Err(e) => {
 				// if there's an error, tell the main loop
 				sender.send(Err(RenderError::Doc(e)))?;
+
+				match stored_doc {
+					Some(ref d) => d,
+					None => {
 						// then wait for a reload notif (since what probably happened is that the file was
 						// temporarily removed to facilitate a save or something like that)
 						while let Ok(msg) = receiver.recv() {
@@ -106,7 +115,14 @@ pub fn start_rendering(
 						// done, so we're fine to just return
 						return Ok(());
 					}
-			Ok(d) => d
+				}
+			}
+			Ok(d) => {
+				if stored_doc.is_some() {
+					sender.send(Ok(RenderInfo::Reloaded))?;
+				}
+				&*stored_doc.insert(d)
+			}
 		};
 
 		let n_pages = doc.n_pages() as usize;
@@ -189,8 +205,8 @@ pub fn start_rendering(
 						.map(|(idx, p)| (start_point - (idx + 1), p))
 				);
 
-			let area_w = area.width as f64 * col_w as f64;
-			let area_h = area.height as f64 * col_h as f64;
+			let area_w = f64::from(area.width) * f64::from(col_w);
+			let area_h = f64::from(area.height) * f64::from(col_h);
 
 			// we go through each page
 			for (num, rendered) in page_iter {
@@ -227,8 +243,8 @@ pub fn start_rendering(
 
 				// render the page
 				match render_single_page_to_ctx(
-					page,
-					&search_term,
+					&page,
+					search_term.as_deref(),
 					rendered_with_no_results,
 					(area_w, area_h)
 				) {
@@ -251,11 +267,11 @@ pub fn start_rendering(
 						// since the effects of parallelizing that will be noticeable if the user
 						// tries to move through pages more quickly
 						if num == start_point {
-							render_ctx_to_png(ctx, &mut sender, (col_w, col_h), num)?;
+							render_ctx_to_png(&ctx, &mut sender, (col_w, col_h), num)?;
 						} else {
 							let mut sender = sender.clone();
 							thread::spawn(move || {
-								render_ctx_to_png(ctx, &mut sender, (col_w, col_h), num)
+								render_ctx_to_png(&ctx, &mut sender, (col_w, col_h), num)
 							});
 						}
 					}
@@ -298,8 +314,8 @@ struct RenderedContext {
 unsafe impl Send for RenderedContext {}
 
 fn render_single_page_to_ctx(
-	page: Page,
-	search_term: &Option<String>,
+	page: &Page,
+	search_term: Option<&str>,
 	already_rendered_no_results: bool,
 	(area_w, area_h): (f64, f64)
 ) -> Result<Option<RenderedContext>, String> {
@@ -375,7 +391,7 @@ fn render_single_page_to_ctx(
 		highlight_color.set_green((u16::MAX / 5) * 4);
 
 		let mut old_rect = Rectangle::new();
-		for rect in result_rects.iter_mut() {
+		for rect in &mut result_rects {
 			// According to https://gitlab.freedesktop.org/poppler/poppler/-/issues/763, these rects
 			// need to be corrected since they use different references as the y-coordinate base
 			rect.set_y1(p_height - rect.y1());
@@ -401,7 +417,7 @@ fn render_single_page_to_ctx(
 }
 
 fn render_ctx_to_png(
-	ctx: RenderedContext,
+	ctx: &RenderedContext,
 	sender: &mut Sender<Result<RenderInfo, RenderError>>,
 	(col_w, col_h): (u16, u16),
 	page: usize

src/tui.rs

@@ -1,4 +1,4 @@
-use std::{io::stdout, rc::Rc};
+use std::{borrow::Cow, io::stdout, num::NonZeroUsize, rc::Rc};
 
 use crossterm::{
 	event::{Event, KeyCode, MouseEventKind},
@@ -24,7 +24,8 @@ pub struct Tui {
 	// we use `prev_msg` to, for example, restore the 'search results' message on the bottom after
 	// jumping to a specific page
 	prev_msg: Option<BottomMessage>,
-	rendered: Vec<RenderedInfo>
+	rendered: Vec<RenderedInfo>,
+	page_constraints: PageConstraints
 }
 
 #[derive(Default, Debug)]
@@ -42,7 +43,8 @@ pub enum BottomMessage {
 	Help,
 	SearchResults(String),
 	Error(String),
-	Input(InputCommand)
+	Input(InputCommand),
+	Reloaded
 }
 
 pub enum InputCommand {
@@ -50,12 +52,17 @@ pub enum InputCommand {
 	Search(String)
 }
 
+struct PageConstraints {
+	max_wide: Option<NonZeroUsize>,
+	r_to_l: bool
+}
+
 // This seems like a kinda weird struct because it holds two optionals but any representation
 // within it is valid; I think it's the best way to represent it
 #[derive(Default)]
 struct RenderedInfo {
 	// The image, if it has been rendered by `Converter` to that struct
-	img: Option<Box<dyn Protocol>>,
+	img: Option<Protocol>,
 	// The number of results for the current search term that have been found on this page. None if
 	// we haven't checked this page yet
 	// Also this isn't the most efficient representation of this value, but it's accurate, so like
@@ -64,14 +71,15 @@ struct RenderedInfo {
 }
 
 impl Tui {
-	pub fn new(name: String) -> Tui {
+	pub fn new(name: String, max_wide: Option<NonZeroUsize>, r_to_l: bool) -> Tui {
 		Self {
 			name,
 			page: 0,
 			prev_msg: None,
 			bottom_msg: BottomMessage::Help,
 			last_render: LastRender::default(),
-			rendered: vec![]
+			rendered: vec![],
+			page_constraints: PageConstraints { max_wide, r_to_l }
 		}
 	}
 
@@ -145,18 +153,18 @@ impl Tui {
 		let rendered_span = Span::styled(&rendered_str, Style::new().fg(Color::Cyan));
 		frame.render_widget(rendered_span, bottom_layout[1]);
 
-		let (msg_str, color) = match self.bottom_msg {
+		let (msg_str, color): (Cow<'_, str>, _) = match self.bottom_msg {
 			BottomMessage::Help => (
-				"/: Search, g: Go To Page, n: Next Search Result, N: Previous Search Result"
-					.to_string(),
+				"/: Search, g: Go To Page, n: Next Search Result, N: Previous Search Result".into(),
 				Color::Blue
 			),
-			BottomMessage::Error(ref e) => (format!("Couldn't render a page: {e}"), Color::Red),
+			BottomMessage::Error(ref e) => (e.as_str().into(), Color::Red),
 			BottomMessage::Input(ref input_state) => (
 				match input_state {
 					InputCommand::GoToPage(page) => format!("Go to: {page}"),
 					InputCommand::Search(s) => format!("Search: {s}")
-				},
+				}
+				.into(),
 				Color::Blue
 			),
 			BottomMessage::SearchResults(ref term) => {
@@ -174,10 +182,12 @@ impl Tui {
 					format!(
 						"Results for '{term}': {num_found} (searched: {}%)",
 						num_searched / self.rendered.len()
-					),
+					)
+					.into(),
 					Color::Blue
 				)
 			}
+			BottomMessage::Reloaded => ("Document was reloaded!".into(), Color::Blue)
 		};
 
 		let span = Span::styled(msg_str, Style::new().fg(color));
@@ -196,15 +206,21 @@ impl Tui {
 			// here we calculate how many pages can fit in the available area.
 			let mut test_area_w = img_area.width;
 			// go through our pages, starting at the first one we want to view
-			let page_widths = self.rendered[self.page..]
+			let mut page_widths = self.rendered[self.page..]
 				.iter()
 				// and get their indices (I know it's offset, we fix it down below when we actually
 				// render each page)
 				.enumerate()
 				// and only take as many as are ready to be rendered
-				.take_while(|(_, page)| page.img.is_some())
+				.take_while(|(idx, page)| {
+					let mut take = page.img.is_some();
+					if let Some(max) = self.page_constraints.max_wide {
+						take &= *idx < max.get();
+					}
+					take
+				})
 				// and map it to their width (in cells on the terminal, not pixels)
-				.flat_map(|(idx, page)| page.img.as_ref().map(|img| (idx, img.rect().width)))
+				.filter_map(|(idx, page)| page.img.as_ref().map(|img| (idx, img.rect().width)))
 				// and then take them as long as they won't overflow the available area.
 				.take_while(|(_, width)| match test_area_w.checked_sub(*width) {
 					Some(new_val) => {
@@ -215,6 +231,10 @@ impl Tui {
 				})
 				.collect::<Vec<_>>();
 
+			if self.page_constraints.r_to_l {
+				page_widths.reverse();
+			}
+
 			if page_widths.is_empty() {
 				// If none are ready to render, just show the loading thing
 				Self::render_loading_in(frame, img_area);
@@ -252,7 +272,7 @@ impl Tui {
 
 	fn render_single_page(&mut self, frame: &mut Frame<'_>, page_idx: usize, img_area: Rect) {
 		match self.rendered[page_idx].img {
-			Some(ref page_img) => frame.render_widget(Image::new(&**page_img), img_area),
+			Some(ref page_img) => frame.render_widget(Image::new(page_img), img_area),
 			None => Self::render_loading_in(frame, img_area)
 		};
 	}
@@ -268,12 +288,23 @@ impl Tui {
 		frame.render_widget(loading_span, inner_space[0]);
 	}
 
-	fn change_page(&mut self, change: PageChange, amt: ChangeAmount) -> Option<InputAction> {
+	fn change_page(&mut self, mut change: PageChange, amt: ChangeAmount) -> Option<InputAction> {
 		let diff = match amt {
 			ChangeAmount::Single => 1,
 			ChangeAmount::WholeScreen => self.last_render.pages_shown
 		};
 
+		// This is a kinda weird way to switch around the controls for this sort of thing but it
+		// allows it to be pretty centralized and avoids annoyingly duplicated match arms (since
+		// we'd have to do `match key { 'h' if r_to_l | 'l' => {}}` and that doesn't play well with
+		// `if` guards on match arms)
+		if self.page_constraints.r_to_l {
+			change = match change {
+				PageChange::Next => PageChange::Prev,
+				PageChange::Prev => PageChange::Next
+			};
+		}
+
 		let old = self.page;
 		match change {
 			PageChange::Next => self.set_page((self.page + diff).min(self.rendered.len() - 1)),
@@ -293,7 +324,7 @@ impl Tui {
 		self.page = self.page.min(n_pages - 1);
 	}
 
-	pub fn page_ready(&mut self, img: Box<dyn Protocol>, page_num: usize, num_results: usize) {
+	pub fn page_ready(&mut self, img: Protocol, page_num: usize, num_results: usize) {
 		// If this new image woulda fit within the available space on the last render AND it's
 		// within the range where it might've been rendered with the last shown pages, then reset
 		// the last rect marker so that all images are forced to redraw on next render and this one
@@ -323,7 +354,7 @@ impl Tui {
 		self.rendered[page_num].num_results = Some(num_results);
 	}
 
-	pub fn handle_event(&mut self, ev: Event) -> Option<InputAction> {
+	pub fn handle_event(&mut self, ev: &Event) -> Option<InputAction> {
 		fn jump_to_page(
 			page: &mut usize,
 			rect: &mut Rect,
@@ -340,54 +371,43 @@ impl Tui {
 		match ev {
 			Event::Key(key) => {
 				match key.code {
-					KeyCode::Char(c)
+					KeyCode::Char(c) => {
+						// TODO: refactor back to `if let` arm guards when those are stabilized
 						if let BottomMessage::Input(InputCommand::Search(ref mut term)) =
-							self.bottom_msg =>
+							self.bottom_msg
 						{
 							term.push(c);
-						Some(InputAction::Redraw)
+							return Some(InputAction::Redraw);
 						}
-					KeyCode::Backspace
-						if let BottomMessage::Input(InputCommand::Search(ref mut term)) =
-							self.bottom_msg =>
-					{
-						term.pop();
-						Some(InputAction::Redraw)
-					}
-					KeyCode::Char(c)
+
 						if let BottomMessage::Input(InputCommand::GoToPage(ref mut page)) =
-							self.bottom_msg =>
-						c.to_digit(10).map(|input_num| {
+							self.bottom_msg
+						{
+							return c.to_digit(10).map(|input_num| {
 								*page = (*page * 10) + input_num as usize;
 								InputAction::Redraw
-						}),
-					KeyCode::Right | KeyCode::Char('l') =>
-						self.change_page(PageChange::Next, ChangeAmount::Single),
-					KeyCode::Down | KeyCode::Char('j') =>
-						self.change_page(PageChange::Next, ChangeAmount::WholeScreen),
-					KeyCode::Left | KeyCode::Char('h') =>
-						self.change_page(PageChange::Prev, ChangeAmount::Single),
-					KeyCode::Up | KeyCode::Char('k') =>
-						self.change_page(PageChange::Prev, ChangeAmount::WholeScreen),
-					KeyCode::Esc => match self.bottom_msg {
-						BottomMessage::Input(_) => {
-							self.set_bottom_msg(None);
+							});
+						}
+
+						match c {
+							'l' => self.change_page(PageChange::Next, ChangeAmount::Single),
+							'j' => self.change_page(PageChange::Next, ChangeAmount::WholeScreen),
+							'h' => self.change_page(PageChange::Prev, ChangeAmount::Single),
+							'k' => self.change_page(PageChange::Prev, ChangeAmount::WholeScreen),
+							'q' => Some(InputAction::QuitApp),
+							'g' => {
+								self.set_msg(MessageSetting::Some(BottomMessage::Input(
+									InputCommand::GoToPage(0)
+								)));
 								Some(InputAction::Redraw)
 							}
-						_ => Some(InputAction::QuitApp)
-					},
-					KeyCode::Char('q') => Some(InputAction::QuitApp),
-					KeyCode::Char('g') => {
-						self.set_bottom_msg(Some(BottomMessage::Input(InputCommand::GoToPage(0))));
+							'/' => {
+								self.set_msg(MessageSetting::Some(BottomMessage::Input(
+									InputCommand::Search(String::new())
+								)));
 								Some(InputAction::Redraw)
 							}
-					KeyCode::Char('/') => {
-						self.set_bottom_msg(Some(BottomMessage::Input(InputCommand::Search(
-							String::new()
-						))));
-						Some(InputAction::Redraw)
-					}
-					KeyCode::Char('n') if self.page < self.rendered.len() - 1 => {
+							'n' if self.page < self.rendered.len() - 1 => {
 								// TODO: If we can't find one, then maybe like block until we've verified
 								// all the pages have been checked?
 								let next_page = self.rendered[(self.page + 1)..]
@@ -401,7 +421,7 @@ impl Tui {
 
 								jump_to_page(&mut self.page, &mut self.last_render.rect, next_page)
 							}
-					KeyCode::Char('N') if self.page > 0 => {
+							'N' if self.page > 0 => {
 								let prev_page = self.rendered[..(self.page)]
 									.iter()
 									.rev()
@@ -414,27 +434,56 @@ impl Tui {
 
 								jump_to_page(&mut self.page, &mut self.last_render.rect, prev_page)
 							}
+							_ => None
+						}
+					}
+					KeyCode::Backspace => {
+						if let BottomMessage::Input(InputCommand::Search(ref mut term)) =
+							self.bottom_msg
+						{
+							term.pop();
+							return Some(InputAction::Redraw);
+						}
+						None
+					}
+					KeyCode::Right => self.change_page(PageChange::Next, ChangeAmount::Single),
+					KeyCode::Down => self.change_page(PageChange::Next, ChangeAmount::WholeScreen),
+					KeyCode::Left => self.change_page(PageChange::Prev, ChangeAmount::Single),
+					KeyCode::Up => self.change_page(PageChange::Prev, ChangeAmount::WholeScreen),
+					KeyCode::Esc => match self.bottom_msg {
+						BottomMessage::Help => Some(InputAction::QuitApp),
+						_ => {
+							// When we hit escape, we just want to pop off the current message and
+							// show the underlying one.
+							self.set_msg(MessageSetting::Pop);
+							Some(InputAction::Redraw)
+						}
+					},
 					KeyCode::Enter => {
-						let BottomMessage::Input(_) = self.bottom_msg else {
+						let mut default = BottomMessage::default();
+						std::mem::swap(&mut self.bottom_msg, &mut default);
+						let BottomMessage::Input(ref cmd) = default else {
+							std::mem::swap(&mut self.bottom_msg, &mut default);
 							return None;
 						};
 
-						self.set_bottom_msg(None);
-						let Some(BottomMessage::Input(ref cmd)) = self.prev_msg else {
-							// We need to verify it's an input msg currently, and only then take it
-							// and replace it by a default Help message. Don't exactly know how to
-							// do this otherwise.
-							unreachable!();
-						};
-
 						match cmd {
 							// Only forward the command if it's within range
 							InputCommand::GoToPage(page) => {
-								let page = *page;
-								(page < self.rendered.len()).then(|| {
-									self.set_page(page);
-									InputAction::JumpingToPage(page)
-								})
+								// We need to subtract 1 b/c they're tracked internally as
+								// 0-indexed but input and displayed as 1-indexed
+								let zero_page = page.saturating_sub(1);
+								let rendered_len = self.rendered.len();
+
+								if zero_page < rendered_len {
+									self.set_page(zero_page);
+									Some(InputAction::JumpingToPage(zero_page))
+								} else {
+									self.set_msg(MessageSetting::Some(BottomMessage::Error(
+										format!("Cannot jump to page {page}; there are only {rendered_len} pages in the document")
+									)));
+									Some(InputAction::Redraw)
+								}
 							}
 							InputCommand::Search(term) => {
 								let term = term.clone();
@@ -442,14 +491,14 @@ impl Tui {
 								// We only want to show search results if there would actually be
 								// data to show
 								if !term.is_empty() {
-									self.set_bottom_msg(Some(BottomMessage::SearchResults(
-										term.clone()
-									)));
+									self.set_msg(MessageSetting::Some(
+										BottomMessage::SearchResults(term.clone())
+									));
 								} else {
 									// else, if it's not empty, we just want to reset the bottom
 									// area to show the default data; we don't want it to like show
 									// the data from a previous search
-									self.set_bottom_msg(Some(BottomMessage::Help));
+									self.set_msg(MessageSetting::Reset);
 								}
 
 								// Reset all the search results
@@ -483,7 +532,7 @@ impl Tui {
 	}
 
 	pub fn show_error(&mut self, err: RenderError) {
-		self.set_bottom_msg(Some(BottomMessage::Error(match err {
+		self.set_msg(MessageSetting::Some(BottomMessage::Error(match err {
 			RenderError::Notify(e) => format!("Auto-reload failed: {e}"),
 			RenderError::Doc(e) => format!("Couldn't open document: {e}"),
 			RenderError::Render(e) => format!("Couldn't render page: {e}")
@@ -500,17 +549,18 @@ impl Tui {
 
 	// We have `msg` as optional so that if they reset it to none, it'll replace it with
 	// `prev_msg`, but if they reset it to something else, it'll put the current thing in prev_msg
-	pub fn set_bottom_msg(&mut self, msg: Option<BottomMessage>) {
+	pub fn set_msg(&mut self, msg: MessageSetting) {
 		match msg {
-			Some(mut msg) => {
+			MessageSetting::Some(mut msg) => {
 				std::mem::swap(&mut self.bottom_msg, &mut msg);
 				self.prev_msg = Some(msg);
 			}
-			None => {
-				let mut new_bottom = self.prev_msg.take().unwrap_or_default();
-				std::mem::swap(&mut self.bottom_msg, &mut new_bottom);
-				self.prev_msg = Some(new_bottom);
+			MessageSetting::Default => self.set_msg(MessageSetting::Some(BottomMessage::default())),
+			MessageSetting::Reset => {
+				self.prev_msg = None;
+				self.bottom_msg = BottomMessage::default();
 			}
+			MessageSetting::Pop => self.bottom_msg = self.prev_msg.take().unwrap_or_default()
 		}
 	}
 }
@@ -522,12 +572,21 @@ pub enum InputAction {
 	QuitApp
 }
 
+#[derive(Copy, Clone)]
 enum PageChange {
 	Prev,
 	Next
 }
 
+#[derive(Copy, Clone)]
 enum ChangeAmount {
 	WholeScreen,
 	Single
 }
+
+pub enum MessageSetting {
+	Some(BottomMessage),
+	Default,
+	Reset,
+	Pop
+}