metalsmith-concat

metalsmith-concat copied to clipboard

On Windows (process.platform === 'win32') the path separator is \, not / as used here:

https://github.com/aymericbeaumet/metalsmith-concat/blob/4a93c964c331cfb1c937b7d486d893f47aa8c118/index.js#L67

Looks like this is attempting to "fix" Windows to be Unix-like, but this is wrong, Metalsmith does use \ path separators in the files object.

I think I implemented this to address https://github.com/aymericbeaumet/metalsmith-concat/issues/8.

Can you tell me a little bit more about your use-case? Ideally providing a repro of something that is not working for you.

@emmercm: Can you give a try to https://github.com/aymericbeaumet/metalsmith-concat/pull/43?

Hi @aymericbeaumet I used https://github.com/aymericbeaumet/metalsmith-concat/blob/e6a128c99ad10d5e16b3faa3e6f170f96ee684ad/index.js in my project and it didn't work as expected.

As far as repro - running Metalsmith (v2.3.0 in my case) on Windows will use the native path separator of \ in the files object, e.g.:

index.js
static\js\scripts.js
static\css\styles.css

But the metalsmithifyPath() and backslashToSlash() functions will cause the result of the plugin to be output with a filename with Unix path separators always, even on Windows machines.

I think the change is as simple as:

function metalsmithifyPath(pathToReplace) {
	return pathToReplace.replace(/[/\\]+/g, path.sep);
}

which tested fine, but I haven't thought through additional side-effects that might have.

Can you share your metalsmith configuration so I can repro this?

The output is supposed to be written at the file pointed at by options.output (with the path separator of your choice):

https://github.com/aymericbeaumet/metalsmith-concat/blob/e6a128c99ad10d5e16b3faa3e6f170f96ee684ad/index.js#L140)

Also I updated documentation for options.files and options.output, can you have a look?

Not seeing a difference in options.output, but the update to options.files is a good one - though I was already using forward slashes with the glob pattern.

---

So I definitely wasn't doing the right thing in my build, I was using forward slashes for options.output, but because of metalsmithifyPath() it didn't matter.

For reproduction, a bare minimum build such as:

const Metalsmith = require('metalsmith');
const concat = require('metalsmith-concat');

const path = require('path');

Metalsmith(__dirname)
    .source('./src')
    .use(concat({
        files: '**/*',
        output: path.join('deeply', 'nested', 'file')
    }))
    .destination('./build')
    .clean(true)
    .build((err) => {
        if (err) {
            throw err;
        }
    });

where path.join('deeply', 'nested', 'file') === 'deeply\\nested\\file' on Windows, output is converted to all forward slashes here where it shouldn't be:

https://github.com/aymericbeaumet/metalsmith-concat/blob/4a93c964c331cfb1c937b7d486d893f47aa8c118/index.js#L77

Are you trying with the latest version I pushed? It's not on npm, you have to use the git hash. I do not normalize the output anymore, which should address the issue you are reporting. https://github.com/aymericbeaumet/metalsmith-concat/blob/e6a128c99ad10d5e16b3faa3e6f170f96ee684ad/index.js#L83