protoc-gen-validate icon indicating copy to clipboard operation
protoc-gen-validate copied to clipboard

Update protoc-gen-start dependency and add filepath.ToSlash() to fix #611

Open nvx opened this issue 2 years ago • 1 comments

Required more than just the dependency update, also had to add in filepath.ToSlash() to fix the module parameter issue.

nvx avatar Jul 12 '22 04:07 nvx

Looks like the build is failing due to some unrelated python protoc version issues? As far as I can tell my PR is fine. Hesitant to regenerate files that are unrelated to the PR and pollute the commits.

nvx avatar Jul 12 '22 04:07 nvx

Hi @nvx, thanks for your patience with this PR. I don't quite understand whats happening here...

ToSlash returns the result of replacing each separator character in path with a slash ('/') character. Multiple separators are replaced by multiple slashes.

we have a replace all func to create the path, and a ToSlash func to perform the same operation... please explain 👓

elliotmjackson avatar Sep 15 '22 19:09 elliotmjackson

Hi @nvx, thanks for your patience with this PR. I don't quite understand whats happening here...

ToSlash returns the result of replacing each separator character in path with a slash ('/') character. Multiple separators are replaced by multiple slashes.

we have a replace all func to create the path, and a ToSlash func to perform the same operation... please explain 👓

#611 describes the issue that this PR fixes. The filepath.ToSlash func is required to translate backslashes on Windows to forward slashes as the module name always uses forward slashes, as does the TrimLeft function call.

nvx avatar Sep 16 '22 00:09 nvx

It is worth noting, i have upgraded PGS so this issue is likely already solved in v0.6.8

elliotmjackson avatar Sep 16 '22 15:09 elliotmjackson

It is worth noting, i have upgraded PGS so this issue is likely already solved in v0.6.8

I noticed that, and updating PGS is also needed, but I found just updating PGS wasn't enough, the ToSlash change is also needed.

nvx avatar Sep 19 '22 02:09 nvx