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

Pegasus documentation cannot produce valid javadoc with certain symbols #432

Open
jplaisted opened this issue Oct 1, 2020 · 7 comments
Open
Assignees
Labels

Comments

@jplaisted
Copy link

It seems like Pegasus will do some parsing of documentation tags and spit out the "parsed" strings into the javadoc. This actually makes it invalid.

Example:

  // some enum PDL
  /**
   * Represent the relation greater than, e.g. ownerCount > 5
   */
  GREATER_THAN
    // generated java
    /**
     * Represent the relation greater than, e.g. ownerCount > 5
     * 
     */
    GREATER_THAN,
@BrianPin
Copy link

BrianPin commented Oct 7, 2020

@jplaisted Hi, Would you mind give me some context? from the examples, are you saying the "> 5" is invalid for you?

@jplaisted
Copy link
Author

In the above I gave the PDL (which used >) and the javadoc it generated (which translated it to >).

> is an invalid character in javadoc.

@jplaisted
Copy link
Author

If you're unaware, Javadoc supports HTML tags. So characters like < and > and invalid on their own and need to use HTML escaping for some things.

I suggest you don't "translate" any documentation here when generating javadoc. Just print it literally. Then PDL annotations can use javadoc syntax directly (so &gt; in PDL becomes &gt; in javadoc, which is valid). The other option is to transform PDL doc to javadoc; so if there is a > in the pegasus doc translate it to &gt;).

@BrianPin
Copy link

BrianPin commented Oct 8, 2020

got it, I will investigate and get back here

@BrianPin
Copy link

BrianPin commented Oct 8, 2020

Well, looks like that is the intended design, in restli:

  @Test
  public void testUnescapeDocstring()
  {
    String extracted = PdlParseUtils.extractMarkdown(
        "  /**\n" +
        "   * &lt;div&gt;Some html&lt;/div&gt;\n" +
        "   * &#47;&#42; A comment &#42;&#47;\n" +
        "   */\n");
    assertEquals(extracted,
        "<div>Some html</div>\n" +
        "/* A comment */");
  }

The specific unescape is happening inside

StringEscapeUtils.unescapeHtml4(commentUnescaped);

I think if we change this it will affect a lot.

I would like to discuss with our staffs to gain better decision making.

@evanw555
Copy link
Contributor

evanw555 commented Nov 4, 2020

We agreed that the current behavior isn't ideal, but there are other concerns about how this would affect current use cases. We're tracking this internally and we'll revisit this issue later once we have bandwidth.

@BrianPin
Copy link

BrianPin commented Nov 4, 2020

I created a ticket under my name! FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants