Consider using node-glob instead of shell expansion

[christiannaths]

First, awesome lib! Using this for our internal developer's wiki and it's such a nice & simple solution.

Context

My package.json

{
 "scripts": {
    "build": "embedme **/*.md"
  },
  "devDependencies": {
    "embedme": "^1.15.0",
  }
}

Problem

Using shell expansion for globbing leads to inconsistencies across platforms.

Because npm is using /bin/sh, and I'm on OSX, I'm stuck with Bash 3.x, and globbing doesn't work the same as it does my usual shell (zsh). Results may vary among other devs on my team.

So given the context above, when I run npm run build a file like foo/bar/baz/doc.md will not be included. However, that file will be included for developer whose /binb/sh points to Bash 4.x, for example.

Proposed Solution

Many node libraries use node-glob to solve this. The downside is that a user must quote glob patterns in their scripts entries, however this pattern is so common that it's easy to find this solution when things don't work as expected.

Workarounds

Currently, I'm handling this (rather naively) with this:

// scripts/build.js

const glob = require('glob');
const { execSync } = require('child_process');

const PATTERN = '**/*.md';

function init(pattern) {
  glob(pattern, {}, function(error, files) {
    const cmd = ['embedme', ...files].join(' ');
    execSync(cmd, { stdio: 'inherit' });
  });
}

init(PATTERN);

{
  "scripts": {
    "build": "node scripts/build.js"
  },
  "devDependencies": {
    "embedme": "^1.15.0",
    "glob": "^7.1.4"
  }
}

---

I'd be more than happy to contribute with a PR, but I wanted to raise this for discussion first.

[zakhenry]

Hey Christian, Thanks for the kind words, glad you found it handy. On globing I’d gladly accept a PR for this - I wondered about it briefly when implementing but thought the native shell expansion would be good enough but you make an excellent point about cross platform problems. node-glob would be an great choice. One note though - to swap to quoted paths would be considered a breaking change, however it should be possible to make this change non-breaking by detecting if the string is quoted then picking the right strategy. Let me know what you think :)

On Sun, 21 Jul 2019 at 5:08 pm, Christian Naths ***@***.***> wrote: First, awesome lib! Using this for our internal developer's wiki and it's such a nice & simple solution. Context My package.json { "scripts": { "build": "embedme **/*.md" }, "devDependencies": { "embedme": "^1.15.0", } } Problem Using shell expansion for globbing leads to inconsistencies across platforms. Because npm is using /bin/sh, and I'm on OSX, I'm stuck with Bash 3.x, and globbing doesn't work the same as it does my usual shell (zsh). Results may vary among other devs on my team. So given the context above, when I run npm run build a file like foo/bar/baz/doc.md will not be included. However, that file will be included for developer whose /binb/sh points to Bash 4.x, for example. Proposed Solution Many node libraries use node-glob to solve this. The downside is that a user must quote glob patterns in their scripts entries, however this pattern is so common that it's easy to find this solution when things don't work as expected. Workarounds Currently, I'm handling this (rather naively) with this: // scripts/build.js const glob = require('glob');const { execSync } = require('child_process'); const PATTERN = '**/*.md'; function init(pattern) { glob(pattern, {}, function(error, files) { const cmd = ['embedme', ...files].join(' '); execSync(cmd, { stdio: 'inherit' }); }); } init(PATTERN); { "scripts": { "build": "node scripts/build.js" }, "devDependencies": { "embedme": "^1.15.0", "glob": "^7.1.4" } } ------------------------------ I'd be more than happy to contribute with a PR, but I wanted to raise this for discussion first. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#35?email_source=notifications&email_token=AAFQE2KKI7Y33SJW52L5F2TQASCWZA5CNFSM4IFSZIP2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HAPZWDQ>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFQE2PJKUMTVOASGJUHKGDQASCWZANCNFSM4IFSZIPQ> .

-- Zak Henry

[christiannaths]

Yeah, good call on it being a breaking change. I think the idea to pick a strategy based on quoted/non-quoted glob patterns makes perfect sense. I'll try to find some time this week to put a PR in for this 👍

[caesarcc]

Hy dear, I'm trying to use npx embedme wiki/*.md, but it's not working for me. I'm getting "File wiki/*.md doesn't exists" which seems that the wildcard is interpreted as a file name. Am I doing something wrong or is it related to this reported bug?
Using npx embedme wiki/script1.md wiki/script.2md wiki/script3.md is working very well.

I'm using Windows and I built the project with node 11.13

[christiannaths]

@caesarcc What happens when you do ls wiki/*.md? Same error? Or does it list the files? Also, what shell are you using? (try $(echo $SHELL) --version, or which sh)

Wups, my bad. Just read the part about how you're on Windows. I'm afraid I can't help, I have no idea how globbing works on Windows.

[zakhenry]

🎉 This issue has been resolved in version 1.17.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀