glob() regular file paths on Windows, drop --glob parameter

[mprobst]

See comment from review (that I didn't see on Friday, sorry for the late drive by comment!):

Could you add docs that this is of course a Windows-only issue, and that on UNIXens including Macs the shell itself is doing the globbing?

To that point, I think it might be smarter to not have --glob=...., but rather just check if the machine is running on Windows, and then glob over the regular arguments. That'd allow users to run mostly the same command line on Linux, Macs, and Windows, which is nice for consistent development environments.

[mprobst]

@filipesilva @alexeagle

[filipesilva]

The way I read #21, I thought clang-format binary didn't support globs. I tried it in a linux machine and it seems to be working for nested directories though.

I can definitely do what you suggest @mprobst , but I'd like to better understand the original issue then so that I don't incorrectly fix it again.

@alexeagle, what functionality does the clang-format binary not have in linux for which globs are needed?

[mprobst]

@filipesilva on Linux, your shell resolves globs for you. E.g. even if you type echo *, the * is being replaced by your shell interpreter. On Windows, cmd.exe does not do this, so each and every tool has to implement globbing itself (sigh).

So my suggestion would be to detect that we're running on Windows, and only there expand globs in the passed in list of files. That means every user, Linux or not, can run clang-format *.js and have it work as expected.

I don't understand @alexeagle's comment about nested directories either, shells can glob with nested directories just fine (clang-format */src/*.ts). For arbitrary nesting, you can always use find (find src/ modules/ whatever/ -iname '*.js' | xargs clang-format -i).

So I'm not entirely sure what we're trying to fix here?

[alexeagle]

I want /*.ts minus node_modules//*.ts - that's the one I can't do with
just glob

On Mon, Feb 15, 2016 at 3:27 AM Martin Probst notifications@github.com
wrote:

@filipesilva https://github.com/filipesilva on Linux, your shell
resolves globs for you. E.g. even if you type echo *, the * is being
replaced by your shell interpreter. On Windows, cmd.exe does not do this,
so each and every tool has to implement globbing itself (sigh).

So my suggestion would be to detect that we're running on Windows, and
only there expand globs in the passed in list of files. That means every
user, Linux or not, can run clang-format *.js and have it work as
expected.

I don't understand @alexeagle https://github.com/alexeagle's comment
about nested directories either, shells can glob with nested directories
just fine (clang-format /src/.ts). For arbitrary nesting, you can
always use find (find src/ modules/ whatever/ -iname '*.js' | xargs
clang-format -i).

—
Reply to this email directly or view it on GitHub
#25 (comment)
.

[alexeagle]

And I think in a nodejs environment, globbing is well understood but piping together the find command is more advanced

[filipesilva]

So how about this, to satisfy both this issue and #24:

• dropping the --glob= arg
• taking all args that do not start with - and using them to create globs on windows
• adding an --exclude= arg that takes a glob, this argument can appear any number of times
• when there is least one --exclude, or the command is run on windows, node-glob will be used
• otherwise the command is passed directly to the clang-format binary

[mprobst]

Generally SGTM, one comment:

• when there is least one --exclude, or the command is run on windows,
  node-glob will be used

I presume we then just run node-glob to process the excludes?

There's also one potential confusion: node-glob understands "*/.js", i.e.
the double star path glob. Shells don't. So if you run "clang-format
*/.js", your shell may or may not expand some of those stars, and then we
might still have to run node-glob afterwards to resolve those **s.

That makes me thing we might want to unconditionally always run node-glob
on the resulting paths, for consistency?

[filipesilva]

I wasn't aware there would be a difference with the doublestar, but if the behavior might be different then it is better to always do node-glob, yes.

[filipesilva]

Apologies for the delay in getting to this. I put up a PR with proposed changes, which aren't quite what we discussed but I explain my reasoning there.

See #26