From 341d9ac4f9d8d96720c317e2d61f55be5e92b743 Mon Sep 17 00:00:00 2001 From: William Chang Date: Fri, 5 Aug 2022 14:47:13 -0400 Subject: [PATCH 1/5] Testing adding a new parameter which allows output files to map 1:1 to input files instead of grouping input files into one output file. --- plugin.go | 53 ++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/plugin.go b/plugin.go index bc769016..cd58fc3d 100644 --- a/plugin.go +++ b/plugin.go @@ -21,6 +21,7 @@ type PluginOptions struct { OutputFile string ExcludePatterns []*regexp.Regexp SourceRelative bool + SeparateFiles bool } // SupportedFeatures describes a flag setting for supported features. @@ -51,19 +52,40 @@ func (p *Plugin) Generate(r *plugin_go.CodeGeneratorRequest) (*plugin_go.CodeGen } resp := new(plugin_go.CodeGeneratorResponse) - fdsGroup := groupProtosByDirectory(result, options.SourceRelative) - for dir, fds := range fdsGroup { - template := NewTemplate(fds) - output, err := RenderTemplate(options.Type, template, customTemplate) - if err != nil { - return nil, err + if !options.SeparateFiles { + fdsGroup := groupProtosByDirectory(result, options.SourceRelative) + for dir, fds := range fdsGroup { + template := NewTemplate(fds) + + output, err := RenderTemplate(options.Type, template, customTemplate) + if err != nil { + return nil, err + } + + resp.File = append(resp.File, &plugin_go.CodeGeneratorResponse_File{ + Name: proto.String(filepath.Join(dir, options.OutputFile)), + Content: proto.String(string(output)), + }) } + } else { + for _, fd := range result { + template := NewTemplate([]*protokit.FileDescriptor{fd}) + + output, err := RenderTemplate(options.Type, template, customTemplate) + if err != nil { + return nil, err + } + + dir, protoFilename := filepath.Split(fd.GetName()) - resp.File = append(resp.File, &plugin_go.CodeGeneratorResponse_File{ - Name: proto.String(filepath.Join(dir, options.OutputFile)), - Content: proto.String(string(output)), - }) + filenameParts := strings.Split(protoFilename, ".") + + resp.File = append(resp.File, &plugin_go.CodeGeneratorResponse_File{ + Name: proto.String(filepath.Join(dir, filenameParts[0]+"."+options.OutputFile)), + Content: proto.String(string(output)), + }) + } } resp.SupportedFeatures = proto.Uint64(SupportedFeatures) @@ -115,6 +137,7 @@ func ParseOptions(req *plugin_go.CodeGeneratorRequest) (*PluginOptions, error) { TemplateFile: "", OutputFile: "index.html", SourceRelative: false, + SeparateFiles: false, } params := req.GetParameter() @@ -140,7 +163,7 @@ func ParseOptions(req *plugin_go.CodeGeneratorRequest) (*PluginOptions, error) { } parts := strings.Split(params, ",") - if len(parts) < 2 || len(parts) > 3 { + if len(parts) < 2 { return nil, fmt.Errorf("Invalid parameter: %s", params) } @@ -156,6 +179,14 @@ func ParseOptions(req *plugin_go.CodeGeneratorRequest) (*PluginOptions, error) { return nil, fmt.Errorf("Invalid parameter: %s", params) } } + if len(parts) > 3 { + switch parts[3] { + case "separate_files": + options.SeparateFiles = true + default: + return nil, fmt.Errorf("Invalid parameter: %s", params) + } + } options.SourceRelative = len(parts) > 2 && parts[2] == "source_relative" renderType, err := NewRenderType(options.TemplateFile) From 441271fc305ed16f0525873a3ba3262ccc0929f3 Mon Sep 17 00:00:00 2001 From: William Chang Date: Thu, 18 Aug 2022 16:01:52 -0400 Subject: [PATCH 2/5] Cleaned up checking for additional params as well as processing of files based on new flag. --- plugin.go | 75 +++++++++++++++++++++++++++++++------------------------ 1 file changed, 42 insertions(+), 33 deletions(-) diff --git a/plugin.go b/plugin.go index cd58fc3d..d8d23049 100644 --- a/plugin.go +++ b/plugin.go @@ -53,9 +53,9 @@ func (p *Plugin) Generate(r *plugin_go.CodeGeneratorRequest) (*plugin_go.CodeGen resp := new(plugin_go.CodeGeneratorResponse) - if !options.SeparateFiles { - fdsGroup := groupProtosByDirectory(result, options.SourceRelative) - for dir, fds := range fdsGroup { + fdsGroup := groupProtosByDirectory(result, options.SourceRelative) + for dir, fds := range fdsGroup { + if !options.SeparateFiles { template := NewTemplate(fds) output, err := RenderTemplate(options.Type, template, customTemplate) @@ -67,24 +67,24 @@ func (p *Plugin) Generate(r *plugin_go.CodeGeneratorRequest) (*plugin_go.CodeGen Name: proto.String(filepath.Join(dir, options.OutputFile)), Content: proto.String(string(output)), }) - } - } else { - for _, fd := range result { - template := NewTemplate([]*protokit.FileDescriptor{fd}) + } else { + for _, fd := range fds { + template := NewTemplate([]*protokit.FileDescriptor{fd}) - output, err := RenderTemplate(options.Type, template, customTemplate) - if err != nil { - return nil, err - } + output, err := RenderTemplate(options.Type, template, customTemplate) + if err != nil { + return nil, err + } - dir, protoFilename := filepath.Split(fd.GetName()) + _, protoFilename := filepath.Split(fd.GetName()) - filenameParts := strings.Split(protoFilename, ".") + filenameParts := strings.Split(protoFilename, ".") - resp.File = append(resp.File, &plugin_go.CodeGeneratorResponse_File{ - Name: proto.String(filepath.Join(dir, filenameParts[0]+"."+options.OutputFile)), - Content: proto.String(string(output)), - }) + resp.File = append(resp.File, &plugin_go.CodeGeneratorResponse_File{ + Name: proto.String(filepath.Join(dir, filenameParts[0]+"."+options.OutputFile)), + Content: proto.String(string(output)), + }) + } } } @@ -169,25 +169,34 @@ func ParseOptions(req *plugin_go.CodeGeneratorRequest) (*PluginOptions, error) { options.TemplateFile = parts[0] options.OutputFile = path.Base(parts[1]) + if len(parts) > 2 { - switch parts[2] { - case "source_relative": - options.SourceRelative = true - case "default": - options.SourceRelative = false - default: - return nil, fmt.Errorf("Invalid parameter: %s", params) - } - } - if len(parts) > 3 { - switch parts[3] { - case "separate_files": - options.SeparateFiles = true - default: - return nil, fmt.Errorf("Invalid parameter: %s", params) + extraOptions := parts[2:] + + for i := 0; i < len(extraOptions); i++ { + switch i { + case 0: // Third option + switch extraOptions[i] { + case "source_relative": + options.SourceRelative = true + case "default": + options.SourceRelative = false + default: + return nil, fmt.Errorf("Invalid parameter: %s", params) + } + case 1: // Fourth option + switch extraOptions[i] { + case "separate_files": + options.SeparateFiles = true + case "default": + options.SeparateFiles = false + default: + return nil, fmt.Errorf("Invalid parameter: %s", params) + } + } + } } - options.SourceRelative = len(parts) > 2 && parts[2] == "source_relative" renderType, err := NewRenderType(options.TemplateFile) if err == nil { From 77121ba2358e6f1c1716ba417adebfa78a8228cc Mon Sep 17 00:00:00 2001 From: William Chang Date: Thu, 18 Aug 2022 16:18:21 -0400 Subject: [PATCH 3/5] Updated documentation and readme. --- README.md | 14 +++++++++++++- cmd/protoc-gen-doc/flags.go | 3 +++ plugin.go | 2 +- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 2bdc8587..68c386c2 100644 --- a/README.md +++ b/README.md @@ -29,13 +29,25 @@ example](examples/gradle). The plugin is invoked by passing the `--doc_out`, and `--doc_opt` options to the `protoc` compiler. The option has the following format: - --doc_opt=|,[,default|source_relative] + --doc_opt=|,[,default|source_relative][,default|separate_files] The format may be one of the built-in ones ( `docbook`, `html`, `markdown` or `json`) or the name of a file containing a custom [Go template][gotemplate]. +### `source_relative` + If the `source_relative` flag is specified, the output file is written in the same relative directory as the input file. +### `separate_files` + +If the `separate_files` flag is specified, there will be one file outputted per input file and the second parameter, +``, will be used as the extension of the outputted files. + +For example, the following will result in the outputted files `foo.md` and `bar.md` since `md` is passed as the second parameter. +``` +--doc_opt=markdown,md,source_relative,separate_files foo.proto bar.proto +``` + ### Using the Docker Image (Recommended) The docker image has two volumes: `/out` and `/protos` which are the directory to write the documentation to and the diff --git a/cmd/protoc-gen-doc/flags.go b/cmd/protoc-gen-doc/flags.go index 40aa8976..9ae7cc75 100644 --- a/cmd/protoc-gen-doc/flags.go +++ b/cmd/protoc-gen-doc/flags.go @@ -24,6 +24,9 @@ protoc --doc_out=. --doc_opt=custom.tmpl,docs.txt protos/*.proto EXAMPLE: Generate docs relative to source protos protoc --doc_out=. --doc_opt=html,index.html,source_relative protos/*.proto +EXAMPLE: Generate docs relative to source protos, one output file per input file +protoc --doc_out=. --doc_opt=html,html,source_relative,separate_files protos/*.proto + See https://github.com/pseudomuto/protoc-gen-doc for more details. ` diff --git a/plugin.go b/plugin.go index d8d23049..45b047ac 100644 --- a/plugin.go +++ b/plugin.go @@ -129,7 +129,7 @@ OUTER: // ParseOptions parses plugin options from a CodeGeneratorRequest. It does this by splitting the `Parameter` field from // the request object and parsing out the type of renderer to use and the name of the file to be generated. // -// The parameter (`--doc_opt`) must be of the format ,[,default|source_relative]:,*. +// The parameter (`--doc_opt`) must be of the format ,[,default|source_relative][,default|separate_files]:,*. // The file will be written to the directory specified with the `--doc_out` argument to protoc. func ParseOptions(req *plugin_go.CodeGeneratorRequest) (*PluginOptions, error) { options := &PluginOptions{ From bb0c8eebf2bf114264a9d6deb23aac44a8da9342 Mon Sep 17 00:00:00 2001 From: William Chang Date: Thu, 18 Aug 2022 16:27:55 -0400 Subject: [PATCH 4/5] Added tests for parsing separate_files param and also expected files outputted when running plugin. --- plugin_test.go | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/plugin_test.go b/plugin_test.go index ebb18b7d..2d877dcf 100644 --- a/plugin_test.go +++ b/plugin_test.go @@ -53,6 +53,24 @@ func TestParseOptionsForSourceRelative(t *testing.T) { require.Equal(t, options.SourceRelative, false) } +func TestParseOptionsForSeparateFiles(t *testing.T) { + req := new(plugin_go.CodeGeneratorRequest) + req.Parameter = proto.String("markdown,index.md,source_relative,separate_files") + options, err := ParseOptions(req) + require.NoError(t, err) + require.Equal(t, options.SeparateFiles, true) + + req.Parameter = proto.String("markdown,index.md,source_relative,default") + options, err = ParseOptions(req) + require.NoError(t, err) + require.Equal(t, options.SeparateFiles, false) + + req.Parameter = proto.String("markdown,index.md,source_relative") + options, err = ParseOptions(req) + require.NoError(t, err) + require.Equal(t, options.SeparateFiles, false) +} + func TestParseOptionsForCustomTemplate(t *testing.T) { req := new(plugin_go.CodeGeneratorRequest) req.Parameter = proto.String("/path/to/template.tmpl,/base/name/only/output.md") @@ -150,3 +168,47 @@ func TestRunPluginForSourceRelative(t *testing.T) { require.NotEmpty(t, resp.File[0].GetContent()) require.NotEmpty(t, resp.File[1].GetContent()) } + +func TestRunPluginForSeparateFilesSourceRelative(t *testing.T) { + set, _ := utils.LoadDescriptorSet("fixtures", "fileset.pb") + req := utils.CreateGenRequest(set, "Booking.proto", "Vehicle.proto", "nested/Book.proto") + req.Parameter = proto.String("markdown,md,source_relative,separate_files") + + plugin := new(Plugin) + resp, err := plugin.Generate(req) + require.NoError(t, err) + require.Len(t, resp.File, 3) + expected := map[string]int{"Booking.md": 1, "Vehicle.md": 1, "nested/Book.md": 1} + require.Contains(t, expected, resp.File[0].GetName()) + delete(expected, resp.File[0].GetName()) + require.Contains(t, expected, resp.File[1].GetName()) + delete(expected, resp.File[1].GetName()) + require.Contains(t, expected, resp.File[2].GetName()) + delete(expected, resp.File[2].GetName()) + + require.NotEmpty(t, resp.File[0].GetContent()) + require.NotEmpty(t, resp.File[1].GetContent()) + require.NotEmpty(t, resp.File[2].GetContent()) +} + +func TestRunPluginForSeparateFilesNoSourceRelative(t *testing.T) { + set, _ := utils.LoadDescriptorSet("fixtures", "fileset.pb") + req := utils.CreateGenRequest(set, "Booking.proto", "Vehicle.proto", "nested/Book.proto") + req.Parameter = proto.String("markdown,md,default,separate_files") + + plugin := new(Plugin) + resp, err := plugin.Generate(req) + require.NoError(t, err) + require.Len(t, resp.File, 3) + expected := map[string]int{"Booking.md": 1, "Vehicle.md": 1, "Book.md": 1} + require.Contains(t, expected, resp.File[0].GetName()) + delete(expected, resp.File[0].GetName()) + require.Contains(t, expected, resp.File[1].GetName()) + delete(expected, resp.File[1].GetName()) + require.Contains(t, expected, resp.File[2].GetName()) + delete(expected, resp.File[2].GetName()) + + require.NotEmpty(t, resp.File[0].GetContent()) + require.NotEmpty(t, resp.File[1].GetContent()) + require.NotEmpty(t, resp.File[2].GetContent()) +} From 38c90d2417ca8feeeee6dc3dc2f947f421fbd80c Mon Sep 17 00:00:00 2001 From: William Chang Date: Fri, 19 Aug 2022 12:54:05 -0400 Subject: [PATCH 5/5] Restored condition to check for number of parameters above expected number (now 4). Also changed for loop to be more idiomatic. --- plugin.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugin.go b/plugin.go index 45b047ac..e8470f4a 100644 --- a/plugin.go +++ b/plugin.go @@ -163,17 +163,18 @@ func ParseOptions(req *plugin_go.CodeGeneratorRequest) (*PluginOptions, error) { } parts := strings.Split(params, ",") - if len(parts) < 2 { + if len(parts) < 2 || len(parts) > 4 { return nil, fmt.Errorf("Invalid parameter: %s", params) } options.TemplateFile = parts[0] options.OutputFile = path.Base(parts[1]) + // Handle extra options if len(parts) > 2 { extraOptions := parts[2:] - for i := 0; i < len(extraOptions); i++ { + for i := range extraOptions { switch i { case 0: // Third option switch extraOptions[i] { @@ -194,7 +195,6 @@ func ParseOptions(req *plugin_go.CodeGeneratorRequest) (*PluginOptions, error) { return nil, fmt.Errorf("Invalid parameter: %s", params) } } - } }