TheThingsIndustries / protoc-gen-fieldmask

Generate field mask utilities from proto files
Apache License 2.0
11 stars 3 forks source link

Group nested field paths #7

Closed rvolosatovs closed 5 years ago

rvolosatovs commented 5 years ago

Summary:

Field paths, which modify the same nested path should be grouped.

Consider https://github.com/TheThingsIndustries/protoc-gen-fieldmask/blob/7fc131b1b10f52a2aaebd8a9dfccb0f051da87fa/testdata/testdata.pb.fm.go#L54-L98

Instead of having a case per path, we could have:

case "a.a", "a.a.a", /* other cases... */ "a.e":
   if dst.A == nil {
        dst.A = &Test_TestNested{}
    }
   dst.A.SetFields(src.A, _pathsWithoutPrefix("a", paths)...)

or

// `switch {` somewhere above in a loop over paths...
case strings.HasPrefix(path, "a."):
   if dst.A == nil {
        dst.A = &Test_TestNested{}
    }
   dst.A.SetFields(src.A, _pathsWithoutPrefix("a", paths)...)

What is already there? What do you see now?

Case per path

What is missing? What do you want to see?

Grouped paths in a case

How do you propose implementing this?

After generating the path set, handle top-level paths as just set(with =), and group nested paths by the top-level field and generate a case with SetFields per group(using either a for loop and additional switch or simply matching the final fields).