Add --r-to-l and --max-wide flags to cli args

CHANGELOG.md

@@ -1,3 +1,9 @@
+# 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
+
 # v0.1.0
 
 Initial tag :)

Cargo.lock

@@ -40,15 +40,15 @@ checksum = "4b46cbb362ab8752921c97e041f5e366ee6297bd428a31275b9fcf1e380f7299"
 
 [[package]]
 name = "anstyle"
-version = "1.0.9"
+version = "1.0.10"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8365de52b16c035ff4fcafe0092ba9390540e3e352870ac09933bebcaa2c8c56"
+checksum = "55cc3b69f167a1ef2e161439aa98aed94e6028e5f9a59be9a6ffb47aef1651f9"
 
 [[package]]
 name = "anyhow"
-version = "1.0.91"
+version = "1.0.92"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c042108f3ed77fd83760a5fd79b53be043192bb3b9dba91d8c574c0ada7850c8"
+checksum = "74f37166d7d48a0284b99dd824694c26119c700b53bf0d1540cdb147dbdaaf13"
 
 [[package]]
 name = "async-stream"
@@ -213,9 +213,9 @@ checksum = "9ac0150caa2ae65ca5bd83f25c7de183dea78d4d366469f148435e2acfbad0da"
 
 [[package]]
 name = "cairo-rs"
-version = "0.20.1"
+version = "0.20.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "e8a0ea147c94108c9613235388f540e4d14c327f7081c9e471fc8ee8a2533e69"
+checksum = "d7fa699e1d7ae691001a811dda5ef0e3e42e1d4119b26426352989df9e94e3e6"
 dependencies = [
  "bitflags 2.6.0",
  "cairo-sys-rs",
@@ -540,9 +540,9 @@ dependencies = [
 
 [[package]]
 name = "fdeflate"
-version = "0.3.5"
+version = "0.3.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d8090f921a24b04994d9929e204f50b498a33ea6ba559ffaa05e04f7ee7fb5ab"
+checksum = "07c6f4c64c1d33a3111c4466f7365ebdcc37c5bd1ea0d62aae2e3d722aacbedb"
 dependencies = [
  "simd-adler32",
 ]
@@ -705,9 +705,9 @@ checksum = "07e28edb80900c19c28f1072f2e8aeca7fa06b23cd4169cefe1af5aa3260783f"
 
 [[package]]
 name = "gio"
-version = "0.20.4"
+version = "0.20.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b8d999e8fb09583e96080867e364bc1e701284ad206c76a5af480d63833ad43c"
+checksum = "d8569975884fdfdbed536b682448fbd8c70bafbd69cac2d45eb1a7a372702241"
 dependencies = [
  "futures-channel",
  "futures-core",
@@ -722,9 +722,9 @@ dependencies = [
 
 [[package]]
 name = "gio-sys"
-version = "0.20.4"
+version = "0.20.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "4f7efc368de04755344f0084104835b6bb71df2c1d41e37d863947392a894779"
+checksum = "217f464cad5946ae4369c355155e2d16b488c08920601083cb4891e352ae777b"
 dependencies = [
  "glib-sys",
  "gobject-sys",
@@ -735,9 +735,9 @@ dependencies = [
 
 [[package]]
 name = "glib"
-version = "0.20.4"
+version = "0.20.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "adcf1ec6d3650bf9fdbc6cee242d4fcebc6f6bfd9bea5b929b6a8b7344eb85ff"
+checksum = "358431b0e0eb15b9d02db52e1f19c805b953c5c168099deb3de88beab761768c"
 dependencies = [
  "bitflags 2.6.0",
  "futures-channel",
@@ -756,9 +756,9 @@ dependencies = [
 
 [[package]]
 name = "glib-macros"
-version = "0.20.4"
+version = "0.20.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a6bf88f70cd5720a6197639dcabcb378dd528d0cb68cb1f45e3b358bcb841cd7"
+checksum = "e7d21ca27acfc3e91da70456edde144b4ac7c36f78ee77b10189b3eb4901c156"
 dependencies = [
  "heck",
  "proc-macro-crate",
@@ -769,9 +769,9 @@ dependencies = [
 
 [[package]]
 name = "glib-sys"
-version = "0.20.4"
+version = "0.20.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5f9eca5d88cfa6a453b00d203287c34a2b7cac3a7831779aa2bb0b3c7233752b"
+checksum = "8a5911863ab7ecd4a6f8d5976f12eeba076b23669c49b066d877e742544aa389"
 dependencies = [
  "libc",
  "system-deps",
@@ -940,9 +940,9 @@ dependencies = [
 
 [[package]]
 name = "hyper-timeout"
-version = "0.5.1"
+version = "0.5.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3203a961e5c83b6f5498933e78b6b263e208c197b63e9c6c53cc82ffd3f63793"
+checksum = "2b90d566bffbce6a75bd8b09a05aa8c2cb1fabb6cb348f8840c9e4c90a0d83b0"
 dependencies = [
  "hyper",
  "hyper-util",
@@ -953,9 +953,9 @@ dependencies = [
 
 [[package]]
 name = "hyper-util"
-version = "0.1.9"
+version = "0.1.10"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "41296eb09f183ac68eec06e03cdbea2e759633d4067b2f6552fc2e009bcad08b"
+checksum = "df2dcfbe0677734ab2f3ffa7fa7bfd4706bfdc1ef393f2ee30184aed67e631b4"
 dependencies = [
  "bytes",
  "futures-channel",
@@ -1654,9 +1654,9 @@ checksum = "719b953e2095829ee67db738b3bfa9fa368c94900df327b3f07fe6e794d2fe1f"
 
 [[package]]
 name = "rustix"
-version = "0.38.37"
+version = "0.38.38"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8acb788b847c24f28525660c4d7758620a7210875711f79e7f663cc152726811"
+checksum = "aa260229e6538e52293eeb577aabd09945a09d6d9cc0fc550ed7529056c2e32a"
 dependencies = [
  "bitflags 2.6.0",
  "errno",
@@ -1694,18 +1694,18 @@ checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49"
 
 [[package]]
 name = "serde"
-version = "1.0.213"
+version = "1.0.214"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3ea7893ff5e2466df8d720bb615088341b295f849602c6956047f8f80f0e9bc1"
+checksum = "f55c3193aca71c12ad7890f1785d2b73e1b9f63a0bbc353c08ef26fe03fc56b5"
 dependencies = [
  "serde_derive",
 ]
 
 [[package]]
 name = "serde_derive"
-version = "1.0.213"
+version = "1.0.214"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7e85ad2009c50b58e87caa8cd6dac16bdf511bbfb7af6c33df902396aa480fa5"
+checksum = "de523f781f095e28fa605cdce0f8307e451cc0fd14e2eb4cd2e98a355b147766"
 dependencies = [
  "proc-macro2",
  "quote",
@@ -1842,9 +1842,9 @@ dependencies = [
 
 [[package]]
 name = "syn"
-version = "2.0.85"
+version = "2.0.87"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5023162dfcd14ef8f32034d8bcd4cc5ddc61ef7a247c024a33e24e1f24d21b56"
+checksum = "25aa4ce346d03a6dcd68dd8b4010bcb74e54e62c90c573f394c46eae99aba32d"
 dependencies = [
  "proc-macro2",
  "quote",
@@ -1901,6 +1901,7 @@ dependencies = [
  "ratatui",
  "ratatui-image",
  "tokio",
+ "xflags",
 ]
 
 [[package]]
@@ -2402,6 +2403,21 @@ dependencies = [
  "memchr",
 ]
 
+[[package]]
+name = "xflags"
+version = "0.4.0-pre.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f6a40f95e4e200baabdfe8b813e3ee754b58407a677141bd2890c28ef4a89c21"
+dependencies = [
+ "xflags-macros",
+]
+
+[[package]]
+name = "xflags-macros"
+version = "0.4.0-pre.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8a6d9b56f406f5754a3808524166b6e6bdfe219c0526e490cfc39ecc0582a4e6"
+
 [[package]]
 name = "zerocopy"
 version = "0.7.35"

Cargo.toml

@@ -36,6 +36,7 @@ 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"
 
 # for tracing with tokio-console
 console-subscriber = { version = "0.4.0", optional = true }

src/main.rs

@@ -2,8 +2,8 @@
 
 use std::{
 	io::{stdout, Read, Write},
-	path::PathBuf,
+	num::NonZeroUsize,
-	str::FromStr
+	path::PathBuf
 };
 
 use converter::{run_conversion_loop, ConvertedPage, ConverterMsg};
@@ -44,42 +44,33 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 	#[cfg(feature = "tracing")]
 	console_subscriber::init();
 
-	let file = std::env::args()
+	let flags = xflags::parse_or_exit! {
-		.nth(1)
+		/// Display the pdf with the pages starting at the right hand size and moving left and
-		.ok_or("Program requires a file to process")?;
+		/// adjust input keys to match
-	let path = PathBuf::from_str(&file)?.canonicalize()?;
+		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 path = flags.file.canonicalize()?;
-	let tui_tx = watch_tx.clone();
+
+	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
+	let mut watcher =
-	// will be calling it from a thread not owned by the tokio runtime (since it's created by
+		notify::recommended_watcher(on_notify_ev(watch_to_tui_tx, watch_to_render_tx))?;
-	// 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)),
-		}
-	})?;
 
 	// 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)?;
 
+	// 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()?;
@@ -158,7 +149,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 		|| "Unknown file".into(),
 		|n| n.to_string_lossy().to_string()
 	);
-	let mut tui = tui::Tui::new(file_name);
+	let mut tui = 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)?;
@@ -183,16 +174,16 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 	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) {
+				match tui.handle_event(&ev) {
-					None => false,
+					None => needs_redraw = false,
-					Some(action) => {
+					Some(action) => match action {
-						match action {
 						InputAction::Redraw => (),
 						InputAction::QuitApp => break,
 						InputAction::JumpingToPage(page) => {
@@ -200,8 +191,6 @@ 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
 					}
 				}
 			},
@@ -224,14 +213,12 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 					},
 					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
 			},
 		};
 
@@ -260,6 +247,29 @@ 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>
+) -> 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) => match ev.kind {
+			EventKind::Access(_) => (),
+			EventKind::Remove(_) =>
+				drop(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(to_render_tx.send(renderer::RenderNotif::Reload)),
+		}
+	}
+}
+
 fn noop(_: LogLevel, _: &[LogField<'_>]) -> LogWriterOutput {
 	LogWriterOutput::Handled
 }

src/renderer.rs

@@ -64,6 +64,10 @@ 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: &str,
 	mut sender: Sender<Result<RenderInfo, RenderError>>,
@@ -227,8 +231,8 @@ pub fn start_rendering(
 
 				// render the page
 				match render_single_page_to_ctx(
-					page,
+					&page,
-					&search_term,
+					search_term.as_deref(),
 					rendered_with_no_results,
 					(area_w, area_h)
 				) {
@@ -251,11 +255,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 +302,8 @@ struct RenderedContext {
 unsafe impl Send for RenderedContext {}
 
 fn render_single_page_to_ctx(
-	page: Page,
+	page: &Page,
-	search_term: &Option<String>,
+	search_term: Option<&str>,
 	already_rendered_no_results: bool,
 	(area_w, area_h): (f64, f64)
 ) -> Result<Option<RenderedContext>, String> {
@@ -375,7 +379,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 +405,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::{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)]
@@ -50,6 +51,11 @@ 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)]
@@ -64,14 +70,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 }
 		}
 	}
 
@@ -196,13 +203,19 @@ 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)
 				.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.
@@ -215,6 +228,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);
@@ -268,12 +285,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)),
@@ -323,7 +351,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,
@@ -522,11 +550,13 @@ pub enum InputAction {
 	QuitApp
 }
 
+#[derive(Copy, Clone)]
 enum PageChange {
 	Prev,
 	Next
 }
 
+#[derive(Copy, Clone)]
 enum ChangeAmount {
 	WholeScreen,
 	Single