add supporting wildcard in windows

[wzshiming]

No description provided.

[x1ddos]

Could you provide a command sample which would work with this change but otherwise doesn't.

[x1ddos]

This is already added in #7

[x1ddos]

So, executing addlicense * doesn't work on Windows?
What kind of "glob" works/doesn't natively on Windows and how much is it different from filepath.Glob?

[wzshiming]

Windows wildcards are handled by the program itself, so the program receives the * text instead of matching files.

[x1ddos]

Ok, sounds reasonable. Although PowerShell does support expansion.
Either way, I wonder what the implications are for the cases were a file name contains meta/expansion characters recognized by filepath.Match. Currently, filepath.Glob does this check:

// hasMeta reports whether path contains any of the magic characters
// recognized by Match.
func hasMeta(path string) bool {
	// TODO(niemeyer): Should other magic characters be added here?
	return strings.ContainsAny(path, "*?[")
}

What if a path is something like ./mydir[hello] and it's passed verbatim in a shell which supports expansion like bash:

addlicense ./mydir\[hello\]/*

It might have undesirable effect, triggering filepath.Glob to try and expand ./mydir[hello].

We could add separate files with +build tags for Windows and Linux, but even on Windows, how would that work in DOS vs PowerShell without a confusion what expands/doesn't and at which point the expansion is done? Because I'm pretty sure filepath.Match has different set of expansion rules compared to PowerShell.

[x1ddos]

An alternative for Windows, when not in PowerShell, could be to have another separate little program called expand and then feed the output of the expand to addlicense. It is probably out of scope for addlicense repo though.

[x1ddos]

@marcacohen maybe you have other ideas?

[wzshiming]

PowerShell is also only partial command support, which is also command itself support, not PowerShell support.
So I don't think you have to worry about the difference DOS vs PowerShell.

[wzshiming]

Passed verbatim in a shell which supports expansion, this is a problem.

[x1ddos]

Another alternative is to add a cmd line flag like -expand or -expand-glob, false by default. Then, Windows users can enable it explicitly, whenever needed.

…

On Sun, 22 Jul 2018, 12:03 Shiming Zhang, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In main.go <#6 (comment)>: > @@ -95,7 +95,17 @@ type file struct { mode os.FileMode } -func walk(ch chan<- *file, start string) { +func walk(ch chan<- *file, path string) { + matches, _ := filepath.Glob(path) Passed verbatim in a shell which supports expansion, this is a problem. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#6 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABjPYKg1eQbZ7zYXT7Cs-DmGjMq_xn4ks5uJE3kgaJpZM4VT9tA> .

[mco-gh]

If the use case here is 'addlicense *', then would a non-globbing work around would be 'addlicense .' ?

I realize that does help the case of more refined globs (e.g. *.js) but I also feel like using Powershell (or Cygwin) might be acceptable vs. having to build essentially another mode into this comment ("local file globbing") or jump through other hoops for a limited range of use cases. How bad would it be to document this as a limitation of using addlicense with the dos shell?

[x1ddos]

Another way to solve this is to move func walk or the matching algorithm into a separate file like walk.go and then have another one named walk_windows.go so that addlicense for Windows expands paths within the program and relies on a shell expansion otherwise. Or use build tags instead of _windows.go suffix.

[wzshiming]

This also needs to distinguish between shells, preferably only in the case of dos shell.

[x1ddos]

This also needs to distinguish between shells, preferably only in the case of dos shell.

I thought last time you said:

So I don't think you have to worry about the difference DOS vs PowerShell.

Or did you mean something else?

[wzshiming]

I got it wrong. no problem.