jhump / protoreflect

Reflection (Rich Descriptors) for Go Protocol Buffers
Apache License 2.0
1.35k stars 172 forks source link

String escaping fix #580

Closed japersik closed 1 year ago

japersik commented 1 year ago

https://github.com/jhump/protoreflect/issues/579#issue-1970215724

jhump commented 1 year ago

Oof, great catch! What an embarrassing bug.

What's missing from this pull request is updates to tests to catch this sort of thing in the future. The best way to is to edit test files in internal/testprotos and then to re-generate the associated "golden" output files to make sure they are now correct.

I went ahead and did this, because I noticed that you checked the box so that maintainers are allowed to edit this pull request. However, it looks like you opened this PR from the main branch of your fork, and I am unable to push any commits to that branch.

I think you'll need to apply this diff to your fork to improve test coverage for all escapes, and then I can merge this pull request once you've pushed those: https://github.com/japersik/protoreflect/compare/main...jhump:japersik-pr

japersik commented 1 year ago

I think you'll need to apply this diff to your fork to improve test coverage for all escapes

Done