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

Bugfix/issue 8101 #10268

Merged
merged 3 commits into from
Aug 2, 2022
Merged

Conversation

tonydnewell
Copy link
Contributor

@tonydnewell tonydnewell commented Jul 19, 2022

Fix for #8101

When converting identifiers for C# camel case, handle the case where the input starts with "_<digit>" and preserve the underscore.

It does not alter existing behaviour for other identifiers:

  • e.g. starts with "_<alpha>" - the underscore is consumed, so as to not break anything that relies on existing behaviour.
  • e.g. input is already invalid (e.g. identifier starts with a digit) - it does not try and fix it.

@jskeet jskeet requested a review from fowles August 2, 2022 14:02
@fowles fowles merged commit adb5d60 into protocolbuffers:main Aug 2, 2022
Copy link
Contributor

@jtattermusch jtattermusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as well.

@nickwinger
Copy link

@tonydnewell thank you for this fix, but on our side there is still a similar bug open with this, e.g.: namespace "my.space._3" gets compiled to c# "my.space.3" which again causes an error. So also with "sub" namespaces after dots the numbers with an underscore should be preserved. thank you

@tonydnewell
Copy link
Contributor Author

@nickwinger Thank you for your observation. I can see you've created a new issue #13482
I can't guarantee when it will be fixed but it is on the list.

@nickwinger
Copy link

here is the PR: #13504

bithium pushed a commit to bithium/protobuf that referenced this pull request Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants