Skip to content

Conversation

@ianbrandt
Copy link

The 'use' parameter now overrides the default fontmin plugins instead of augmenting them, per Issue #6.

@nikoladev
Copy link

This pull request totally works. Thanks!

To anybody trying to figure out how to use it:

gulp.task('subset', function () {
    var gulpFontmin = require('gulp-fontmin');
    var Fontmin = require('fontmin');
    return gulp.src('path/to/font.ttf')
        .pipe(gulpFontmin({
            use: [Fontmin.glyph({
                text: '天地玄黄 宇宙洪荒'
            })]
        }))
        .pipe(gulp.dest('destination/to/subset/fonts/'));
});

@ianbrandt
Copy link
Author

@junmer, This has been working without issue for me for some time, and also has a confirmation from @nikoladev. I'd love to move off "gulp-fontmin": "ianbrandt/gulp-fontmin.git#issue-6" to a release version. Any chance for a merge?

@chpio
Copy link

chpio commented Sep 26, 2018

Any chance for a merge?

Nope, fontmin is dead

Latest commit 52c76b2 on Apr 10, 2016

-- gulp-fontmin

Latest commit 8f579ff on May 5, 2016

-- fontmin

@specious
Copy link

specious commented Aug 5, 2021

@junmer, I don't mean to bother you, but could you please take a brief look at merging this pull request?

Copy link

@specious specious left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent idea for the default list of fontmin plugins to take effect only when the user hasn't provided their own list. However, it would also make sense to still let the opts.chineseOnly option be applied regardless of that.

Comment on lines +95 to +97
if (text && opts.chineseOnly) {
text = text.replace(/[^\u4e00-\u9fa5]/g, '');
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like opts.ChineseOnly should take effect regardless of whether custom opts.use were passed. I would consider taking this part back out of this conditional block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants