Skip to content

Commit

Permalink
Merge pull request #758 from googlefonts/deduplicate_glyphname
Browse files Browse the repository at this point in the history
remove the GlyphName defined in fea-rs, and use the one in fontdrasil
  • Loading branch information
qxliu76 authored Mar 28, 2024
2 parents b6fbe7f + ad86575 commit 2cd0468
Show file tree
Hide file tree
Showing 9 changed files with 24 additions and 20 deletions.
5 changes: 1 addition & 4 deletions fea-rs/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use std::fmt::{Display, Formatter};

use smol_str::SmolStr;
use fontdrasil::types::GlyphName;
pub use write_fonts::types::GlyphId;

mod glyph_class;
Expand All @@ -15,9 +15,6 @@ pub use glyph_map::GlyphMap;

use crate::compile::Anchor;

/// A glyph name
pub type GlyphName = SmolStr;

/// A glyph or glyph class.
///
/// Various places in the FEA spec accept either a single glyph or a glyph class.
Expand Down
17 changes: 13 additions & 4 deletions fea-rs/src/common/glyph_map.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use write_fonts::tables::post::Post;

use super::{GlyphId, GlyphIdent, GlyphName};
use super::{GlyphId, GlyphIdent};
use fontdrasil::types::GlyphName;
use std::{
borrow::Cow,
collections::{BTreeMap, HashMap},
Expand Down Expand Up @@ -134,7 +135,9 @@ impl FromIterator<GlyphIdent> for GlyphMap {
}

mod sealed {
use super::{super::GlyphIdent, GlyphName};
use super::super::GlyphIdent;
use fontdrasil::types::GlyphName;
use smol_str::SmolStr;

/// Something that is either a Cid or a glyph name.
///
Expand All @@ -158,9 +161,15 @@ mod sealed {
}
}

impl AsGlyphIdent for SmolStr {
fn named(&self) -> Option<&str> {
Some(self.as_str())
}
}

impl AsGlyphIdent for GlyphName {
fn named(&self) -> Option<&str> {
Some(self)
Some(self.as_str())
}
}

Expand All @@ -173,7 +182,7 @@ mod sealed {
impl AsGlyphIdent for GlyphIdent {
fn named(&self) -> Option<&str> {
if let GlyphIdent::Name(name) = self {
Some(name)
Some(name.as_str())
} else {
None
}
Expand Down
4 changes: 2 additions & 2 deletions fea-rs/src/compile.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
//! Compiling OpenType Layout tables

use crate::{parse::ParseTree, DiagnosticSet, GlyphMap};
use fontdrasil::types::GlyphName;
use write_fonts::types::GlyphId;

use crate::{parse::ParseTree, DiagnosticSet, GlyphMap, GlyphName};

use self::{
compile_ctx::CompilationCtx,
error::{FontGlyphOrderError, GlyphOrderError},
Expand Down
2 changes: 1 addition & 1 deletion fea-rs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub mod util;
#[cfg(test)]
mod tests;

pub use common::{GlyphIdent, GlyphMap, GlyphName, GlyphSet};
pub use common::{GlyphIdent, GlyphMap, GlyphSet};
pub use compile::{Compiler, Opts};
pub use diagnostic::{Diagnostic, DiagnosticSet, Level};
pub use parse::{ParseTree, TokenSet};
Expand Down
3 changes: 2 additions & 1 deletion fea-rs/src/parse/grammar/glyph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,8 @@ mod tests {
use super::*;
use crate::parse::FileId;
use crate::token_tree::AstSink;
use crate::{GlyphMap, GlyphName};
use crate::GlyphMap;
use fontdrasil::types::GlyphName;

#[test]
fn name_like() {
Expand Down
3 changes: 2 additions & 1 deletion fea-rs/src/tests/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ use std::path::{Path, PathBuf};
use crate::{
compile::{error::CompilerError, Compiler, MockVariationInfo, NopFeatureProvider, Opts},
util::ttx::{self as test_utils, Report, TestCase, TestResult},
GlyphMap, GlyphName,
GlyphMap,
};
use fontdrasil::types::GlyphName;

static ROOT_TEST_DIR: &str = "./test-data/compile-tests";
static GOOD_DIR: &str = "good";
Expand Down
2 changes: 1 addition & 1 deletion fontbe/src/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ impl Work<Context, AnyWorkId, Error> for FeatureParsingWork {
let features = context.ir.features.get();
let glyph_order = context.ir.glyph_order.get();
let static_metadata = context.ir.static_metadata.get();
let glyph_map = glyph_order.iter().map(|g| g.clone().into_inner()).collect();
let glyph_map = glyph_order.iter().cloned().collect();

let result = self.parse(&features, &glyph_map);

Expand Down
2 changes: 1 addition & 1 deletion fontbe/src/features/kern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ impl Work<Context, AnyWorkId, Error> for KerningGatherWork {
let arc_fragments = context.kern_fragments.all();
let ast = context.fea_ast.get();
let glyph_order = context.ir.glyph_order.get();
let glyph_map = glyph_order.iter().map(|g| g.clone().into_inner()).collect();
let glyph_map = glyph_order.iter().cloned().collect();
let mut fragments: Vec<_> = arc_fragments
.iter()
.map(|(_, fragment)| fragment.as_ref())
Expand Down
6 changes: 1 addition & 5 deletions fontbe/src/features/marks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,7 @@ impl Work<Context, AnyWorkId, Error> for MarkWork {
let groups = create_mark_to_base_groups(&anchors, &glyph_order);

let mut all_marks = FeaRsMarks {
glyphmap: glyph_order
.iter()
.cloned()
.map(GlyphName::into_inner)
.collect(),
glyphmap: glyph_order.iter().cloned().collect(),
..Default::default()
};

Expand Down

0 comments on commit 2cd0468

Please sign in to comment.