Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose file to modifyReved/modifyUnreved #40

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

crzidea
Copy link

@crzidea crzidea commented Aug 27, 2015

Hi, I am writing a task to support replace relative path. Below is my code snippet:

gulp.task('html', ['revision'], function () {
    var manifest = gulp.src('.tmp/rev-manifest.json');

    var srcPath = path.join(__dirname, 'app');
    var revReplaceOptions = {};
    revReplaceOptions.manifest = manifest;
    function modifyDistPath(distPath, file) {
        var fileRelativePath = path.relative(srcPath, file.path);
        var fileDirname = path.dirname(fileRelativePath);
        var distPathRelativePath = path.relative(fileDirname, distPath);
        if (/^../.test(distPathRelativePath)) {
            return distPathRelativePath
        }
        return distPath;
    }
    revReplaceOptions.modifyUnreved = modifyDistPath;
    revReplaceOptions.modifyReved = modifyDistPath;

    return gulp.src('app/**/*.html')
        .pipe($.revReplace(revReplaceOptions))
        .pipe(gulp.dest('dist'));
});

So I need the file.path as an argument to support the feature. The PR is to support this.

@jamesknelson
Copy link
Owner

Thanks @crzidea. If you could update test.js to test for the new argument, I'd be happy to merge.

@crzidea
Copy link
Author

crzidea commented Sep 15, 2015

I will update test.
And should I update README about this feature?

@jamesknelson
Copy link
Owner

Yep, updating README would be great too, thanks.

@ankon
Copy link

ankon commented Mar 10, 2016

I'm also having issues with relative paths and the rev-replace plugin, if this could be merged it would help me a lot. Can I help moving this forward?

@crzidea
Copy link
Author

crzidea commented Mar 11, 2016

@ankon Yes, please!
I will be grateful if you can take over this!

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.

3 participants