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
Grpc.Tools: support generated filename corner cases #18470
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,29 +66,28 @@ internal class CSharpGeneratorServices : GeneratorServices | |
public override string[] GetPossibleOutputs(ITaskItem protoItem) | ||
{ | ||
bool doGrpc = GrpcOutputPossible(protoItem); | ||
string filename = LowerUnderscoreToUpperCamel( | ||
Path.GetFileNameWithoutExtension(protoItem.ItemSpec)); | ||
|
||
var outputs = new string[doGrpc ? 2 : 1]; | ||
string basename = Path.GetFileNameWithoutExtension(protoItem.ItemSpec); | ||
|
||
string outdir = protoItem.GetMetadata(Metadata.OutputDir); | ||
string fileStem = Path.Combine(outdir, filename); | ||
outputs[0] = fileStem + ".cs"; | ||
string filename = LowerUnderscoreToUpperCamelProtocWay(basename); | ||
outputs[0] = Path.Combine(outdir, filename) + ".cs"; | ||
|
||
if (doGrpc) | ||
{ | ||
// Override outdir if kGrpcOutputDir present, default to proto output. | ||
outdir = protoItem.GetMetadata(Metadata.GrpcOutputDir); | ||
if (outdir != "") | ||
{ | ||
fileStem = Path.Combine(outdir, filename); | ||
} | ||
outputs[1] = fileStem + "Grpc.cs"; | ||
string grpcdir = protoItem.GetMetadata(Metadata.GrpcOutputDir); | ||
filename = LowerUnderscoreToUpperCamelGrpcWay(basename); | ||
outputs[1] = Path.Combine( | ||
grpcdir != "" ? grpcdir : outdir, filename) + "Grpc.cs"; | ||
} | ||
return outputs; | ||
} | ||
|
||
string LowerUnderscoreToUpperCamel(string str) | ||
// This is how the gRPC codegen currently construct its output filename. | ||
// See src/compiler/generator_helpers.h:118. | ||
string LowerUnderscoreToUpperCamelGrpcWay(string str) | ||
{ | ||
// See src/compiler/generator_helpers.h:118 | ||
var result = new StringBuilder(str.Length, str.Length); | ||
bool cap = true; | ||
foreach (char c in str) | ||
|
@@ -109,6 +108,26 @@ string LowerUnderscoreToUpperCamel(string str) | |
} | ||
return result.ToString(); | ||
} | ||
|
||
// This is how the protoc codegen constructs its output filename. | ||
// See protobuf/compiler/csharp/csharp_helpers.cc:356. | ||
// Note that protoc explicitly discards non-ASCII letters. | ||
string LowerUnderscoreToUpperCamelProtocWay(string str) | ||
{ | ||
var result = new StringBuilder(str.Length, str.Length); | ||
bool cap = true; | ||
foreach (char c in str) | ||
{ | ||
char upperC = char.ToUpperInvariant(c); | ||
bool isAsciiLetter = 'A' <= upperC && upperC <= 'Z'; | ||
if (isAsciiLetter || ('0' <= c && c <= '9')) | ||
{ | ||
result.Append(cap ? upperC : c); | ||
} | ||
cap = !isAsciiLetter; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe there's still a subtle problem - after a digit, protoc will still capitalize the next letter: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Uhhhh, thanks! I'll take care of this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This passes, I added a test and verified a real build, too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done, aa40424. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, thanks for clarification! |
||
} | ||
return result.ToString(); | ||
} | ||
}; | ||
|
||
// C++ generator services. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for comparison, the logic is here:
https://github.com/protocolbuffers/protobuf/blob/3a3956e8a258784461270961c6577341356bce52/src/google/protobuf/compiler/csharp/csharp_helpers.cc#L137 with
cap_next_letter=true
andpreserve_period=false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, this is where I got it from. Looks like I gave a wrong line number, will fix in a moment.
I do not understand what is this last
#
check at the very end of the function, but it does not seem to be important?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.