vanjacosic / gulp-asset-manifest

gulp plugin for adding generated assets to a manifest file
MIT License
7 stars 4 forks source link

pathSeparator seems to have no effect? #7

Closed johnnyreilly closed 10 years ago

johnnyreilly commented 10 years ago

Hello again!,

I've just been experimenting with the options.pathSeparator property and it doesn't seem to have any effect.I've been supplying a pathSeparator of "/" but the output of my manifest looks like this:

{"scripts":["scripts\\angular.js","scripts\\angular-animate.js","scripts\\angular-route.js","scripts\\angular-sanitize.js","scripts\\angular-ui\\ui-bootstrap-tpls.js","scripts\\toastr.js","scripts\\moment.js","scripts\\spin.js","scripts\\underscore.js","app\\app.js","app\\config.route.js","app\\common\\common.js","app\\common\\logger.js","app\\common\\spinner.js","app\\common\\bootstrap\\bootstrap.dialog.js","app\\directives\\imgPerson.js","app\\directives\\serverError.js","app\\directives\\sidebar.js","app\\directives\\spinner.js","app\\directives\\waiter.js","app\\directives\\widgetClose.js","app\\directives\\widgetHeader.js","app\\directives\\widgetMinimize.js","app\\services\\datacontext.js","app\\services\\repositories.js","app\\services\\repository.sage.js","app\\services\\repository.saying.js","app\\about\\about.js","app\\admin\\admin.js","app\\dashboard\\dashboard.js","app\\layout\\shell.js","app\\layout\\sidebar.js","app\\layout\\topnav.js","app\\sages\\sageDetail.js","app\\sages\\sageEdit.js","app\\sages\\sages.js","app\\sayings\\sayingEdit.js","app\\sayings\\sayings.js"],"styles":["content\\ie10mobile.css","content\\bootstrap.css","content\\font-awesome.css","content\\toastr.css","content\\styles.css"]}

So still using the `"\\"`` as a separator (I'm on Windows).

In case it helps the gulpfile I'm using is here. (This is part of a little toy project of mine that I use to test out technologies.)

johnnyreilly commented 10 years ago

@vanjacosic - I think I have it. The code in npm (0.0.4) looks like it's out of date. This is the index.js I downloaded from npm:

// Requirements
var map = require('map-stream');
var gutil = require('gulp-util');
var path = require('path');
var fs = require('fs');

// Helper function
function errorMessage(message){
    throw new gutil.PluginError('gulp-asset-manifest', message);
}

function checkManifestFile(filename) {
    // Check if manifest file exists
    return fs.existsSync(filename, function (exists) {
        return exists;
    });
}

function readManifestFile(filename) {
    // Read data from manifest file
    return fs.readFileSync(filename, 'utf8', function(err, data) {
        if (err) {
            errorMessage('Error reading manifest file.');
        }
        return data;
    });
}

function writeManifestFile(data, filename) {
    // Write data to manifest file
    fs.writeFileSync(filename, JSON.stringify(data));
}

function resetManifestFile(bundlename, filename) {
    // Check if manifest file exists
    var doesFileExist = checkManifestFile(filename);

    if(doesFileExist){
        // Read manifest file contents
        var contents = readManifestFile(filename);

        // Copy data into file list
        fileList = JSON.parse(contents);

        // Reset or create array for each bundle
        fileList[bundlename] = [];
    }
    else{
        // Create empty file list
        fileList = {};
    }

    // Write file list to manifest file
    writeManifestFile(fileList, filename);
}

// Plugin function
module.exports = function(options) {

    // Reset file list
    var fileList;

    // Prepare options
    options = options || {};
    options.manifestFile = options.manifestFile || 'asset_manifest.json';

    var pathPrepend = options.pathPrepend || '';

    if(!options.bundleName){
        errorMessage('A bundle name is required. Please refer to the docs.');
    }

    if (options.log) {
        gutil.log('Preparing bundle:', gutil.colors.green(options.bundleName));
    }

    // Reset asset file
    resetManifestFile(options.bundleName, options.manifestFile);

    // Process files
    return map(function(file, callback) {

        // Let empty files pass
        if (file.isNull()) {
            return callback(null, file);
        }

        // Emit error for streams
        if (file.isStream()) {
            errorMessage('Streams are not supported');
        }

        // Read asset file contents
        var contents = readManifestFile(options.manifestFile);

        // Copy data into file list
        fileList = JSON.parse(contents);

        var filename;

        // Retrieve filename
        if (options.includeRelativePath) {
            filename =  path.relative(process.cwd(), file.path);
        }
        else {
            filename = path.basename(file.path);
        }

        // Add filename to fileList
        if (!fileList[options.bundleName]){
            fileList[options.bundleName] = [];
        }

        fileList[options.bundleName].push(pathPrepend + filename);

        // Write list to asset file
        writeManifestFile(fileList, options.manifestFile);

        if (options.log) {
            gutil.log('Added', gutil.colors.green(filename), 'to asset manifest.');
        }

        callback(null, file);
    });
};

As you can see this does not feature the pathSeparator at all whereas the code on GitHub does. I think you might need to update npm with the latest version of index.js.

johnnyreilly commented 10 years ago

Yup - bingo. I just manually copied the index.js from GitHub to my project and it all worked as expected. It would be great if you could update npm. Thanks again for your hard work!

vanjacosic commented 10 years ago

@johnnyreilly oh wow, I feel like an idiot :smile:

I took a look at the problem yesterday but couldn't immediately figure out what was wrong, so decided to take a closer look today.

But you of course beat me to it, I totally forgot to publish to npm after the last two PRs.

Thanks for letting me know - and please keep it up. It's so great to get feedback on this stuff :)

johnnyreilly commented 10 years ago

No problem @vanjacosic - it happens to us all! Do you know when you'll get a chance to publish?

vanjacosic commented 10 years ago

@johnnyreilly Yeah (I guess I didn't state that very explicitly) but I did an npm publish right before I closed the issue :)

johnnyreilly commented 10 years ago

Fantastic!