skera: apply glyph-map selections in command-line order

skera/README.md

@@ -14,6 +14,9 @@ Custom glyph IDs are supported through `--gid-map 1:4,2:7` or
 `Plan::set_glyph_mapping`. Unspecified retained glyphs follow the highest
 requested output ID; gaps become empty glyphs. The final mapping must preserve
 original glyph order, keep `.notdef` at zero, and use unique output IDs.
+CLI mappings add their source glyphs at their command-line position; later
+selectors can remove them. Repeated mappings accumulate, with the last
+request for each source glyph taking precedence.
 Custom mappings cannot be combined with `--retain-gids`. Add
 `--retain-num-glyphs` alongside `--retain-gids` to keep the source glyph count
 by appending empty glyphs. `--iftb-requirements` forces 32-bit `loca`, `gvar`,

skera/src/main.rs

@@ -68,8 +68,8 @@ struct Args {
     keep_everything: Vec<bool>,
 
     /// Original:new glyph ID pairs preserving glyph order, for example 1:4,2:7.
-    #[arg(long = "gid-map", alias = "glyph-map")]
-    glyph_map: Option<String>,
+    #[arg(long = "gid-map", alias = "glyph-map", action = clap::ArgAction::Append)]
+    glyph_map: Vec<String>,
 
     /// List of Unicode codepoints
     #[arg(short, long, action = clap::ArgAction::Append)]
@@ -245,7 +245,13 @@ fn main() {
     let args = Args::from_arg_matches(&matches).unwrap();
 
     let subset_flags = parse_subset_flags(&matches);
-    let glyph_mapping = parse_glyph_mapping(args.glyph_map.as_deref().unwrap_or_default())
+    let glyph_mapping = args
+        .glyph_map
+        .iter()
+        .try_fold(Vec::new(), |mut mapping, input| {
+            mapping.extend(parse_glyph_mapping(input)?);
+            Ok::<_, SubsetError>(mapping)
+        })
         .unwrap_or_else(|err| {
             eprintln!("{err}");
             std::process::exit(1);
@@ -309,12 +315,25 @@ fn main() {
             "glyphs_add",
             "glyphs_remove",
             "glyphs_file",
+            "glyph_map",
         ],
     ) {
         if name == "keep_everything" {
             gids = IntSet::all();
             continue;
         }
+        if name == "glyph_map" {
+            gids.extend(
+                parse_glyph_mapping(input)
+                    .unwrap_or_else(|err| {
+                        eprintln!("{err}");
+                        std::process::exit(1);
+                    })
+                    .into_iter()
+                    .map(|(old, _)| old),
+            );
+            continue;
+        }
         let from_file = name.ends_with("_file");
         let inputs = if from_file {
             read_selector_file(input, true)
@@ -334,7 +353,6 @@ fn main() {
             apply_selection(&mut gids, name, selected);
         }
     }
-    gids.extend(glyph_mapping.iter().map(|&(old, _)| old));
     let default_drop_tables = {
         [
             // Layout disabled by default.
@@ -490,7 +508,7 @@ fn main() {
             &name_ids,
             &name_languages,
         );
-        if args.glyph_map.is_some() {
+        if !args.glyph_map.is_empty() {
             plan.set_glyph_mapping(&glyph_mapping)
                 .unwrap_or_else(|err| {
                     eprintln!("{err}");

skera/src/parsing_util.rs

@@ -87,7 +87,8 @@ pub fn parse_glyph_mapping(input: &str) -> Result<Vec<(GlyphId, GlyphId)>, Subse
         return Ok(Vec::new());
     }
     input
-        .split(',')
+        .split(|c: char| c == ',' || c.is_ascii_whitespace())
+        .filter(|pair| !pair.is_empty())
         .map(|pair| {
             let invalid = || SubsetError::InvalidGlyphMapping(pair.into());
             let (old, new) = pair.trim().split_once(':').ok_or_else(invalid)?;

skera/tests/cli_selection_test.rs

@@ -690,3 +690,62 @@ fn varc_preliminary_subsets_keep_inputs_until_instancing_and_selection_finish()
         expected.maxp().unwrap().num_glyphs()
     );
 }
+
+#[test]
+fn glyph_maps_include_glyphs_in_order_and_accumulate_repeated_options() {
+    use write_fonts::read::TableProvider;
+    let input = "test-data/fonts/Roboto-Regular.abc.ttf";
+    for (options, expected) in [
+        (vec!["--gid-map=1:5", "--gids=2"], vec!["--gids=2"]),
+        (vec!["--gid-map=1:5", "--gids-=1"], vec!["--gids="]),
+        (
+            vec!["--gids=2", "--gid-map=1:5"],
+            vec!["--gid-map=1:5", "--gids=1,2"],
+        ),
+        (
+            vec![
+                "--gid-map=1:5",
+                "--keep-everything",
+                "--gids-=1",
+                "--unicodes-=61",
+            ],
+            vec!["--keep-everything", "--gids=2,3", "--unicodes="],
+        ),
+        (
+            vec!["--gid-map=1:5", "--gid-map=2:7"],
+            vec!["--gid-map=1:5,2:7"],
+        ),
+        (vec!["--gid-map=1:5 2:7,"], vec!["--gid-map=1:5,2:7"]),
+        (
+            vec!["--gid-map=1:5", "--glyph-map=1:7"],
+            vec!["--gid-map=1:7"],
+        ),
+        (
+            vec!["--gid-map=1:5", "--gid-map=", "--glyph-map=2:7"],
+            vec!["--gid-map=1:5,2:7"],
+        ),
+        (
+            vec!["--gid-map=1:5", "--glyphs=uni0062", "--gid-map=2:7"],
+            vec!["--gid-map=2:7"],
+        ),
+    ] {
+        assert_eq!(
+            cli_subset(input, &options),
+            cli_subset(input, &expected),
+            "{options:?}"
+        );
+    }
+    let bytes = cli_subset(input, &["--gid-map=1:5", "--gids=2"]);
+    assert_eq!(
+        FontRef::new(&bytes).unwrap().maxp().unwrap().num_glyphs(),
+        2
+    );
+    let directory = tempfile::tempdir().unwrap();
+    let file = directory.path().join("glyphs.txt");
+    std::fs::write(&file, "gid2\n").unwrap();
+    let option = format!("--glyphs-file={}", file.display());
+    assert_eq!(
+        cli_subset(input, &["--gid-map=1:5", &option, "--gids-=1"]),
+        cli_subset(input, &["--gids=2"])
+    );
+}

skera/tests/glyph_mapping_test.rs

@@ -176,7 +176,7 @@ fn invalid_mappings_leave_the_plan_intact_and_empty_requests_reset_it() {
     assert!(retained
         .old_to_new_glyph_mapping()
         .all(|(old, new)| old == new));
-    for input in ["1", "1:", ":2", "1:2:3", "-1:2", "1:-2", "1:2,"] {
+    for input in ["1", "1:", ":2", "1:2:3", "-1:2", "1:-2"] {
         assert!(skera::parse_glyph_mapping(input).is_err(), "{input}");
     }
 }