On Wed, Feb 28, 2018 at 12:29 PM, Alban Gruin <alban.gruin@xxxxxxxxx> wrote: > This adds xfuncname and word_regex patterns for golang, a quite > popular programming language. It also includes test cases for the > xfuncname regex (t4018) and an updated documentation. s/an // > The xfuncname regex finds functions, structs and interfaces. The > word_regex pattern finds identifiers, integers, floats, complex > numbers and operators, according to the go specification. > > Signed-off-by: Alban Gruin <alban.gruin@xxxxxxxxx> > --- > diff --git a/t/t4018/golang-complex-function b/t/t4018/golang-complex-function > @@ -0,0 +1,8 @@ > +func (t *Test) RIGHT(a Type) (Type, error) { > + t.a = a > + return ChangeMe, nil > +} Most of these files use a mix of spaces and tabs for indentation. Please use tabs only. > diff --git a/userdiff.c b/userdiff.c > @@ -38,6 +38,15 @@ IPATTERN("fortran", > +PATTERNS("golang", > + /* Functions */ > + "^[ \t]*(func[ \t]*.*(\\{[ \t]*)?)\n" Why is the brace (and possible following whitespace) optional? Considering that the language demands that the brace be on the same line, I'd think the brace should be mandatory. I suppose you made whitespace after 'func' optional to be able to recognize a method (which hasn't been gofmt'd): func(x *X) name { rather than the more typical: func (x *X) name { I wonder if it would make sense to tighten the expression to recognize functions and methods as distinct cases: function: mandatory whitespace following 'func' method: optional whitespace but mandatory '(' following 'func' Your current expression could accidentally match: /* Fish like to have functors for lunch { just like eels}. */ but, even the suggested tighter expression could "accidentally" match example code in a comment block anyhow, so I guess it probably doesn't matter much in practice. > + /* Structs and interfaces */ > + "^[ \t]*(type[ \t].*(struct|interface)[ \t]*(\\{[ \t]*)?)", Whitespace is required after 'type'. Good. The language doesn't require '{' to be on the same line as 'struct' or 'interface', and this expression makes it optional. Okay. > + /* -- */ > + "[a-zA-Z_][a-zA-Z0-9_]*" > + "|[-+0-9.eE]+i?|0[xX]?[0-9a-fA-F]+i?" > + "|[-+*/<>%&^|=!:]=|--|\\+\\+|<<=?|>>=?|&\\^=?|&&|\\|\\||<-|\\.{3}"),