Skip no longer accepts partial paths

[marstr]

Formerly, -skip=baz/*/foo.go would skip any file that ended with that pattern. More specifically, an absolute path of the form /home/travis/baz/bar/foo.go would be skipped. However, after switching from path/filepath to GitHub.com/ryanuber/go-glob to resolve globs, the same file will be scanned. This change was made in commit 1a481fa.

[gcmurphy]

@endophage any thoughts on this?

[endophage]

I believe this specific case will now require another * on the front of the skip path. The full string must be matched and it's simple globbing where * matches a substring of any length.

@marstr by way of explanation, the Golang implementation of filepath globbing only matches * against a single segment and doesn't recognize ** as meaning "any number of segments." Per my testing I did and added in #94, baz/*/foo.go probably wasn't working the way you expected in the first place anyway. That pattern would only match paths where there was exactly one path segment between baz and foo.go, i.e. baz/bar/bash/foo.go would not have been matched.

@gcmurphy permitted me to modify the pathing to use simple globbing, but it does now require a full match. We could update this to assume if the first character isn't / (i.e. based against the root of the FS) or * (already wildcarded), we prefix a * on the basis that's probably the behaviour the user wants.

[marstr]

Your suggested fix of adding a wildcard to the beginning of the pattern is indeed what we did to fix this in our repository. For our repository, it happens to be irrelevant whether /*/ matches a single or multiple segments, but a single segment matches my expectations.

Mostly, I wanted to make sure that folks here were aware that this had broken the paths in our continuous integration script by changing the effective requirement from /<somePattern>$/ to /^<somePattern>$/. If that's okay with folks, it's fine with me.

[endophage]

@marstr the biggest thing the change fixed is that the existing pattern, **/*_test.go that was default included to filter out test files didn't actually work. It only matched test files exactly one dir deep from your pwd when using ./....

I'll leave the decision up to @gcmurphy but I'm happy to do the change to prefix * when it makes sense. I think the updated path matching though (this issue not withstanding) is generally much better than Golang's filepath globbing behaviour.

[gcmurphy]

So what cases would it make sense? Would something like this work?

func prependWildcard(pattern string) string {
	char := string([]rune(pattern)[0])
	indicators := `*./\`
	if ! strings.ContainsAny(char, indicators) {
   		return  "*" + pattern
	}
	return pattern
}

If the logic gets too gnarly here it might be easier to focus on improving the documentation.

[endophage]

Yeah, I thought about this over the weekend and docs might be simpler. Thinking back to my Python days, I was always a fan of "Explicit is better than implicit."