diff --git a/README.md b/README.md index 8643cbc..7a61d65 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,7 @@ Bug reports and pull requests are welcome. Read [CONTRIBUTING.md](https://github Thanks to everyone who has sent a pull request: - [@mariusrueve](https://github.com/mariusrueve) for the Conda and Pixi instructions ([#9](https://github.com/borghei/ink/pull/9)) and the [conda-forge package](https://github.com/conda-forge/ink-md-feedstock) behind them +- [@narma](https://github.com/narma) for honouring `--width full` and wide column counts in the reader ([#34](https://github.com/borghei/ink/pull/34)) ## License diff --git a/docs/configuration.md b/docs/configuration.md index 504a7a4..106fb1a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -32,8 +32,9 @@ Every setting is optional. These are the defaults: # terminal, or the name of a custom theme. Unset: match the terminal background. # theme = "dark" -# Max text width in columns, 20 to 1000. Unset: the terminal width, up to 120. -# Plain output uses 80. +# Max text width: 20 to 1000 columns, or "narrow" (60), "wide" (100) or +# "full" (the whole reader pane). Unset: the terminal width, up to 120. +# Plain output uses 80 unless a column count is set. # width = 90 # Line spacing: compact, normal, relaxed diff --git a/src/app.rs b/src/app.rs index 5c1241d..49ec4c9 100644 --- a/src/app.rs +++ b/src/app.rs @@ -8,7 +8,7 @@ use crate::selection::{self, Pos, SelMode, Selection}; use crate::stats; use crate::theme; use crate::toc::TocState; -use crate::Args; +use crate::{Args, Width}; use anyhow::Result; use comrak::{parse_document, Arena}; use crossterm::event::{DisableMouseCapture, EnableMouseCapture}; @@ -2377,10 +2377,10 @@ fn build_tab( // it at term_width - 4 (a requested --width wider than the terminal would // otherwise overflow and be clipped at the right edge). let available_width = term_width.saturating_sub(4).max(8); - let max_content_width = if args.full_width { - available_width - } else { - args.width.unwrap_or(120).clamp(8, available_width) + let max_content_width = match args.width { + Width::Full => available_width, + Width::Columns(n) => n.clamp(8, available_width), + Width::Auto => 120.min(available_width), }; let center_margin = if term_width > max_content_width + 4 { ((term_width - max_content_width) / 2) as usize @@ -2846,8 +2846,7 @@ mod section_tests { Args { inputs: Vec::new(), theme: "dark".into(), - width: None, - full_width: false, + width: Width::Auto, slides: false, plain: false, watch: false, @@ -3091,8 +3090,7 @@ mod section_tests { for terminal_width in [80, 200, 320] { for toc_visible in [false, true] { let mut args = test_args(); - args.full_width = true; - args.width = Some(60); // A configured width must not limit full-width mode. + args.width = Width::Full; args.toc = toc_visible; let pane_width = effective_width(terminal_width, toc_visible); let tab = build_tab(source.as_str(), "t.md", &args, pane_width, 0, &graphics); @@ -3120,7 +3118,7 @@ mod section_tests { (140, 80, 2, 76), ] { let mut args = test_args(); - args.width = Some(requested); + args.width = Width::Columns(requested); let tab = build_tab(source.as_str(), "t.md", &args, terminal_width, 0, &graphics); let first = tab.plain.iter().find(|line| !line.is_empty()).unwrap(); assert_eq!(first.len() - first.trim_start().len(), margin); diff --git a/src/cli.rs b/src/cli.rs index a10f977..b18e5c1 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -24,8 +24,8 @@ pub struct Cli { pub theme: String, /// Max rendering width in columns, 20-1000 (or: narrow, wide, full) - #[arg(short, long, value_parser = parse_width)] - pub width: Option, + #[arg(short, long, value_parser = Width::parse)] + pub width: Option, /// Presentation mode (split on ---) #[arg(short, long)] @@ -101,30 +101,50 @@ pub struct Cli { pub ascii: bool, } -/// A parsed `--width` value. -#[derive(Debug, Clone, Copy, PartialEq)] -pub enum WidthArg { - /// A column count (`narrow` = 60, `wide` = 100). +/// How wide to lay out text: `--width` or the config file's `width`. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +#[non_exhaustive] +pub enum Width { + /// Nothing asked for: the reader caps text at 120 columns, plain output + /// renders 80. + #[default] + Auto, + /// A column count in [`config::WIDTH_RANGE`] (`narrow` = 60, `wide` = 100). + /// The reader still keeps it inside the document pane. Columns(u16), - /// `full`: no width cap. + /// `full`: the whole document pane in the reader; 80 columns in plain + /// output, where there is no pane. Full, } -/// `--width` parser: a column count in [`config::WIDTH_RANGE`] or a keyword. -fn parse_width(s: &str) -> Result { - let (lo, hi) = (config::WIDTH_RANGE.start(), config::WIDTH_RANGE.end()); - match s { - "narrow" => Ok(WidthArg::Columns(60)), - "wide" => Ok(WidthArg::Columns(100)), - "full" => Ok(WidthArg::Full), - _ => s - .parse::() - .ok() - .filter(|n| config::WIDTH_RANGE.contains(n)) - .map(WidthArg::Columns) - .ok_or_else(|| { - format!("expected a column count from {lo} to {hi}, or one of: narrow, wide, full") - }), +impl Width { + /// Parse a column count in [`config::WIDTH_RANGE`] or a keyword + /// (`narrow`, `wide`, `full`). Shared by `--width` and the config file. + pub fn parse(s: &str) -> Result { + let (lo, hi) = (config::WIDTH_RANGE.start(), config::WIDTH_RANGE.end()); + match s { + "narrow" => Ok(Width::Columns(60)), + "wide" => Ok(Width::Columns(100)), + "full" => Ok(Width::Full), + _ => s + .parse::() + .ok() + .filter(|n| config::WIDTH_RANGE.contains(n)) + .map(Width::Columns) + .ok_or_else(|| { + format!( + "expected a column count from {lo} to {hi}, or one of: narrow, wide, full" + ) + }), + } + } + + /// The explicit column count, if one was asked for. + pub fn columns(self) -> Option { + match self { + Width::Columns(n) => Some(n), + Width::Auto | Width::Full => None, + } } } @@ -200,13 +220,14 @@ pub enum ConfigAction { } /// Resolved arguments for the app. +/// +/// Library users should build it with `..Default::default()` so that fields +/// added later do not break their code. #[derive(Clone)] pub struct Args { pub inputs: Vec, pub theme: String, - pub width: Option, - /// Use the entire document pane in the interactive reader. - pub full_width: bool, + pub width: Width, pub slides: bool, pub plain: bool, pub watch: bool, @@ -219,6 +240,28 @@ pub struct Args { pub clipboard: crate::clipboard::ClipboardMode, } +/// The values `ink` uses with no flags and no config file, except that +/// `plain` is on: a library caller has no terminal for the reader. +impl Default for Args { + fn default() -> Self { + Args { + inputs: Vec::new(), + theme: "auto".to_string(), + width: Width::Auto, + slides: false, + plain: true, + watch: false, + toc: false, + images: crate::image::ImageMode::LocalOnly, + image_protocol: crate::graphics::ProtocolChoice::Auto, + frontmatter: false, + spacing: Spacing::Normal, + mouse_capture: true, + clipboard: crate::clipboard::ClipboardMode::Auto, + } + } +} + #[derive(Debug, Clone, Copy, PartialEq, ValueEnum)] pub enum Spacing { Compact, @@ -369,7 +412,6 @@ pub fn run() -> Result<()> { inputs, theme, width, - full_width: cli.width == Some(WidthArg::Full), slides: cli.slides, // The interactive reader needs a terminal: with stdout piped or // redirected (`ink file.md | cat`), render plain output instead. @@ -648,12 +690,11 @@ pub(crate) fn mouse_capture( } } -fn resolve_width(width: &Option, config: &Option) -> Option { - match width { - Some(WidthArg::Columns(n)) => Some(*n), - Some(WidthArg::Full) => None, - None => config.as_ref().and_then(|c| c.width), - } +/// The `--width` flag wins over the config file's `width`. +fn resolve_width(width: &Option, config: &Option) -> Width { + width + .or_else(|| config.as_ref().and_then(|c| c.width)) + .unwrap_or_default() } fn config_init(force: bool) -> Result<()> { @@ -680,7 +721,7 @@ pub(crate) const STARTER_CONFIG: &str = r#"# ink configuration # Color theme: dark, light, dracula, catppuccin, nord, tokyo-night, gruvbox, solarized, terminal # theme = "catppuccin" -# Max rendering width in columns (or use --width on the CLI) +# Max rendering width: columns, or "narrow", "wide", "full" (or --width) # width = 90 # Line spacing: compact, normal, relaxed @@ -888,6 +929,32 @@ mod tests { config::parse_config(toml, "test").config } + #[test] + fn width_flag_wins_over_the_config_file() { + let c = cfg("width = \"full\""); + assert_eq!(resolve_width(&None, &c), Width::Full); + assert_eq!( + resolve_width(&Some(Width::Columns(70)), &c), + Width::Columns(70) + ); + assert_eq!( + resolve_width(&Some(Width::Full), &cfg("width = 90")), + Width::Full + ); + assert_eq!(resolve_width(&None, &None), Width::Auto); + } + + #[test] + fn width_keywords_parse_the_same_everywhere() { + assert_eq!(Width::parse("full"), Ok(Width::Full)); + assert_eq!(Width::parse("wide"), Ok(Width::Columns(100))); + assert_eq!(Width::parse("narrow"), Ok(Width::Columns(60))); + assert_eq!(Width::parse("140"), Ok(Width::Columns(140))); + assert!(Width::parse("19").is_err()); + assert!(Width::parse("1001").is_err()); + assert!(Width::parse("huge").is_err()); + } + #[test] fn no_mouse_flag_parses_before_and_after_a_subcommand() { assert!( diff --git a/src/config.rs b/src/config.rs index 59a657f..2f7359c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -6,7 +6,9 @@ use std::path::{Path, PathBuf}; #[derive(Debug, Deserialize)] pub struct Config { pub theme: Option, - pub width: Option, + /// Parsed by hand in [`parse_config`]: a number or a `--width` keyword. + #[serde(skip)] + pub width: Option, pub spacing: Option, pub toc: Option, pub frontmatter: Option, @@ -162,15 +164,37 @@ pub fn parse_config(content: &str, path: &str) -> ConfigLoad { warnings: &mut warnings, }; - let mut width: Option = w.take(&mut table, "", "width"); - if let Some(n) = width.filter(|n| !WIDTH_RANGE.contains(n)) { - w.push(format!( - "`width` = {n} is out of range ({}..={}); ignoring it", - WIDTH_RANGE.start(), - WIDTH_RANGE.end() - )); - width = None; - } + // A column count or a `--width` keyword (`narrow`, `wide`, `full`). + let width = match table.remove("width") { + None => None, + Some(toml::Value::Integer(n)) => { + match u16::try_from(n).ok().filter(|n| WIDTH_RANGE.contains(n)) { + Some(n) => Some(crate::cli::Width::Columns(n)), + None => { + w.push(format!( + "`width` = {n} is out of range ({}..={}); ignoring it", + WIDTH_RANGE.start(), + WIDTH_RANGE.end() + )); + None + } + } + } + Some(toml::Value::String(s)) => match crate::cli::Width::parse(&s) { + Ok(width) => Some(width), + Err(e) => { + w.push(format!("`width` = \"{s}\": {e}; ignoring it")); + None + } + }, + Some(other) => { + w.push(format!( + "`width`: expected a column count or one of: narrow, wide, full, found {}; ignoring it", + other.type_str() + )); + None + } + }; let mut spacing: Option = w.take(&mut table, "", "spacing"); if let Some(s) = spacing.as_deref().filter(|s| !SPACING_VALUES.contains(s)) { w.push(format!( @@ -275,6 +299,7 @@ pub fn set_theme(name: &str) -> anyhow::Result<()> { #[cfg(test)] mod tests { use super::*; + use crate::cli::Width; fn platform() -> Option { Some(PathBuf::from("platform-config")) @@ -302,7 +327,7 @@ mod tests { #[test] fn bad_value_warns_and_keeps_the_rest() { - let src = "width = \"wide\"\ntheme = \"nord\"\n[keybindings]\npreset = \"vim\"\n"; + let src = "width = \"huge\"\ntheme = \"nord\"\n[keybindings]\npreset = \"vim\"\n"; let load = parse_config(src, "cfg.toml"); let cfg = load.config.expect("rest of the config still applies"); assert_eq!(cfg.width, None); @@ -316,7 +341,7 @@ mod tests { fn unknown_keys_warn_by_name() { let src = "widht = 90\nwidth = 90\n[behavior]\nmouse_capure = false\n"; let load = parse_config(src, "cfg.toml"); - assert_eq!(load.config.unwrap().width, Some(90)); + assert_eq!(load.config.unwrap().width, Some(Width::Columns(90))); let all = load.warnings.join("\n"); assert!( all.contains("unknown config key `widht` in cfg.toml"), @@ -326,6 +351,29 @@ mod tests { assert_eq!(load.warnings.len(), 2); } + #[test] + fn width_takes_the_cli_keywords() { + for (src, want) in [ + ("width = \"full\"", Width::Full), + ("width = \"wide\"", Width::Columns(100)), + ("width = \"narrow\"", Width::Columns(60)), + ] { + let load = parse_config(src, "c"); + assert!(load.warnings.is_empty(), "{src}: {:?}", load.warnings); + assert_eq!(load.config.unwrap().width, Some(want), "{src}"); + } + } + + #[test] + fn width_of_the_wrong_type_warns() { + let load = parse_config("width = true\ntheme = \"nord\"\n", "c"); + let cfg = load.config.unwrap(); + assert_eq!(cfg.width, None); + assert_eq!(cfg.theme.as_deref(), Some("nord")); + assert_eq!(load.warnings.len(), 1, "{:?}", load.warnings); + assert!(load.warnings[0].contains("`width`"), "{:?}", load.warnings); + } + #[test] fn out_of_range_values_warn_and_fall_back() { let load = parse_config("width = 5\nspacing = \"bogus\"\ntheme = 5\n", "c"); @@ -344,7 +392,7 @@ mod tests { "c", ); assert!(load.warnings.is_empty(), "{:?}", load.warnings); - assert_eq!(load.config.unwrap().width, Some(90)); + assert_eq!(load.config.unwrap().width, Some(Width::Columns(90))); } #[test] diff --git a/src/lib.rs b/src/lib.rs index b17e62a..d16f151 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,4 +25,4 @@ pub mod toc; pub mod watch; pub mod wikilink; -pub use cli::{Args, Cli, Commands, ConfigAction, Spacing}; +pub use cli::{Args, Cli, Commands, ConfigAction, Spacing, Width}; diff --git a/src/render/plain.rs b/src/render/plain.rs index 7a5fa18..4a646ca 100644 --- a/src/render/plain.rs +++ b/src/render/plain.rs @@ -33,7 +33,7 @@ pub fn render_plain_with_color(source: &str, args: &Args, color: bool) -> Result // `width` is the total output width; reserve the left margin from it so the // rendered lines (margin + content) never exceed the requested width. let margin: usize = 2; - let width = args.width.unwrap_or(80); + let width = args.width.columns().unwrap_or(80); let content_width = width.saturating_sub(margin as u16).max(8); let styled_lines = layout::layout_document( root, diff --git a/tests/cli.rs b/tests/cli.rs index e45a4b6..168e3c6 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -437,7 +437,7 @@ fn config_unknown_key_warns_and_valid_keys_still_apply() { #[test] fn config_bad_value_warns_and_falls_back() { - let dir = xdg_with_config("width = \"wide\"\nspacing = \"bogus\"\n"); + let dir = xdg_with_config("width = \"huge\"\nspacing = \"bogus\"\n"); ink() .env("XDG_CONFIG_HOME", dir.path()) .arg("--plain") @@ -447,6 +447,27 @@ fn config_bad_value_warns_and_falls_back() { .stderr(predicate::str::contains("`width`").and(predicate::str::contains("`spacing`"))); } +#[test] +fn config_width_keyword_applies() { + let dir = xdg_with_config("width = \"narrow\"\n"); + let out = ink() + .env("XDG_CONFIG_HOME", dir.path()) + .args(["--plain", "--color=never"]) + .write_stdin(LONG_PARAGRAPH) + .assert() + .success() + .stderr(predicate::str::is_empty()) + .get_output() + .stdout + .clone(); + let text = String::from_utf8(out).unwrap(); + let widest = text.lines().map(|l| l.chars().count()).max().unwrap_or(0); + assert!( + (40..=60).contains(&widest), + "config width = \"narrow\" must apply, widest line {widest}: {text}" + ); +} + /// `ink` with the color-related environment cleared, so the host shell's /// NO_COLOR / FORCE_COLOR cannot change what these tests see. fn ink_clean_env() -> Command { diff --git a/tests/mermaid.rs b/tests/mermaid.rs index b644fdd..609a25c 100644 --- a/tests/mermaid.rs +++ b/tests/mermaid.rs @@ -8,8 +8,7 @@ fn args(width: u16) -> Args { Args { inputs: vec![], theme: "dark".to_string(), - width: Some(width), - full_width: false, + width: ink_md::Width::Columns(width), slides: false, plain: true, watch: false, diff --git a/tests/plain_snapshots.rs b/tests/plain_snapshots.rs index 421e24d..9b7a1d1 100644 --- a/tests/plain_snapshots.rs +++ b/tests/plain_snapshots.rs @@ -11,8 +11,7 @@ fn test_args() -> Args { Args { inputs: vec![], theme: "dark".to_string(), - width: Some(80), - full_width: false, + width: ink_md::Width::Columns(80), slides: false, plain: true, watch: false, diff --git a/tests/rendering_features.rs b/tests/rendering_features.rs index 7c39717..6a02841 100644 --- a/tests/rendering_features.rs +++ b/tests/rendering_features.rs @@ -9,8 +9,7 @@ fn args() -> Args { Args { inputs: vec![], theme: "dark".to_string(), - width: Some(80), - full_width: false, + width: ink_md::Width::Columns(80), slides: false, plain: true, watch: false, @@ -24,6 +23,20 @@ fn args() -> Args { } } +/// Library callers can fill in only what they need: fields added to `Args` +/// later must not break this. +#[test] +fn args_default_renders_plain_output() { + let args = Args { + width: ink_md::Width::Columns(40), + images: ink_md::image::ImageMode::Off, + ..Default::default() + }; + let out = render_plain_with_color(&"word ".repeat(40), &args, false).unwrap(); + assert!(out.lines().count() > 1, "{out}"); + assert!(out.lines().all(|l| l.width() <= 40), "{out}"); +} + fn render(src: &str) -> String { render_plain_with_color(src, &args(), false).unwrap() } @@ -87,7 +100,7 @@ fn table_alignment_applies_to_each_wrapped_line() { // visual line of it is right-aligned within the column. let out = render_with( "| k | value |\n|---|------:|\n| a | one two three four five six seven eight |\n", - |a| a.width = Some(30), + |a| a.width = ink_md::Width::Columns(30), ); assert_eq!( table_rows(&out), @@ -418,7 +431,7 @@ fn frontmatter_box_wraps_long_values_within_the_width() { "---\ndescription: one two three four five six seven eight nine ten eleven\n---\n", |a| { a.frontmatter = true; - a.width = Some(40); + a.width = ink_md::Width::Columns(40); }, ); for line in out.lines() { @@ -498,7 +511,9 @@ fn math_in_table_cells() { #[test] fn display_math_wider_than_the_view_is_broken_not_clipped() { let long = "x + ".repeat(30); - let out = render_with(&format!("$$\n{long}y\n$$\n"), |a| a.width = Some(40)); + let out = render_with(&format!("$$\n{long}y\n$$\n"), |a| { + a.width = ink_md::Width::Columns(40) + }); for line in out.lines() { assert!(line.width() <= 40, "{line:?}"); } diff --git a/tests/security.rs b/tests/security.rs index bfedf3c..4df45e1 100644 --- a/tests/security.rs +++ b/tests/security.rs @@ -8,8 +8,7 @@ fn args() -> Args { Args { inputs: vec![], theme: "dark".to_string(), - width: Some(80), - full_width: false, + width: ink_md::Width::Columns(80), slides: false, plain: true, watch: false,