Merge pull request #6840 from dotCooCoo/fix/tab-stops-limit

Tabs: the ruler and Repeat Tab stop at 256 tab stops, and a paragraph with more can lose them

crates/engine/src/cmd/tabs.rs

@@ -13,7 +13,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Set Tab Stops",
             ["Window", "Tabs"],
             None,
-            "{stops: [{position: pt, align?: left|center|right|decimal, leader?: \". \", alignOn?: \".\"}], ids?|id?, start?: byte, end?: byte} replace the tab stops of the selected text (with a range: of the paragraphs it touches); at most 256 stops, positions 0..=100000 pt; supplied align, leader and alignOn must be strings, a leader has at most 8 characters and alignOn exactly one; stops are sorted by position, and of stops at one position the first given is kept",
+            "{stops: [{position: pt, align?: left|center|right|decimal, leader?: \". \", alignOn?: \".\"}], ids?|id?, start?: byte, end?: byte} replace the tab stops of the selected text (with a range: of the paragraphs it touches); at most 256 stops, or no more than each targeted paragraph already holds, positions 0..=100000 pt; supplied align, leader and alignOn must be strings, a leader has at most 8 characters and alignOn exactly one; stops are sorted by position, and of stops at one position the first given is kept",
             has_selection,
             set
         ),
@@ -38,10 +38,6 @@ pub fn specs() -> Vec<CommandSpec> {
 fn parse_stops(p: &Value) -> Result<Vec<TabStop>> {
     const C: &str = "text.tabs.set";
     let arr = p.get("stops").and_then(Value::as_array).ok_or_else(|| bad(C, "missing stops"))?;
-    let max = TabStop::MAX_STOPS;
-    if arr.len() > max {
-        return Err(bad(C, format!("at most {max} tab stops")));
-    }
     let mut stops = vec![];
     for s in arr {
         let position = s.get("position").and_then(Value::as_f64).ok_or_else(|| bad(C, "each stop needs a position"))?;
@@ -90,10 +86,39 @@ fn parse_stops(p: &Value) -> Result<Vec<TabStop>> {
 }
 
 fn set(s: &mut Session, p: &Value) -> Result<Value> {
+    // More than the limit is taken only when every targeted paragraph already holds at least as
+    // many stops, so a paragraph that holds more can still lose them one at a time. The supplied
+    // list is counted before it is read.
+    let max = TabStop::MAX_STOPS;
+    let supplied = p.get("stops").and_then(Value::as_array).map_or(0, Vec::len);
+    if supplied > max && supplied > fewest_held(s, p)? {
+        return Err(bad("text.tabs.set", format!("at most {max} tab stops")));
+    }
     let stops = parse_stops(p)?;
     apply(s, p, stops, "Tab Stops")
 }
 
+/// The fewest tab stops a paragraph that `text.tabs.set` with `p` targets holds (0 with none).
+fn fewest_held(s: &Session, p: &Value) -> Result<usize> {
+    let ids = text_targets(s, p, "text.tabs.set")?;
+    let range = TextRange::parse(p, "text.tabs.set")?;
+    let doc = &s.doc()?.doc;
+    let mut fewest: Option<usize> = None;
+    for id in &ids {
+        if let Some(NodeKind::Text(t)) = doc.node(*id).map(|node| &node.kind) {
+            let held = match para_span(range, t) {
+                None => t.para_styles().map(|pa| pa.tabs.len()).min(),
+                Some(r) => r.map(|i| t.para_at(i).tabs.len()).min(),
+            };
+            fewest = match (fewest, held) {
+                (Some(a), Some(b)) => Some(a.min(b)),
+                (a, b) => a.or(b),
+            };
+        }
+    }
+    Ok(fewest.unwrap_or(0))
+}
+
 fn apply(s: &mut Session, p: &Value, stops: Vec<TabStop>, label: &str) -> Result<Value> {
     let ids = text_targets(s, p, "text.tabs.set")?;
     let range = TextRange::parse(p, "text.tabs.set")?;
@@ -159,14 +184,52 @@ mod tests {
         let err = |stops: Value| super::parse_stops(&json!({ "stops": stops })).unwrap_err().to_string();
         assert!(err(json!([{"position": 40, "leader": "123456789"}])).contains("`leader` must be at most 8 characters"));
         assert!(err(json!([{"position": 40, "alignOn": ",."}])).contains("`alignOn` must be one character"));
+        let mut s = Session::new();
+        s.execute("file.new", &json!({})).unwrap();
+        s.execute("text.create", &json!({"x": 10, "y": 20, "text": "a\tb"})).unwrap();
         let many: Vec<Value> = (0..257).map(|i| json!({"position": i})).collect();
-        assert!(err(Value::from(many.clone())).contains("at most 256 tab stops"));
-        let ok = super::parse_stops(&json!({ "stops": &many[..256] })).unwrap();
-        assert_eq!(ok.len(), 256);
+        let e = s.execute("text.tabs.set", &json!({ "stops": many.clone() })).unwrap_err().to_string();
+        assert!(e.contains("at most 256 tab stops"), "{e}");
+        // The supplied list is counted, not the stops left once those at one position merge.
+        let same: Vec<Value> = (0..257).map(|_| json!({"position": 5})).collect();
+        assert!(s.execute("text.tabs.set", &json!({ "stops": same })).unwrap_err().to_string().contains("at most 256 tab stops"));
+        assert_eq!(s.execute("text.tabs.set", &json!({ "stops": &many[..256] })).unwrap()["stops"], json!(256));
         let ok = super::parse_stops(&json!({"stops": [{"position": 0, "leader": "12345678", "alignOn": "€"}]})).unwrap();
         assert_eq!((ok[0].leader.as_str(), ok[0].align_on), ("12345678", '€'));
     }
 
+    /// A paragraph that already holds more than 256 stops (from a file) can lose them one at a
+    /// time: a list no longer than every targeted paragraph's own is taken, a longer one is not, and
+    /// a paragraph with fewer stops targeted beside it keeps the limit.
+    #[test]
+    fn stops_can_be_removed_from_a_paragraph_holding_more_than_the_limit() {
+        use vectorcraft_doc::{NodeId, NodeKind, TabAlign, TabStop};
+        let mut s = Session::new();
+        s.execute("file.new", &json!({})).unwrap();
+        let id = NodeId(s.execute("text.create", &json!({"x": 10, "y": 20, "text": "a\tb\nc\td"})).unwrap()["id"].as_u64().unwrap());
+        let held: Vec<TabStop> =
+            (0..300).map(|i| TabStop { position: f64::from(i), align: TabAlign::Left, leader: String::new(), align_on: '.' }).collect();
+        s.edit("Load", |d, _| {
+            if let Some(NodeKind::Text(t)) = d.node_mut(id).map(|n| &mut n.kind) {
+                t.edit_paras(None, |pa| pa.tabs = held.clone());
+            }
+            Ok(())
+        })
+        .unwrap();
+        let list = |n: i32| json!({ "stops": (0..n).map(|i| json!({"position": i})).collect::<Vec<_>>() });
+        assert!(s.execute("text.tabs.set", &list(301)).unwrap_err().to_string().contains("at most 256 tab stops"));
+        assert_eq!(s.execute("text.tabs.set", &list(299)).unwrap()["stops"], json!(299));
+        assert_eq!(s.execute("text.tabs.get", &json!({})).unwrap()["stops"].as_array().unwrap().len(), 299);
+        // The second paragraph (bytes 4 on) now holds 2 stops: 298 for both paragraphs is refused,
+        // and 298 for the first alone is taken.
+        s.execute("text.tabs.set", &json!({"stops": [{"position": 1}, {"position": 2}], "start": 4, "end": 4})).unwrap();
+        let mut both = list(298);
+        assert!(s.execute("text.tabs.set", &both).is_err());
+        both["start"] = json!(0);
+        both["end"] = json!(0);
+        assert_eq!(s.execute("text.tabs.set", &both).unwrap()["stops"], json!(298));
+    }
+
     #[test]
     fn set_get_clear_tabs() {
         let mut s = Session::new();

crates/ui-egui/src/panels/tabs.rs

@@ -686,15 +686,22 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut Ui) {
                     // A click that lands on a stop's whole point or tick selects that stop and records no
                     // undo step.
                     let added = !stops.iter().any(|s| s.position == position);
-                    if added {
-                        stops.push(TabStop { position, align: ALIGNS[align_idx.min(3)].0, leader: String::new(), align_on: '.' });
-                        stops.sort_by(|a, b| a.position.total_cmp(&b.position));
-                    }
-                    let i = stops.iter().position(|s| s.position == position).unwrap_or(0);
-                    set_pstate::<Option<IndentMarker>>(ui.ctx(), "tabs-marker-sel", None);
-                    set_pstate(ui.ctx(), "tabs-sel", i);
-                    if added {
-                        apply(app, &stops);
+                    let max = TabStop::MAX_STOPS;
+                    if added && stops.len() >= max {
+                        // A full list takes no more stops: the click leaves the stops and the
+                        // selection as they are and shows the limit.
+                        app.status(format!("at most {max} tab stops"));
+                    } else {
+                        if added {
+                            stops.push(TabStop { position, align: ALIGNS[align_idx.min(3)].0, leader: String::new(), align_on: '.' });
+                            stops.sort_by(|a, b| a.position.total_cmp(&b.position));
+                        }
+                        let i = stops.iter().position(|s| s.position == position).unwrap_or(0);
+                        set_pstate::<Option<IndentMarker>>(ui.ctx(), "tabs-marker-sel", None);
+                        set_pstate(ui.ctx(), "tabs-sel", i);
+                        if added {
+                            apply(app, &stops);
+                        }
                     }
                 }
             }
@@ -729,7 +736,7 @@ pub fn menu(app: &mut VectorcraftApp, ui: &mut Ui) {
     }
     // Repeat Tab: copies of the selected stop across the ruler, spaced by its distance from the
     // previous stop, or from the paragraph's left indent plus its list level's indent when it is
-    // the first stop.
+    // the first stop. The copies stop at the 256-stop limit, which the status bar then shows.
     if menu_item(ui, tl!("Repeat Tab"), sel < stops.len() && selected_marker.is_none(), false) {
         let step = match sel {
             0 => (stops[0].position - left_indent).abs(),
@@ -738,8 +745,13 @@ pub fn menu(app: &mut VectorcraftApp, ui: &mut Ui) {
         .max(1.0);
         let base = stops[sel].clone();
         stops.truncate(sel + 1);
+        let max = TabStop::MAX_STOPS;
         let mut pos = base.position + step;
         while pos <= base.position + step * 20.0 && pos < 2000.0 {
+            if stops.len() >= max {
+                app.status(format!("at most {max} tab stops"));
+                break;
+            }
             stops.push(TabStop { position: pos, ..base.clone() });
             pos += step;
         }

crates/ui-egui/src/panels/tabs/tests.rs

@@ -1255,6 +1255,35 @@ fn panel_menu_does_not_change_stops_while_an_indent_marker_is_selected() {
     assert_eq!(current(&app).unwrap().1, before);
 }
 
+/// With 256 stops, a click on an empty part of the ruler adds none and shows the limit in the
+/// status bar. Repeat Tab near the end of the list adds copies only up to 256 stops and shows the
+/// limit too.
+#[test]
+fn the_ruler_and_repeat_tab_stop_at_256_stops() {
+    let mut app = VectorcraftApp::new(Session::new(), Default::default());
+    app.session.execute("file.new", &json!({})).unwrap();
+    app.session.execute("text.create", &json!({"x": 10, "y": 20, "text": "a	b", "area": {"width": 300, "height": 100}})).unwrap();
+    // 250 stops 1 pt apart from 0 pt, then six 2 pt apart from 260 pt.
+    let start: Vec<f64> = (0..250).map(f64::from).chain((0..6).map(|i| 260.0 + 2.0 * f64::from(i))).collect();
+    let stops: Vec<Value> = start.iter().map(|p| json!({"position": p})).collect();
+    app.session.execute("text.tabs.set", &json!({ "stops": stops })).unwrap();
+    let ctx = egui::Context::default();
+    let positions = |app: &VectorcraftApp| current(app).unwrap().1.iter().map(|s| s.position).collect::<Vec<_>>();
+    set_pstate(&ctx, "tabs-sel", 3usize);
+    drag_on_ruler(&ctx, &mut app, 290.0, 290.0, false);
+    assert_eq!(positions(&app), start);
+    assert_eq!(pstate::<usize>(&ctx, "tabs-sel"), 3);
+    assert_eq!(app.ui.status, "at most 256 tab stops");
+    // Repeat Tab on the stop at 260 pt (11 pt past the one before it): five copies, not 20.
+    app.status("");
+    set_pstate(&ctx, "tabs-sel", 250usize);
+    choose(&ctx, &mut app, "Repeat Tab");
+    let repeated = positions(&app);
+    assert_eq!(repeated.len(), 256);
+    assert_eq!(repeated[249..], [249.0, 260.0, 271.0, 282.0, 293.0, 304.0, 315.0]);
+    assert_eq!(app.ui.status, "at most 256 tab stops");
+}
+
 /// Repeat Tab on the first stop spaces the copies by the stop's distance from the paragraph's
 /// left indent (18 pt further in for each list level past the first), from the paragraph the
 /// caret is in while the Type tool edits text.

docs/mcp.md

@@ -7922,8 +7922,9 @@ targeted objects, as before. With a range, the character attributes of `text.set
 that range of characters. When every paragraph that `text.tabs.set` or `text.tabs.clear` targets already has those
 stops, the command changes nothing and records no undo step. `text.tabs.set` rejects a supplied `align`, `leader`, or
 `alignOn` value that is not a string, an empty `alignOn` value or one of more than one character, a leader of more than
-8 characters, a position outside 0 to 100000 pt, and more than 256 stops. It sorts the stops by position and keeps the
-first of stops at one position.
+8 characters, a position outside 0 to 100000 pt, and more than 256 stops unless every paragraph it targets already
+holds at least that many, so stops can be removed from a paragraph that holds more. It sorts the stops by position and
+keeps the first of stops at one position.
 
 `text.tabs.get {start?}` and `paraStyle.new` / `paraStyle.redefine {id, start?}` read the
 paragraph at `start`; `paraStyle.list` counts paragraphs as uses, and `charStyle.list` counts runs; both count the type in