Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add supporting wildcard in windows #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

@x1ddos x1ddos Jul 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@wzshiming wzshiming Jul 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marcacohen maybe you have other ideas?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if matches == nil {
matches = []string{path}
}
for _, start := range matches {
walkPath(ch, start)
}
}

func walkPath(ch chan<- *file, start string) {
filepath.Walk(start, func(path string, fi os.FileInfo, err error) error {
if err != nil {
log.Printf("%s error: %v", path, err)
Expand Down