font: allow non-boolean font feature settings (#4139)

\+ much more flexible syntax and lenient parser
\+ allows comma-separated list as a single config value

This allows, e.g. `cv01 = 2` to select the second variant of `cv01`.

Resolves #3128 

Parser could probably be a little smaller than it is- would be a lot
cleaner with the labeled switch continue pattern from Zig 0.14. Maybe
should've put it in its own file too...

I spent *much* too long trying to test this with `cv01` with
[monaspace](https://github.com/githubnext/monaspace) before realizing
that the README refers to v1.2 but the latest released version (and
hence the one I had installed) was v1.101 -- I installed the v1.2
version and tested with both CoreText and HarfBuzz and successfully set
`cv01 = 2` and got the expected result.

Feel free to make any stylistic changes you feel necessary before
merging.

src/config/Config.zig

@@ -147,23 +147,28 @@ const c = @cImport({
 /// By default, synthetic styles are enabled.
 @"font-synthetic-style": FontSyntheticStyle = .{},
 
-/// Apply a font feature. This can be repeated multiple times to enable multiple
+/// Apply a font feature. To enable multiple font features you can repeat
-/// font features. You can NOT set multiple font features with a single value
+/// this multiple times or use a comma-separated list of feature settings.
-/// (yet).
+///
+/// The syntax for feature settings is as follows, where `feat` is a feature:
+///
+///   * Enable features with e.g. `feat`, `+feat`, `feat on`, `feat=1`.
+///   * Disabled features with e.g. `-feat`, `feat off`, `feat=0`.
+///   * Set a feature value with e.g. `feat=2`, `feat = 3`, `feat 4`.
+///   * Feature names may be wrapped in quotes, meaning this config should be
+///     syntactically compatible with the `font-feature-settings` CSS property.
+///
+/// The syntax is fairly loose, but invalid settings will be silently ignored.
 ///
 /// The font feature will apply to all fonts rendered by Ghostty. A future
 /// enhancement will allow targeting specific faces.
 ///
-/// A valid value is the name of a feature. Prefix the feature with a `-` to
-/// explicitly disable it. Example: `ss20` or `-ss20`.
-///
 /// To disable programming ligatures, use `-calt` since this is the typical
 /// feature name for programming ligatures. To look into what font features
 /// your font has and what they do, use a font inspection tool such as
 /// [fontdrop.info](https://fontdrop.info).
 ///
-/// To generally disable most ligatures, use `-calt`, `-liga`, and `-dlig` (as
+/// To generally disable most ligatures, use `-calt, -liga, -dlig`.
-/// separate repetitive entries in your config).
 @"font-feature": RepeatableString = .{},
 
 /// Font size in points. This value can be a non-integer and the nearest integer

src/font/shape.zig

@@ -1,6 +1,7 @@
 const builtin = @import("builtin");
 const options = @import("main.zig").options;
 const run = @import("shaper/run.zig");
+const feature = @import("shaper/feature.zig");
 pub const noop = @import("shaper/noop.zig");
 pub const harfbuzz = @import("shaper/harfbuzz.zig");
 pub const coretext = @import("shaper/coretext.zig");
@@ -8,6 +9,9 @@ pub const web_canvas = @import("shaper/web_canvas.zig");
 pub const Cache = @import("shaper/Cache.zig");
 pub const TextRun = run.TextRun;
 pub const RunIterator = run.RunIterator;
+pub const Feature = feature.Feature;
+pub const FeatureList = feature.FeatureList;
+pub const default_features = feature.default_features;
 
 /// Shaper implementation for our compile options.
 pub const Shaper = switch (options.backend) {
@@ -49,10 +53,7 @@ pub const Cell = struct {
 
 /// Options for shapers.
 pub const Options = struct {
-    /// Font features to use when shaping. These can be in the following
+    /// Font features to use when shaping.
-    /// formats: "-feat" "+feat" "feat". A "-"-prefix is used to disable
-    /// a feature and the others are used to enable a feature. If a feature
-    /// isn't supported or is invalid, it will be ignored.
     ///
     /// Note: eventually, this will move to font.Face probably as we may
     /// want to support per-face feature configuration. For now, we only

src/font/shaper/coretext.zig

@@ -7,6 +7,9 @@ const trace = @import("tracy").trace;
 const font = @import("../main.zig");
 const os = @import("../../os/main.zig");
 const terminal = @import("../../terminal/main.zig");
+const Feature = font.shape.Feature;
+const FeatureList = font.shape.FeatureList;
+const default_features = font.shape.default_features;
 const Face = font.Face;
 const Collection = font.Collection;
 const DeferredFace = font.DeferredFace;
@@ -40,9 +43,10 @@ pub const Shaper = struct {
     /// The string used for shaping the current run.
     run_state: RunState,
 
-    /// The font features we want to use. The hardcoded features are always
+    /// CoreFoundation Dictionary which represents our font feature settings.
-    /// set first.
+    features: *macos.foundation.Dictionary,
-    features: FeatureList,
+    /// A version of the features dictionary with the default features excluded.
+    features_no_default: *macos.foundation.Dictionary,
 
     /// The shared memory used for shaping results.
     cell_buf: CellBuf,
@@ -100,51 +104,17 @@ pub const Shaper = struct {
         }
     };
 
-    /// List of font features, parsed into the data structures used by
+    /// Create a CoreFoundation Dictionary suitable for
-    /// the CoreText API. The CoreText API requires a pretty annoying wrapping
+    /// settings the font features of a CoreText font.
-    /// to setup font features:
+    fn makeFeaturesDict(feats: []const Feature) !*macos.foundation.Dictionary {
-    ///
+        const list = try macos.foundation.MutableArray.create();
-    ///   - The key parsed into a CFString
-    ///   - The value parsed into a CFNumber
-    ///   - The key and value are then put into a CFDictionary
-    ///   - The CFDictionary is then put into a CFArray
-    ///   - The CFArray is then put into another CFDictionary
-    ///   - The CFDictionary is then passed to the CoreText API to create
-    ///     a new font with the features set.
-    ///
-    /// This structure handles up to the point that we have a CFArray of
-    /// CFDictionary objects representing the font features and provides
-    /// functions for creating the dictionary to init the font.
-    const FeatureList = struct {
-        list: *macos.foundation.MutableArray,
-
-        pub fn init() !FeatureList {
-            var list = try macos.foundation.MutableArray.create();
         errdefer list.release();
-            return .{ .list = list };
-        }
 
-        pub fn deinit(self: FeatureList) void {
+        for (feats) |feat| {
-            self.list.release();
+            const value_num: c_int = @intCast(feat.value);
-        }
-
-        /// Append the given feature to the list. The feature syntax is
-        /// the same as Harfbuzz: "feat" enables it and "-feat" disables it.
-        pub fn append(self: *FeatureList, name_raw: []const u8) !void {
-            // If the name is `-name` then we are disabling the feature,
-            // otherwise we are enabling it, so we need to parse this out.
-            const name = if (name_raw[0] == '-') name_raw[1..] else name_raw;
-            const dict = try featureDict(name, name_raw[0] != '-');
-            defer dict.release();
-            self.list.appendValue(macos.foundation.Dictionary, dict);
-        }
-
-        /// Create the dictionary for the given feature and value.
-        fn featureDict(name: []const u8, v: bool) !*macos.foundation.Dictionary {
-            const value_num: c_int = @intFromBool(v);
 
             // Keys can only be ASCII.
-            var key = try macos.foundation.String.createWithBytes(name, .ascii, false);
+            var key = try macos.foundation.String.createWithBytes(&feat.tag, .ascii, false);
             defer key.release();
             var value = try macos.foundation.Number.create(.int, &value_num);
             defer value.release();
@@ -154,50 +124,44 @@ pub const Shaper = struct {
                     macos.text.c.kCTFontOpenTypeFeatureTag,
                     macos.text.c.kCTFontOpenTypeFeatureValue,
                 },
-                &[_]?*const anyopaque{
+                &[_]?*const anyopaque{ key, value },
-                    key,
-                    value,
-                },
             );
-            errdefer dict.release();
+            defer dict.release();
-            return dict;
-        }
 
-        /// Returns the dictionary to use with the font API to set the
+            list.appendValue(macos.foundation.Dictionary, dict);
-        /// features. This should be released by the caller.
+        }
-        pub fn attrsDict(
-            self: FeatureList,
-            omit_defaults: bool,
-        ) !*macos.foundation.Dictionary {
-            // Get our feature list. If we're omitting defaults then we
-            // slice off the hardcoded features.
-            const list = if (!omit_defaults) self.list else list: {
-                const list = try macos.foundation.MutableArray.createCopy(@ptrCast(self.list));
-                for (hardcoded_features) |_| list.removeValue(0);
-                break :list list;
-            };
-            defer if (omit_defaults) list.release();
 
         var dict = try macos.foundation.Dictionary.create(
             &[_]?*const anyopaque{macos.text.c.kCTFontFeatureSettingsAttribute},
             &[_]?*const anyopaque{list},
         );
         errdefer dict.release();
+
         return dict;
     }
-    };
-
-    // These features are hardcoded to always be on by default. Users
-    // can turn them off by setting the features to "-liga" for example.
-    const hardcoded_features = [_][]const u8{ "dlig", "liga" };
 
     /// The cell_buf argument is the buffer to use for storing shaped results.
     /// This should be at least the number of columns in the terminal.
     pub fn init(alloc: Allocator, opts: font.shape.Options) !Shaper {
-        var feats = try FeatureList.init();
+        var feature_list: FeatureList = .{};
-        errdefer feats.deinit();
+        defer feature_list.deinit(alloc);
-        for (hardcoded_features) |name| try feats.append(name);
+        for (opts.features) |feature_str| {
-        for (opts.features) |name| try feats.append(name);
+            try feature_list.appendFromString(alloc, feature_str);
+        }
+
+        // We need to construct two attrs dictionaries for font features;
+        // one without the default features included, and one with them.
+        const feats = feature_list.features.items;
+        const feats_df = try alloc.alloc(Feature, feats.len + default_features.len);
+        defer alloc.free(feats_df);
+
+        @memcpy(feats_df[0..default_features.len], &default_features);
+        @memcpy(feats_df[default_features.len..], feats);
+
+        const features = try makeFeaturesDict(feats_df);
+        errdefer features.release();
+        const features_no_default = try makeFeaturesDict(feats);
+        errdefer features_no_default.release();
 
         var run_state = RunState.init();
         errdefer run_state.deinit(alloc);
@@ -242,7 +206,8 @@ pub const Shaper = struct {
             .alloc = alloc,
             .cell_buf = .{},
             .run_state = run_state,
-            .features = feats,
+            .features = features,
+            .features_no_default = features_no_default,
             .writing_direction = writing_direction,
             .cached_fonts = .{},
             .cached_font_grid = 0,
@@ -255,7 +220,8 @@ pub const Shaper = struct {
     pub fn deinit(self: *Shaper) void {
         self.cell_buf.deinit(self.alloc);
         self.run_state.deinit(self.alloc);
-        self.features.deinit();
+        self.features.release();
+        self.features_no_default.release();
         self.writing_direction.release();
 
         {
@@ -509,8 +475,8 @@ pub const Shaper = struct {
         // If we have it, return the cached attr dict.
         if (self.cached_fonts.items[index_int]) |cached| return cached;
 
-        // Features dictionary, font descriptor, font
+        // Font descriptor, font
-        try self.cf_release_pool.ensureUnusedCapacity(self.alloc, 3);
+        try self.cf_release_pool.ensureUnusedCapacity(self.alloc, 2);
 
         const run_font = font: {
             // The CoreText shaper relies on CoreText and CoreText claims
@@ -533,8 +499,10 @@ pub const Shaper = struct {
             const face = try grid.resolver.collection.getFace(index);
             const original = face.font;
 
-            const attrs = try self.features.attrsDict(face.quirks_disable_default_font_features);
+            const attrs = if (face.quirks_disable_default_font_features)
-            self.cf_release_pool.appendAssumeCapacity(attrs);
+                self.features_no_default
+            else
+                self.features;
 
             const desc = try macos.text.FontDescriptor.createWithAttributes(attrs);
             self.cf_release_pool.appendAssumeCapacity(desc);

src/font/shaper/feature.zig

@@ -0,0 +1,390 @@
+const std = @import("std");
+const builtin = @import("builtin");
+const assert = std.debug.assert;
+const Allocator = std.mem.Allocator;
+
+const log = std.log.scoped(.font_shaper);
+
+/// Represents an OpenType font feature setting, which consists of a tag and
+/// a numeric parameter >= 0. Most features are boolean, so only parameters
+/// of 0 and 1 make sense for them, but some (e.g. 'cv01'..'cv99') can take
+/// parameters to choose between multiple variants of a given character or
+/// characters.
+///
+/// Ref:
+/// - https://learn.microsoft.com/en-us/typography/opentype/spec/chapter2#features-and-lookups
+/// - https://harfbuzz.github.io/shaping-opentype-features.html
+pub const Feature = struct {
+    tag: [4]u8,
+    value: u32,
+
+    pub fn fromString(str: []const u8) ?Feature {
+        var fbs = std.io.fixedBufferStream(str);
+        const reader = fbs.reader();
+        return Feature.fromReader(reader);
+    }
+
+    /// Parse a single font feature setting from a std.io.Reader, with a version
+    /// of the syntax of HarfBuzz's font feature strings. Stops at end of stream
+    /// or when a ',' is encountered.
+    ///
+    /// This parsing aims to be as error-tolerant as possible while avoiding any
+    /// assumptions in ambiguous scenarios. When invalid syntax is encountered,
+    /// the reader is advanced to the next boundary (end-of-stream or ',') so
+    /// that further features may be read.
+    ///
+    /// Ref: https://harfbuzz.github.io/harfbuzz-hb-common.html#hb-feature-from-string
+    pub fn fromReader(reader: anytype) ?Feature {
+        var tag: [4]u8 = undefined;
+        var value: ?u32 = null;
+
+        // TODO: when we move to Zig 0.14 this can be replaced with a
+        //       labeled switch continue pattern rather than this loop.
+        var state: union(enum) {
+            /// Initial state.
+            start: void,
+            /// Parsing the tag, data is index.
+            tag: u2,
+            /// In the space between the tag and the value.
+            space: void,
+            /// Parsing an integer parameter directly in to `value`.
+            int: void,
+            /// Parsing a boolean keyword parameter ("on"/"off").
+            bool: void,
+            /// Encountered an unrecoverable syntax error, advancing to boundary.
+            err: void,
+            /// Done parsing feature.
+            done: void,
+        } = .start;
+        while (true) {
+            // If we hit the end of the stream we just pretend it's a comma.
+            const byte = reader.readByte() catch ',';
+            switch (state) {
+                // If we're done then we skip whitespace until we see a ','.
+                .done => switch (byte) {
+                    ' ', '\t' => continue,
+                    ',' => break,
+                    // If we see something other than whitespace or a ','
+                    // then this is an error since the intent is unclear.
+                    else => {
+                        state = .err;
+                        continue;
+                    },
+                },
+
+                // If we're fast-forwarding from an error we just wanna
+                // stop at the first boundary and ignore all other bytes.
+                .err => if (byte == ',') return null,
+
+                .start => switch (byte) {
+                    // Ignore leading whitespace.
+                    ' ', '\t' => continue,
+                    // Empty feature string.
+                    ',' => return null,
+                    // '+' prefix to explicitly enable feature.
+                    '+' => {
+                        value = 1;
+                        state = .{ .tag = 0 };
+                        continue;
+                    },
+                    // '-' prefix to explicitly disable feature.
+                    '-' => {
+                        value = 0;
+                        state = .{ .tag = 0 };
+                        continue;
+                    },
+                    // Quote mark introducing a tag.
+                    '"', '\'' => {
+                        state = .{ .tag = 0 };
+                        continue;
+                    },
+                    // First letter of tag.
+                    else => {
+                        tag[0] = byte;
+                        state = .{ .tag = 1 };
+                        continue;
+                    },
+                },
+
+                .tag => |*i| switch (byte) {
+                    // If the tag is interrupted by a comma it's invalid.
+                    ',' => return null,
+                    // Ignore quote marks.
+                    '"', '\'' => continue,
+                    // A prefix of '+' or '-'
+                    // In all other cases we add the byte to our tag.
+                    else => {
+                        tag[i.*] = byte;
+                        if (i.* == 3) {
+                            state = .space;
+                            continue;
+                        }
+                        i.* += 1;
+                    },
+                },
+
+                .space => switch (byte) {
+                    ' ', '\t' => continue,
+                    // Ignore quote marks since we might have a
+                    // closing quote from the tag still ahead.
+                    '"', '\'' => continue,
+                    // Allow an '=' (which we can safely ignore)
+                    // only if we don't already have a value due
+                    // to a '+' or '-' prefix.
+                    '=' => if (value != null) {
+                        state = .err;
+                        continue;
+                    },
+                    ',' => {
+                        // Specifying only a tag turns a feature on.
+                        if (value == null) value = 1;
+                        break;
+                    },
+                    '0'...'9' => {
+                        // If we already have value because of a
+                        // '+' or '-' prefix then this is an error.
+                        if (value != null) {
+                            state = .err;
+                            continue;
+                        }
+                        value = byte - '0';
+                        state = .int;
+                        continue;
+                    },
+                    'o', 'O' => {
+                        // If we already have value because of a
+                        // '+' or '-' prefix then this is an error.
+                        if (value != null) {
+                            state = .err;
+                            continue;
+                        }
+                        state = .bool;
+                        continue;
+                    },
+                    else => {
+                        state = .err;
+                        continue;
+                    },
+                },
+
+                .int => switch (byte) {
+                    ',' => break,
+                    '0'...'9' => {
+                        // If our value gets too big while
+                        // parsing we consider it an error.
+                        value = std.math.mul(u32, value.?, 10) catch {
+                            state = .err;
+                            continue;
+                        };
+                        value.? += byte - '0';
+                    },
+                    else => {
+                        state = .err;
+                        continue;
+                    },
+                },
+
+                .bool => switch (byte) {
+                    ',' => return null,
+                    'n', 'N' => {
+                        // "ofn"
+                        if (value != null) {
+                            assert(value == 0);
+                            state = .err;
+                            continue;
+                        }
+                        value = 1;
+                        state = .done;
+                        continue;
+                    },
+                    'f', 'F' => {
+                        // To make sure we consume two 'f's.
+                        if (value == null) {
+                            value = 0;
+                        } else {
+                            assert(value == 0);
+                            state = .done;
+                            continue;
+                        }
+                    },
+                    else => {
+                        state = .err;
+                        continue;
+                    },
+                },
+            }
+        }
+
+        assert(value != null);
+
+        return .{
+            .tag = tag,
+            .value = value.?,
+        };
+    }
+
+    /// Serialize this feature to the provided buffer.
+    /// The string that this produces should be valid to parse.
+    pub fn toString(self: *const Feature, buf: []u8) !void {
+        var fbs = std.io.fixedBufferStream(buf);
+        try self.format("", .{}, fbs.writer());
+    }
+
+    /// Formatter for logging
+    pub fn format(
+        self: Feature,
+        comptime layout: []const u8,
+        opts: std.fmt.FormatOptions,
+        writer: anytype,
+    ) !void {
+        _ = layout;
+        _ = opts;
+        if (self.value <= 1) {
+            // Format boolean options as "+tag" for on and "-tag" for off.
+            try std.fmt.format(writer, "{c}{s}", .{
+                "-+"[self.value],
+                self.tag,
+            });
+        } else {
+            // Format non-boolean tags as "tag=value".
+            try std.fmt.format(writer, "{s}={d}", .{
+                self.tag,
+                self.value,
+            });
+        }
+    }
+};
+
+/// A list of font feature settings (see `Feature` for more documentation).
+pub const FeatureList = struct {
+    features: std.ArrayListUnmanaged(Feature) = .{},
+
+    pub fn deinit(self: *FeatureList, alloc: Allocator) void {
+        self.features.deinit(alloc);
+    }
+
+    /// Parse a comma separated list of features.
+    /// See `Feature.fromReader` for more docs.
+    pub fn fromString(alloc: Allocator, str: []const u8) !FeatureList {
+        var self: FeatureList = .{};
+        try self.appendFromString(alloc, str);
+        return self;
+    }
+
+    /// Append features to this list from a string with a comma separated list.
+    /// See `Feature.fromReader` for more docs.
+    pub fn appendFromString(
+        self: *FeatureList,
+        alloc: Allocator,
+        str: []const u8,
+    ) !void {
+        var fbs = std.io.fixedBufferStream(str);
+        const reader = fbs.reader();
+        while (fbs.pos < fbs.buffer.len) {
+            const i = fbs.pos;
+            if (Feature.fromReader(reader)) |feature| {
+                try self.features.append(alloc, feature);
+            } else log.warn(
+                "failed to parse font feature setting: \"{s}\"",
+                .{fbs.buffer[i..fbs.pos]},
+            );
+        }
+    }
+
+    /// Formatter for logging
+    pub fn format(
+        self: FeatureList,
+        comptime layout: []const u8,
+        opts: std.fmt.FormatOptions,
+        writer: anytype,
+    ) !void {
+        for (self.features.items, 0..) |feature, i| {
+            try feature.format(layout, opts, writer);
+            if (i != std.features.items.len - 1) try writer.writeAll(", ");
+        }
+        if (self.value <= 1) {
+            // Format boolean options as "+tag" for on and "-tag" for off.
+            try std.fmt.format(writer, "{c}{s}", .{
+                "-+"[self.value],
+                self.tag,
+            });
+        } else {
+            // Format non-boolean tags as "tag=value".
+            try std.fmt.format(writer, "{s}={d}", .{
+                self.tag,
+                self.value,
+            });
+        }
+    }
+};
+
+/// These features are hardcoded to always be on by default. Users
+/// can turn them off by setting the features to "-liga" for example.
+pub const default_features = [_]Feature{
+    .{ .tag = "dlig".*, .value = 1 },
+    .{ .tag = "liga".*, .value = 1 },
+};
+
+test "Feature.fromString" {
+    const testing = std.testing;
+
+    // This is not *complete* coverage of every possible
+    // combination of syntax, but it covers quite a few.
+
+    // Boolean settings (on)
+    const kern_on = Feature{ .tag = "kern".*, .value = 1 };
+    try testing.expectEqual(kern_on, Feature.fromString("kern"));
+    try testing.expectEqual(kern_on, Feature.fromString("kern, "));
+    try testing.expectEqual(kern_on, Feature.fromString("kern on"));
+    try testing.expectEqual(kern_on, Feature.fromString("kern on, "));
+    try testing.expectEqual(kern_on, Feature.fromString("+kern"));
+    try testing.expectEqual(kern_on, Feature.fromString("+kern, "));
+    try testing.expectEqual(kern_on, Feature.fromString("\"kern\" = 1"));
+    try testing.expectEqual(kern_on, Feature.fromString("\"kern\" = 1, "));
+
+    // Boolean settings (off)
+    const kern_off = Feature{ .tag = "kern".*, .value = 0 };
+    try testing.expectEqual(kern_off, Feature.fromString("kern off"));
+    try testing.expectEqual(kern_off, Feature.fromString("kern off, "));
+    try testing.expectEqual(kern_off, Feature.fromString("-'kern'"));
+    try testing.expectEqual(kern_off, Feature.fromString("-'kern', "));
+    try testing.expectEqual(kern_off, Feature.fromString("\"kern\" = 0"));
+    try testing.expectEqual(kern_off, Feature.fromString("\"kern\" = 0, "));
+
+    // Non-boolean settings
+    const aalt_2 = Feature{ .tag = "aalt".*, .value = 2 };
+    try testing.expectEqual(aalt_2, Feature.fromString("aalt=2"));
+    try testing.expectEqual(aalt_2, Feature.fromString("aalt=2, "));
+    try testing.expectEqual(aalt_2, Feature.fromString("'aalt' 2"));
+    try testing.expectEqual(aalt_2, Feature.fromString("'aalt' 2, "));
+
+    // Various ambiguous/error cases which should be null
+    try testing.expectEqual(null, Feature.fromString("aalt=2x")); // bad number
+    try testing.expectEqual(null, Feature.fromString("toolong")); // tag too long
+    try testing.expectEqual(null, Feature.fromString("sht")); // tag too short
+    try testing.expectEqual(null, Feature.fromString("-kern 1")); // redundant/conflicting
+    try testing.expectEqual(null, Feature.fromString("-kern on")); // redundant/conflicting
+    try testing.expectEqual(null, Feature.fromString("aalt=o,")); // bad keyword
+    try testing.expectEqual(null, Feature.fromString("aalt=ofn,")); // bad keyword
+}
+
+test "FeatureList.fromString" {
+    const testing = std.testing;
+
+    const str =
+        "  kern, kern on , +kern, \"kern\"  = 1," ++ // Boolean settings (on)
+        "kern    off, -'kern' , \"kern\"=0," ++ // Boolean settings (off)
+        "aalt=2,  'aalt'\t2," ++ // Non-boolean settings
+        "aalt=2x, toolong, sht, -kern 1, -kern on, aalt=o, aalt=ofn," ++ // Invalid cases
+        "last"; // To ensure final element is included correctly.
+    var feats = try FeatureList.fromString(testing.allocator, str);
+    defer feats.deinit(testing.allocator);
+    try testing.expectEqualSlices(
+        Feature,
+        &(.{Feature{ .tag = "kern".*, .value = 1 }} ** 4 ++
+            .{Feature{ .tag = "kern".*, .value = 0 }} ** 3 ++
+            .{Feature{ .tag = "aalt".*, .value = 2 }} ** 2 ++
+            .{Feature{ .tag = "last".*, .value = 1 }}),
+        feats.features.items,
+    );
+}

src/font/shaper/harfbuzz.zig

@@ -3,6 +3,10 @@ const assert = std.debug.assert;
 const Allocator = std.mem.Allocator;
 const harfbuzz = @import("harfbuzz");
 const font = @import("../main.zig");
+const terminal = @import("../../terminal/main.zig");
+const Feature = font.shape.Feature;
+const FeatureList = font.shape.FeatureList;
+const default_features = font.shape.default_features;
 const Face = font.Face;
 const Collection = font.Collection;
 const DeferredFace = font.DeferredFace;
@@ -10,7 +14,6 @@ const Library = font.Library;
 const SharedGrid = font.SharedGrid;
 const Style = font.Style;
 const Presentation = font.Presentation;
-const terminal = @import("../../terminal/main.zig");
 
 const log = std.log.scoped(.font_shaper);
 
@@ -27,38 +30,37 @@ pub const Shaper = struct {
     cell_buf: CellBuf,
 
     /// The features to use for shaping.
-    hb_feats: FeatureList,
+    hb_feats: []harfbuzz.Feature,
 
     const CellBuf = std.ArrayListUnmanaged(font.shape.Cell);
-    const FeatureList = std.ArrayListUnmanaged(harfbuzz.Feature);
-
-    // These features are hardcoded to always be on by default. Users
-    // can turn them off by setting the features to "-liga" for example.
-    const hardcoded_features = [_][]const u8{ "dlig", "liga" };
 
     /// The cell_buf argument is the buffer to use for storing shaped results.
     /// This should be at least the number of columns in the terminal.
     pub fn init(alloc: Allocator, opts: font.shape.Options) !Shaper {
-        // Parse all the features we want to use. We use
+        // Parse all the features we want to use.
-        var hb_feats = hb_feats: {
+        const hb_feats = hb_feats: {
-            var list = try FeatureList.initCapacity(alloc, opts.features.len + hardcoded_features.len);
+            var feature_list: FeatureList = .{};
-            errdefer list.deinit(alloc);
+            defer feature_list.deinit(alloc);
-
+            try feature_list.features.appendSlice(alloc, &default_features);
-            for (hardcoded_features) |name| {
+            for (opts.features) |feature_str| {
-                if (harfbuzz.Feature.fromString(name)) |feat| {
+                try feature_list.appendFromString(alloc, feature_str);
-                    try list.append(alloc, feat);
-                } else log.warn("failed to parse font feature: {s}", .{name});
             }
 
-            for (opts.features) |name| {
+            var list = try alloc.alloc(harfbuzz.Feature, feature_list.features.items.len);
-                if (harfbuzz.Feature.fromString(name)) |feat| {
+            errdefer alloc.free(list);
-                    try list.append(alloc, feat);
+
-                } else log.warn("failed to parse font feature: {s}", .{name});
+            for (feature_list.features.items, 0..) |feature, i| {
+                list[i] = .{
+                    .tag = std.mem.nativeToBig(u32, @bitCast(feature.tag)),
+                    .value = feature.value,
+                    .start = harfbuzz.c.HB_FEATURE_GLOBAL_START,
+                    .end = harfbuzz.c.HB_FEATURE_GLOBAL_END,
+                };
             }
 
             break :hb_feats list;
         };
-        errdefer hb_feats.deinit(alloc);
+        errdefer alloc.free(hb_feats);
 
         return Shaper{
             .alloc = alloc,
@@ -71,7 +73,7 @@ pub const Shaper = struct {
     pub fn deinit(self: *Shaper) void {
         self.hb_buf.destroy();
         self.cell_buf.deinit(self.alloc);
-        self.hb_feats.deinit(self.alloc);
+        self.alloc.free(self.hb_feats);
     }
 
     pub fn endFrame(self: *const Shaper) void {
@@ -125,10 +127,10 @@ pub const Shaper = struct {
                 // If we are disabling default font features we just offset
                 // our features by the hardcoded items because always
                 // add those at the beginning.
-                break :i hardcoded_features.len;
+                break :i default_features.len;
             };
 
-            harfbuzz.shape(face.hb_font, self.hb_buf, self.hb_feats.items[i..]);
+            harfbuzz.shape(face.hb_font, self.hb_buf, self.hb_feats[i..]);
         }
 
         // If our buffer is empty, we short-circuit the rest of the work