Skip to content

Issue #6 - Changed 'use' to override the default plugins#8

Open
ianbrandt wants to merge 1 commit into
ecomfe:masterfrom
ianbrandt:issue-6
Open

Issue #6 - Changed 'use' to override the default plugins#8
ianbrandt wants to merge 1 commit into
ecomfe:masterfrom
ianbrandt:issue-6

Conversation

@ianbrandt

Copy link
Copy Markdown

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

@nikoladev

Copy link
Copy Markdown

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
Copy Markdown
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

chpio commented Sep 26, 2018

Copy link
Copy Markdown

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

specious commented Aug 5, 2021

Copy link
Copy Markdown

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

@specious specious left a comment

Copy link
Copy Markdown

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 thread index.js
Comment on lines +95 to +97
if (text && opts.chineseOnly) {
text = text.replace(/[^\u4e00-\u9fa5]/g, '');
}

Copy link
Copy Markdown

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