Escape javadoc special characters #1342
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
iwahbe
added
the
impact/no-changelog-required
t0yv0
reviewed
Apr 30, 2024
| return formatForeignBlockCommentFrom(comment, 0, indent) | ||
| } | ||
|
|
||
| // formatForeignBlockCommentFrom is like [formatForeignBlockComment], except that it |
There was a problem hiding this comment.
Style nit for helper functions fHelper that are only called from f and are not supposed to be used anywhere else I personally forgo the doc comments
There was a problem hiding this comment.
It's not a helper function. It's called directly from the rest of the module. It's used for generating comments like:
fprintf(w, formatForeignBlockCommentFrom("@return "+invoke.Comment, 1))That way any @ in the invoke.Comment string is escaped but the first @ of the @return is left alone.
t0yv0
reviewed
Apr 30, 2024
| // Then write the code block itself | ||
| dst.WriteString(code(comment[codeStart:codeEndOffset])) | ||
| // Then adjust copiedTo to start after the code block | ||
| comment = comment[codeEndOffset:] |
t0yv0
added a commit
that referenced
this pull request
May 1, 2024
Reverts #1342 Testing this out on pulumi/pulumi-azure#1992 it appears that the latest version still cannot properly escape the generated code, moreover it seems to have regressed on not escaping `->` properly which might further break things. I think we might be better off reverting for now until we work this out to pass more test cases. Apologies for merging that without diving deeper - it looked solid at code review time, I guess we don't have downstream checks here yet to demonstrate that derivative providers cannot build with the changes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes #1271
Correctly scope comment escapes to outside of code blocks. The resulting code is both more idiomatic javadoc and safe to embed.
I have two motivating examples:
Correctness: Upstream comments that start lines with
@patternwill now be escaped to{@literal @}pattern. This has prevented us from publishing in the past (A release workflow silently fails for Java sdk pulumi-gcp#1950, Java SDK does not pass javadoc pulumi-auth0#516, Java SDK does not pass javadoc pulumi-azure#1979)Idiomaticity: Generated code blocks will no longer be HTML escaped (since they don't need to be). Instead, they will render correctly.
Checklist