Support exclamation mark to create non-matching list in tail plugin

[sspaink]

Required for all PRs:

• Signed CLA.
• Associated README.md updated.
• Has appropriate unit tests.

Fixes #7419 by converting [! to [^ which allows both characters to be used.

The Tail plugin depends on the standard Go function filepath.Glob which uses filepath.Match and this function does not support ! only ^, evidently this was an oversight during implementation: golang/go#41394. Hopefully in the second version of the filepath package both characters will be supported natively and we can remove this change.

Also updated the relevant unit test in globpath_test.go to use table driven tests.

Thanks!

[ivorybilled]

Makes sense to me.

[srebhan]

Would love to use ReplaceAll() instead of the Replace() function with -1 for readability. Furthermore, \\[! is a valid pattern matching literal [! but is now replaced to \\[^ with your change. Even though I think it is unlikely to be used in the wild, the probability is not zero. Could you please check this corner-case!?

[sspaink]

@srebhan thanks for the review! So because Go doesn't support negative lookahead in regular expressions I am having a hard time thinking of a clean way to check for this edge case, do you have any suggestions?

[srebhan]

Well

	re := regexp.MustCompile(`(?:[^\\])(\[!)`)
	re.ReplaceAllString(s, "[^")

should work according to my test...

[sspaink]

@srebhan So I couldn't get the regex to work exactly like we would want to. I did find another open source project doublestar that supports both double star and !/^ for negation, it does remove the custom logic that globpath has for handling doublestar which is nice. What do you think?

@jagularr the code has changed quite a bit from when you reviewed it after my latest change in case you wanted to look at it again as well.

[ssoroka]

replace ** with **/** for backwards compatibility

[srebhan]

@sspaink what's wrong with the regexp I posted? Could you give me an example where it fails!? Requiring an external dependency to replace a regexp seems odd...

[sspaink]

@srebhan The two issues I noticed when you run the playground example you provided: lala[!w]ee --> lal[^w]ee the result overrides the 'a' in the second 'la' and then if there isn't any characters trailing the negation class it doesn't update: [!p]laid --> [!p]laid.

@ssoroka provided a regex that solved the second issue, playground but the first problem where the previous character gets replaced still happens.

[ssoroka]

re using a library, glob is used in a lot of different places and I'd really love consistent behavior that–ideally–meet some standard that we are not managing ourselves. It seems too easy to accidentally create our own fork of glob behavior, which it seems is exactly what we've done and now we have to support this old broken behavior...

[srebhan]

@sspaink fixed both with

package main

import (
	"fmt"
	"regexp"
)

func main() {
	test := []string {
		"lala[!w]ee",
		"[!p]laid",
		"foo\\[!wja",
		"\\[!wbar",
		"foo[!abc] and \\[!wbar are wunderbar",
	}

	re := regexp.MustCompile(`([^\\]?)(\[!)`)
	for _, s := range test {
		fmt.Printf("%s --> %s\n", s, re.ReplaceAllString(s, "$1[^"))
	}
}

But if I understand @ssoroka correctly he prefers to use the doublestar-package. Also fine with me. :-)

[sspaink]

@srebhan thank you for the updated regular expression, I've updated the pull request to use the suggested change and it looks like it works without having to change any of the existing tests.
@ssoroka I did like the idea of moving to the library but I couldn't figure out how to cleanly replace **->**/** without causing the issue you describe **/**->**/**/**/**. Maybe we can move this to a library safely for a 2.0 release.

[srebhan]

Looks good to me.