fix: integrate 22 reviewed bug fixes with combined validation

crates/engine/src/commands.rs

@@ -194,7 +194,9 @@ fn search_query(s: &mut Session, p: &Params) -> Result<Output, EngineError> {
         Some(l) => l.trim().parse::<usize>().map_err(|_| EngineError::BadParam("limit must be a number".into()))?.min(500),
         None => 40,
     };
-    Ok(Output::Hits(search::search(s.root(), req(p, "q")?, limit)))
+    // Every open notebook, the ones opened from another folder too: the window calls this
+    // Search all pages (issue #127).
+    Ok(Output::Hits(search::search_in(s.root(), s.opened_elsewhere(), req(p, "q")?, limit)))
 }
 
 fn notebook_check(s: &mut Session, p: &Params) -> Result<Output, EngineError> {
@@ -672,6 +674,37 @@ mod tests {
         assert_eq!(std::fs::read_dir(&bare).into_iter().flatten().flatten().filter(|e| e.path().is_dir()).count(), 0);
     }
 
+    /// Issue #127: Search all pages finds pages in a notebook opened from another folder, once
+    /// each, with the limit applied to every notebook together.
+    #[test]
+    fn search_finds_pages_in_a_notebook_opened_from_elsewhere() {
+        let t = TempDir::new("engine-search-elsewhere");
+        let settings = Settings::at(t.path().join("settings").join("s.toml"));
+        let mut s = Session::new(&t.path().join("notebooks"), settings.clone()).expect("session");
+        let here = path_of(s.run("notebook.create", &p(&[("name", "Here")])).expect("notebook"));
+        let here_page = here.join("new-section-1").join("untitled-page");
+        s.run("page.save", &p(&[("path", &here_page.to_string_lossy()), ("body", "<p>shared_token here</p>")])).expect("save");
+
+        let away = t.path().join("away");
+        std::fs::create_dir_all(&away).expect("dir");
+        let mut other = Session::new(&away, settings.clone()).expect("other session");
+        let made = path_of(other.run("notebook.create", &p(&[("name", "Elsewhere")])).expect("notebook"));
+        let away_page = made.join("new-section-1").join("untitled-page");
+        other
+            .run("page.save", &p(&[("path", &away_page.to_string_lossy()), ("body", "<p>external_unique_token and shared_token</p>")]))
+            .expect("save");
+        s.run("notebook.open", &p(&[("path", &made.to_string_lossy())])).expect("open");
+
+        let hits = |s: &mut Session, q: &str, limit: &str| match s.run("search.query", &p(&[("q", q), ("limit", limit)])).expect("search") {
+            Output::Hits(h) => h,
+            other => panic!("{other:?}"),
+        };
+        let found = hits(&mut s, "external_unique_token", "10");
+        assert_eq!(found.len(), 1, "the page in the notebook opened from elsewhere is found: {found:?}");
+        assert_eq!(hits(&mut s, "shared_token", "10").len(), 2, "a page in each notebook, each once");
+        assert_eq!(hits(&mut s, "shared_token", "1").len(), 1, "the limit is for all the notebooks together");
+    }
+
     #[test]
     fn a_notebook_from_elsewhere_opens_beside_the_others_and_stays_open() {
         let t = TempDir::new("engine-open-elsewhere");

crates/formats/src/csv.rs

@@ -3,6 +3,9 @@
 //! The marks inside a cell are dropped, because a spreadsheet cell holds a value, not a
 //! marked-up line. Quotes are doubled and a cell is wrapped when it holds a comma, a
 //! quote or a line break, which is the rule everything that reads CSV agrees on.
+//!
+//! Read back, a cell is its value as written: `*literal*` in a spreadsheet is those nine
+//! characters, not a word in italics.
 
 use crate::markdown::{Block, Span};
 
@@ -11,7 +14,7 @@ pub fn from_table(block: &Block) -> Option<String> {
     let Block::Table { head, rows } = block else { return None };
     let mut out = String::new();
     for row in std::iter::once(head).chain(rows.iter()) {
-        let line: Vec<String> = row.iter().map(|cell| quote(&plain(cell))).collect();
+        let line: Vec<String> = row.iter().map(|cell| quote(&plain(cell), row.len() == 1)).collect();
         out.push_str(&line.join(","));
         out.push('\n');
     }
@@ -23,8 +26,14 @@ fn plain(spans: &[Span]) -> String {
     spans.iter().map(|s| s.text.as_str()).collect()
 }
 
-fn quote(cell: &str) -> String {
-    if cell.contains([',', '"', '\n', '\r']) { format!("\"{}\"", cell.replace('"', "\"\"")) } else { cell.to_string() }
+/// The cell as CSV. The only cell of a row is wrapped when it is blank too: written bare, its
+/// line would be a blank line, which a reader skips, and the rows under it would move up.
+fn quote(cell: &str, alone: bool) -> String {
+    if cell.contains([',', '"', '\n', '\r']) || (alone && cell.trim().is_empty()) {
+        format!("\"{}\"", cell.replace('"', "\"\""))
+    } else {
+        cell.to_string()
+    }
 }
 
 /// A table read back from CSV: the first row is the heading, as it was written out.
@@ -33,20 +42,30 @@ fn quote(cell: &str) -> String {
 /// doubled quote inside one is a quote, and a wrapped cell may hold commas and line
 /// breaks. Anything else is taken as it stands rather than refused, because a spreadsheet
 /// that wrote it is not going to be argued with.
+///
+/// A line with nothing on it is skipped, but a row whose one cell was written as `""` is a
+/// blank row of the table and is kept, so the rows under it stay where they were.
 pub fn to_table(text: &str) -> Option<Block> {
     let rows = rows_of(text);
-    let mut rows = rows.into_iter().filter(|r| !(r.len() == 1 && r[0].trim().is_empty()));
-    let head: Vec<Vec<Span>> = rows.next()?.iter().map(|c| crate::markdown::parse_inline(c)).collect();
-    let body: Vec<Vec<Vec<Span>>> = rows.map(|r| r.iter().map(|c| crate::markdown::parse_inline(c)).collect()).collect();
+    let mut rows = rows.into_iter().filter(|(r, quoted)| *quoted || !(r.len() == 1 && r[0].trim().is_empty())).map(|(r, _)| r);
+    let head: Vec<Vec<Span>> = rows.next()?.iter().map(|c| literal(c)).collect();
+    let body: Vec<Vec<Vec<Span>>> = rows.map(|r| r.iter().map(|c| literal(c)).collect()).collect();
     Some(Block::Table { head, rows: body })
 }
 
-/// The cells of each line, with quotes undone.
-fn rows_of(text: &str) -> Vec<Vec<String>> {
+/// A cell's value as plain text. A spreadsheet cell holds a value, so its asterisks,
+/// brackets and backticks are characters of that value, not marks for the page.
+fn literal(cell: &str) -> Vec<Span> {
+    if cell.is_empty() { Vec::new() } else { vec![Span { text: cell.to_string(), ..Span::default() }] }
+}
+
+/// The cells of each line, with quotes undone, and whether any cell on the line was quoted.
+fn rows_of(text: &str) -> Vec<(Vec<String>, bool)> {
     let mut rows = Vec::new();
     let mut row: Vec<String> = Vec::new();
     let mut cell = String::new();
     let mut quoted = false;
+    let mut any_quoted = false;
     let mut chars = text.chars().peekable();
     while let Some(c) = chars.next() {
         match c {
@@ -61,19 +80,20 @@ fn rows_of(text: &str) -> Vec<Vec<String>> {
             '"' if cell.trim().is_empty() => {
                 cell.clear();
                 quoted = true;
+                any_quoted = true;
             }
             ',' if !quoted => row.push(std::mem::take(&mut cell)),
             '\r' if !quoted => {}
             '\n' if !quoted => {
                 row.push(std::mem::take(&mut cell));
-                rows.push(std::mem::take(&mut row));
+                rows.push((std::mem::take(&mut row), std::mem::take(&mut any_quoted)));
             }
             other => cell.push(other),
         }
     }
-    if !cell.is_empty() || !row.is_empty() {
+    if !cell.is_empty() || !row.is_empty() || any_quoted {
         row.push(cell);
-        rows.push(row);
+        rows.push((row, any_quoted));
     }
     rows
 }
@@ -129,6 +149,37 @@ lines",
         assert_eq!(text(&rows[1][1]), "last");
     }
 
+    /// Issue #110: a cell is read as the value it holds. Read as Markdown, `*literal*` came
+    /// back as an italic `literal` and `[a](b)` as a link, and writing the table out again
+    /// lost the characters.
+    #[test]
+    fn a_cell_that_looks_like_markdown_is_read_as_it_is_written() {
+        for input in ["Value\n*literal*\n", "Value\n[a](b)\n", "Value\n`x` **y** _z_ <b>w</b>\n", "# Heading,- item\n> quote,1. one\n"] {
+            let table = to_table(input).expect("a table");
+            let Block::Table { head, rows } = &table else { panic!("a table") };
+            let plain_only =
+                std::iter::once(head).chain(rows).flatten().flatten().all(|span| span == &Span { text: span.text.clone(), ..Span::default() });
+            assert!(plain_only, "{input:?} is read without marks");
+            assert_eq!(from_table(&table).as_deref(), Some(input), "{input:?} comes back character for character");
+        }
+    }
+
+    /// Issue #112: a blank row of a one-column table survives the trip through CSV, and so
+    /// do the rows under it.
+    #[test]
+    fn a_blank_row_of_a_one_column_table_is_kept() {
+        let cell = |text: &str| if text.is_empty() { Vec::new() } else { vec![Span { text: text.into(), ..Span::default() }] };
+        let table = Block::Table { head: vec![cell("Value")], rows: vec![vec![cell("")], vec![cell("value")], vec![cell("  ")]] };
+        let csv = from_table(&table).expect("a table");
+        assert_eq!(csv, "Value\n\"\"\nvalue\n\"  \"\n", "a blank only cell is written quoted");
+        assert_eq!(to_table(&csv), Some(table), "and read back as a row");
+
+        // What a spreadsheet writes for a blank row, with blank lines between the rows that
+        // are not rows.
+        let Some(Block::Table { rows, .. }) = to_table("Value\n\"\"\n\nvalue\n\n") else { panic!("a table") };
+        assert_eq!(rows, vec![vec![cell("")], vec![cell("value")]]);
+    }
+
     #[test]
     fn nothing_in_nothing_out() {
         assert_eq!(to_table(""), None);

crates/formats/src/html.rs

@@ -110,18 +110,39 @@ pub fn unescape(s: &str) -> String {
     out.replace("&", "&")
 }
 
-/// Plain text of an HTML fragment: tags removed, entities decoded.
+/// Tags that mark words inside a line. They join what is on either side of them, so the
+/// letters of `for<b>mat</b>` stay one word; every other tag ends a word.
+const INLINE_TAGS: &[&str] = &[
+    "a", "abbr", "b", "bdi", "bdo", "big", "cite", "code", "del", "dfn", "em", "font", "i", "ins", "kbd", "mark", "q", "s", "samp", "small", "span",
+    "strike", "strong", "sub", "sup", "time", "tt", "u", "var", "wbr",
+];
+
+/// Plain text of an HTML fragment: tags removed, entities decoded. A tag that starts or ends
+/// a block, a line break or a cell leaves a space behind; formatting inside a line leaves
+/// nothing, so a word that is partly bold is still one word (issue #105).
 pub fn strip_tags(html: &str) -> String {
-    let (mut out, mut in_tag) = (String::new(), false);
+    let (mut out, mut tag, mut in_tag) = (String::new(), String::new(), false);
     for c in html.chars() {
         match c {
-            '<' => in_tag = true,
-            '>' => {
+            '<' => {
+                in_tag = true;
+                tag.clear();
+            }
+            '>' if in_tag => {
                 in_tag = false;
-                out.push(' ');
+                let name: String = tag
+                    .trim_start()
+                    .trim_start_matches('/')
+                    .chars()
+                    .take_while(|c| c.is_ascii_alphanumeric())
+                    .collect::<String>()
+                    .to_ascii_lowercase();
+                if !INLINE_TAGS.contains(&name.as_str()) {
+                    out.push(' ');
+                }
             }
-            c if !in_tag => out.push(c),
-            _ => {}
+            c if in_tag => tag.push(c),
+            c => out.push(c),
         }
     }
     unescape(&out)
@@ -323,9 +344,14 @@ struct Writer {
     lists: Vec<(bool, usize)>, // (ordered, counter)
     in_pre: bool,
     in_code: bool,
+    /// Where the open inline code's text starts, in `cell` inside a table and `out` elsewhere.
+    code_at: Option<usize>,
     pre_diagram: bool,
     /// Inside a `<div class="mermaid">`, which Mermaid's own pages use instead of a `pre`.
     div_diagram: bool,
+    /// Where the open code block's fence starts in `out`, so it can be made longer when the
+    /// code turns out to hold a fence of its own.
+    fence_at: Option<usize>,
     in_math: bool,
     in_math_block: bool,
     span_sizes: Vec<bool>,
@@ -338,6 +364,26 @@ struct Writer {
 }
 
 impl Writer {
+    /// Remembers where the fence of a code block about to be written starts.
+    fn open_fence(&mut self) {
+        self.fence_at = (!self.in_cell).then_some(self.out.len());
+    }
+
+    /// Writes the fence that closes the open code block. When a line of the code starts with
+    /// three or more backticks, both fences are made longer than that run, so the code stays
+    /// one block when it is read again (issue #106).
+    fn close_fence(&mut self) {
+        let mut fence = "```".to_string();
+        if let Some(at) = self.fence_at.take().filter(|&at| self.out.is_char_boundary(at)) {
+            let code_from = self.out[at..].find('\n').map_or(self.out.len(), |n| at + n + 1);
+            fence = crate::markdown::code_fence(&self.out[code_from..]);
+            if fence.len() > 3 {
+                self.out.insert_str(at, &fence[3..]);
+            }
+        }
+        self.out.push_str(&fence);
+    }
+
     fn push(&mut self, s: &str) {
         if self.in_cell {
             self.cell.push_str(s);
@@ -423,6 +469,7 @@ impl Writer {
             // A div with the class `mermaid` holds a diagram, as a pre does. Its text is the source.
             ("div", false) if !self.in_pre && attrs.to_lowercase().contains("mermaid") => {
                 self.block_break();
+                self.open_fence();
                 self.push("```mermaid\n");
                 self.in_pre = true;
                 self.pre_diagram = true;
@@ -434,7 +481,7 @@ impl Writer {
                 if !self.out.ends_with('\n') {
                     self.out.push('\n');
                 }
-                self.out.push_str("```");
+                self.close_fence();
                 self.block_break();
             }
             ("p", true) if self.in_math_block => {
@@ -511,9 +558,23 @@ impl Writer {
             ("i" | "em", _) => self.push("*"),
             ("s" | "del" | "strike", _) => self.push("~~"),
             ("u" | "sub" | "sup", closing) => self.push(&format!("<{}{name}>", if closing { "/" } else { "" })),
-            ("code", closing) if !self.in_pre => {
-                self.in_code = !closing;
-                self.push("`");
+            ("code", false) if !self.in_pre => {
+                self.in_code = true;
+                self.code_at = Some(if self.in_cell { self.cell.len() } else { self.out.len() });
+            }
+            ("code", true) if !self.in_pre => {
+                self.in_code = false;
+                // The code is written with the delimiters it needs once all of it is known: a
+                // backtick inside it would otherwise end it early (issue #107).
+                if let Some(at) = self.code_at.take() {
+                    let buffer = if self.in_cell { &mut self.cell } else { &mut self.out };
+                    if buffer.is_char_boundary(at) {
+                        let code = buffer.split_off(at);
+                        if !code.is_empty() {
+                            buffer.push_str(&crate::render::code_span(&code));
+                        }
+                    }
+                }
             }
             ("pre", false) => {
                 self.block_break();
@@ -522,6 +583,7 @@ impl Writer {
                 let class = attr(attrs, "class").unwrap_or_default();
                 self.pre_diagram = class.eq_ignore_ascii_case("diagram") || class.eq_ignore_ascii_case("mermaid");
                 let language = if self.pre_diagram { "mermaid" } else { class.as_str() };
+                self.open_fence();
                 self.push(&format!("```{language}\n"));
                 self.in_pre = true;
             }
@@ -530,7 +592,7 @@ impl Writer {
                 if !self.out.ends_with('\n') {
                     self.out.push('\n');
                 }
-                self.out.push_str("```");
+                self.close_fence();
                 self.block_break();
             }
             ("span", false) => {
@@ -565,7 +627,7 @@ impl Writer {
             }
             ("a", true) => {
                 let href = self.href.pop().flatten().unwrap_or_default();
-                self.push(&format!("]({href})"));
+                self.push(&format!("]({})", crate::render::destination(&href)));
             }
             ("img", _) => {
                 let alt = attr(attrs, "alt").unwrap_or_default();
@@ -576,7 +638,9 @@ impl Writer {
                 // Block-level img with size: output as HTML. Inline or no size: use markdown.
                 if (width.is_some() || height.is_some()) && self.href.is_empty() && !self.in_cell {
                     self.block_break();
-                    let mut img_tag = format!("<img src=\"{src}\" alt=\"{alt}\"");
+                    // `attr` gave the values unescaped; they are escaped again for the tag, so a quote
+                    // in the text cannot end the attribute (issue #109).
+                    let mut img_tag = format!("<img src=\"{}\" alt=\"{}\"", escape(&src), escape(&alt));
                     if let Some(w) = width {
                         img_tag.push_str(&format!(" width=\"{}\"", w as i32));
                     }
@@ -587,12 +651,12 @@ impl Writer {
                     self.push(&img_tag);
                     self.block_break();
                 } else {
-                    self.push(&format!("![{alt}]({src})"));
+                    self.push(&format!("![{alt}]({})", crate::render::destination(&src)));
                 }
             }
             ("video" | "audio", false) => {
                 let src = attr(attrs, "src").unwrap_or_default();
-                self.push(&format!("![{name}]({src})"));
+                self.push(&format!("![{name}]({})", crate::render::destination(&src)));
             }
             ("table", false) => {
                 self.block_break();
@@ -723,6 +787,19 @@ mod tests {
         assert_eq!(strip_tags("<p>Hello <b>big</b> world</p>").split_whitespace().collect::<Vec<_>>().join(" "), "Hello big world");
     }
 
+    /// Issue #105: formatting inside a word keeps the word whole, and blocks, line breaks
+    /// and cells still keep their words apart.
+    #[test]
+    fn stripping_tags_keeps_a_partly_formatted_word_whole() {
+        let words = |html: &str| strip_tags(html).split_whitespace().collect::<Vec<_>>().join(" ");
+        assert_eq!(words("<p>for<b>mat</b></p>"), "format");
+        assert_eq!(words("<p>co<B>op</B>er<span style=\"color: red\">ate</span> <a href=\"x\">li</a>nk</p>"), "cooperate link");
+        assert_eq!(words("<p>one</p><p>two</p>"), "one two");
+        assert_eq!(words("<p>one<br>two<br/>three</p>"), "one two three");
+        assert_eq!(words("<ul><li>a</li><li>b</li></ul><table><tr><td>c</td><td>d</td></tr></table>"), "a b c d");
+        assert_eq!(words("<h1>Title</h1>Body &amp; more"), "Title Body & more");
+    }
+
     #[test]
     fn converts_blocks_and_inline_markup() {
         let md = to_markdown("<h1>Title</h1><p>One <b>bold</b> and <i>italic</i> and <code>code</code>.</p><ul><li>a</li><li>b</li></ul>");