Add a gogrep talk link into a readme? #44

quasilyte · 2019-12-18T13:25:47Z

I think the documentation could be slightly improved with gogrep-related materials, like https://github.com/mvdan/talks/blob/master/2018/gogrep.slide

It gives a pretty good introduction for people who are new to a concept of ast-based code matching.

That .slide can be compiled into PDF, uploaded to a speakerdeck and that speakerdeck link can be added to a README.

Does it make sense? :)

The text was updated successfully, but these errors were encountered:

mvdan · 2019-12-21T00:05:08Z

Hm. I worry that the material would go stale, and confuse people more than anything.

But maybe that won't be so bad if we add a date right next to the link.

We also don't need to mess with PDFs; https://talks.godoc.org/github.com/mvdan/talks/2018/gogrep.slide should suffice. I get that it stinks on mobile, but we can also link the original plaintext version.

sebastien-rosset · 2020-11-30T18:25:07Z

I am trying to understand how gogrep matches AST nodes when invoked as gogrep -x '$x{$*_}'. It's not immediately obvious how it works.

That command seems to match any statement initializing a composite literal (maps, struct, array, slice), which makes sense. For example, that command matches the following statements:

MyStruct{}    // ast.CompositeLit, ast.Ident
mypkg.MyStruct{}   // ast.CompositeLit, ast.SelectorExpr, ast.Ident, ast.Ident
v := MyStruct{}  // ast.AssignStmt, ast.Ident, ast.CompositeLit, ast.Ident
v := mypkg.MyStruct{}  // ast.AssignStmt, ast.Ident, ast.CompositeLit, ast.SelectorExpr, ast.Ident, ast.Ident
v := mypkg.MyStruct{f1: "abc"}
v := &mypkg.MyStruct{f1: "abc"}
myFunc(mypkg.MyStruct{f1: "abc"})
primes := [6]int{2, 3, 5, 7, 11, 13}
s := []struct {
    i int
    b bool
}{
    {2, true},
    {3, false},
}

What is less obvious is that gogrep -x '$x{$*_}' does not seem to match other code patterns, where an ast node is followed by a {. In particular, that does not match the following statements:

switch { }  // ast.SwitchStmt, ast.BlockStmt
if ok := true ; ok {  }
if true { }
for { }
func main() {}

In the above code, it would be reasonable to assume gogrep -x '$x{$*_}' will match switch {}, since switch is represented in AST as a ast.SwitchStmt and { is a ast.BlockStmt, but it does not. Just to be clear, I'm not trying to argue whether that should or should not match; I am merely suggesting to improve the documentation.

Another behavior that would be worth documenting is how exactly gogrep matches the code:

mypkg.MyStruct{}   // which is represented in AST as ast.CompositeLit, ast.SelectorExpr, ast.Ident, ast.Ident

In the following scenarios:

gogrep -x '$x{$*_}'
gogrep -x '$x.$y{$*_}'

The question is which AST nodes are $x and $y mapped to. For case (1), my observation is that it seems to consume the entire composite literal mypkg.MyStruct, but that wasn't obvious. I wasn't sure if this would map to mypkg.MyStruct or just MyStruct. In case (2), it's less ambiguous.

mvdan · 2020-12-11T12:54:03Z

The matching could be more aggressive; it's always been a fine balance between "matches too much" and "matches too little". This is also why I added an "aggressive" option to the pattern syntax, but I honestly don't like it all that much either.

sebastien-rosset · 2020-12-11T17:12:00Z

I'm just starting to read the code. Is aggressive supposed to be an option like like regex greedy vs reluctant vs possessive?

BTW, I found it is very helpful to add this line after https://github.com/mvdan/gogrep/blob/master/main.go#L162:

ast.Fprint(m.out, m.fset, n, nil)

It helps to understand the details of the matching results. You can quickly look at how any matching piece of code is modeled in AST. Maybe add a --debug flag or similar?

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a gogrep talk link into a readme? #44

Add a gogrep talk link into a readme? #44

Add a gogrep talk link into a readme? #44

Add a gogrep talk link into a readme? #44

Comments