pnpm / cmd-shim

The cmd-shim used in pnpm
Other
14 stars 11 forks source link

Implement read-cmd-shim via a command-based directive instead of regexp parsing #19

Open cspotcode opened 5 years ago

cspotcode commented 5 years ago

Over on npm/cmd-shim, it was mentioned that changing the shim to implement #17 will require a change to read-cmd-shim to parse the new implementation. I guess read-cmd-shim uses a regexp to parse the target path from the shim.

I think it'd be better going forward to embed the target path in a way that can be parsed out more easily. For example, at the top of every shim is a comment like this:

@REM __CMD_SHIM_TARGET__=../foo/bin/bar.js

...or...

# __CMD_SHIM_TARGET__=../foo/bin/bar.js

read-cmd-shim can check for this directive. If found, that's the target path. If not, it'll fall-back to legacy regexp matching. This is a more future-proof approach to parsing the target out of a shim because the shims are free to change in the future. As long as they include that directive, they can be read unambiguously.

...of course I'm not sure who uses read-cmd-shim or what they use it for. So maybe this is not worth the effort. I still want to share the idea.

I also think, if read-cmd-shim is a useful feature, it makes more sense to include it in cmd-shim than to implement as a separate module. It's not much code and putting them in 2 places is just lots of duplicated boilerplate and configuration.

ExE-Boss commented 5 years ago

When linking to external websites, you have to specify the scheme, eg.:

[read-cmd-shim](https://npm.im/read-cmd-shim)

instead of

[read-cmd-shim](npm.im/read-cmd-shim)

as the latter produces a relative link to https://github.com/pnpm/cmd-shim/issues/npm.im/read-cmd-shim, which doesn't exist.