Add globIgnore arg #26
Conversation
| @@ -31,9 +31,9 @@ function clangFormat(file, enc, style, done) { | |||
| function spawnClangFormat(args, done, stdio) { | |||
| // WARNING: This function's interface should stay stable across versions for the cross-version | |||
| // loading below to work. | |||
| if (args.indexOf('-version') !== -1 || args.indexOf('--version') !== -1) { | |||
| if (args.indexOf('--version') !== -1) { | |||
There was a problem hiding this comment.
This used to support -version and --version, please don't break that?
There was a problem hiding this comment.
I removed intercepting of -version so that it could be passed onto the native binary (enabling getting it's version), and so that all intercepted commands would be the double-dash ones.
It's not very obvious at all without reading the readme though, and it is a breaking change. This is confined to the last commit so it should be easy to remove from the PR. Do you want me to do so?
There was a problem hiding this comment.
@mprobst can you still answer this?
There was a problem hiding this comment.
I think --version and -version should behave identically, and both give the version of the NPM package, i.e. should be intercepted.
There was a problem hiding this comment.
Roger, will modify to do that. I'll also squash everything into a single commit.
|
@alexeagle, maybe this is something for you, as you reviewed the original PR? |
|
Closing this out, I don't think we'll get to this. |
Following discussion on #25, I worked on adding an exclude functionality, and also wanted to always use
node-globfor all patterns.When I was thinking about how to parse the args, I figured that a whitelist would be needed so that I could separate between what args to let through to the
clang-formatbinary and which should I try to glob.I also tried to figure a good way to support multiple globs together with multiple ignore patterns, and it began sounding more difficult than I initially thought.
So I ended up only adding the
--globIgnore=argument. I thought it would be simpler and more obvious. If a user wants nativeclang-formatbehaviour he will use everything but the double dash args.This left out version. I refactored it so that
-versionis passed to the native binary, but--versionis used by the wrapper. This way the wrapper only has to process double dash args throughout.I also had to slightly refactor the tests to run on windows though, the path in
EXPECTED_VERSION_STRINGwasn't playing well with windows paths, but now it should work.This might be too much for a single PR but these changes followed one another so I left them.