Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix absolute doc_opt paths on Windows (#497) #506

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 42 additions & 7 deletions plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,50 @@ func ParseOptions(req *plugin_go.CodeGeneratorRequest) (*PluginOptions, error) {
if strings.Contains(params, ":") {
// Parse out exclude patterns if any
parts := strings.Split(params, ":")
for _, pattern := range strings.Split(parts[1], ",") {
r, err := regexp.Compile(pattern)
if err != nil {
return nil, err

// On Windows, there can legitimately be up to two ":" in the first part: one for each filename in case absolute paths
// are used, as a divider between the drive letter and the remainder of the path. That makes it really ugly: we now
// need to do some heuristics to "match" Windows path patterns to detect if a : is not to be treated as a divider
// between the first parameter half and the second half with the exclude patterns.
// This fixes GitHub issue #497.
winPathHeuristic := func(first, second string) bool {
// A Windows path in our doc_opt parameter is assumed to have a backslash in the second part...
if strings.HasPrefix(second, "\\") {
// ...and a drive letter either directly at the start of the first part or being preceded by a comma.
// If both of these conditions match, it is assumed that the : separation actually splitted a Windows
// path in two parts which belong together.
firstMatches, _ := regexp.MatchString("(?:^|,)[a-zA-z]$", first)
return firstMatches
}
return false
}
excludePart := ""
if winPathHeuristic(parts[0], parts[1]) {
params = parts[0] + ":" + parts[1]
if len(parts) > 2 {
if winPathHeuristic(params, parts[2]) {
params = params + ":" + parts[2]
if len(parts) > 3 {
excludePart = parts[3]
}
} else {
excludePart = parts[2]
}
}
} else {
params = parts[0]
excludePart = parts[1]
}

if len(excludePart) > 0 {
for _, pattern := range strings.Split(excludePart, ",") {
r, err := regexp.Compile(pattern)
if err != nil {
return nil, err
}
options.ExcludePatterns = append(options.ExcludePatterns, r)
}
options.ExcludePatterns = append(options.ExcludePatterns, r)
}
// The first part is parsed below
params = parts[0]
}
if params == "" {
return options, nil
Expand Down
25 changes: 25 additions & 0 deletions plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,31 @@ func TestParseOptionsWithInvalidValues(t *testing.T) {
}
}

func TestParseOptionsForCustomTemplateWindowsAbsolutePath(t *testing.T) {
req := new(plugin_go.CodeGeneratorRequest)
req.Parameter = proto.String("C:\\path\\to\\template.tmpl,C:\\path\\to\\output")

options, err := ParseOptions(req)
require.NoError(t, err)

require.Equal(t, RenderTypeHTML, options.Type)
require.Equal(t, "C:\\path\\to\\template.tmpl", options.TemplateFile)
require.Equal(t, "C:\\path\\to\\output", options.OutputFile)
}

func TestParseOptionsForCustomTemplateWindowsAbsolutePathPlusExcludes(t *testing.T) {
req := new(plugin_go.CodeGeneratorRequest)
req.Parameter = proto.String("C:\\path\\to\\template.tmpl,C:\\path\\to\\output:test.*")

options, err := ParseOptions(req)
require.NoError(t, err)

require.Equal(t, RenderTypeHTML, options.Type)
require.Equal(t, "C:\\path\\to\\template.tmpl", options.TemplateFile)
require.Equal(t, "C:\\path\\to\\output", options.OutputFile)
require.Equal(t, "test.*", options.ExcludePatterns[0].String())
}

func TestRunPluginForBuiltinTemplate(t *testing.T) {
set, _ := utils.LoadDescriptorSet("fixtures", "fileset.pb")
req := utils.CreateGenRequest(set, "Booking.proto", "Vehicle.proto", "nested/Book.proto")
Expand Down