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

Some special list items (e.g. SeeAlso) get stripped #282

Closed
pcantrell opened this issue Aug 13, 2015 · 7 comments
Closed

Some special list items (e.g. SeeAlso) get stripped #282

pcantrell opened this issue Aug 13, 2015 · 7 comments

Comments

@pcantrell
Copy link
Collaborator

The following gets stripped from a doc comment:

  - SeeAlso: foo and bar

It’s due to 34796f3, which seems to be intentionally stripping these items…? Some of them (e.g. Parameter) apparently get handled elsewhere, but most don’t. They just disappear.

@jpsim
Copy link
Collaborator

jpsim commented Aug 22, 2015

I believe SourceKit properly parses these (just like parameter and returns) but we're not rendering them appropriately in the generated HTML. So I think this issue is entirely jazzy-related (i.e. not SourceKitten or SourceKit).

Thanks for reporting.

@pcantrell
Copy link
Collaborator Author

Per your comment, I took a look at the SourceKitten output for Parameter (which works) and SeeAlso (which doesn't).

Here’s a parameter:

/**
  Returns the value of the HTTP header with the given key.

  Entity does not support multi-valued headers (i.e. headers which occur more than once in the response).

  - Parameter key: The case-insensitive header name.
*/
public func header(key: String) -> String?

…and the Sourcekitten output:

{
  "key.kind" : "source.lang.swift.decl.function.method.instance",
  "key.offset" : 4172,
  "key.parsed_declaration" : "public func header(key: String) -> String?\n        { return headers[key.lowercaseString] }",
  "key.doc.comment" : "  Returns the value of the HTTP header with the given key.\n\nEntity does not support multi-valued headers (i.e. headers which occur more than once in the response).\n\n- Parameter key: The case-insensitive header name.",
  "key.namelength" : 19,
  "key.doc.line" : 117,
  "key.bodylength" : 37,
  "key.length" : 19,
  "key.doc.parameters" : [
    {
      "name" : "key",
      "discussion" : [
        {
          "Para" : "The case-insensitive header name.",
          "kind" : ""
        }
      ]
    }
  ],
  ...
  "key.attributes" : [
    {
      "key.attribute" : "source.decl.attribute.__raw_doc_comment"
    }
  ],
  "key.doc.full_as_xml" : "<Function file=\"...\/siesta\/Source\/Entity.swift\" line=\"117\" column=\"17\"><Name>header(_:)<\/Name><USR>s:FV6Siesta6Entity6headerFS0_FSSGSqSS_<\/USR><Declaration>public func header(key: String) -&gt; String?<\/Declaration><Abstract><Para>Returns the value of the HTTP header with the given key.<\/Para><\/Abstract><Parameters><Parameter><Name>key<\/Name><Direction isExplicit=\"0\">in<\/Direction><Discussion><Para>The case-insensitive header name.<\/Para><\/Discussion><\/Parameter><\/Parameters><Discussion><Para>Entity does not support multi-valued headers (i.e. headers which occur more than once in the response).<\/Para><\/Discussion><\/Function>",
  "key.doc.name" : "header(_:)",
  "key.substructure" : [

  ],
  "key.doc.usr" : "s:FV6Siesta6Entity6headerFS0_FSSGSqSS_",
  "key.doc.discussion" : [
    {
      "Para" : "Entity does not support multi-valued headers (i.e. headers which occur more than once in the response).",
      "kind" : ""
    }
  ]
},

The parameter doc appears in three different places:

  1. key.doc.comment
  2. under key.doc.parameters
  3. inside key.doc.full_as_xml

Compare to a SeeAlso:

/**
  Options which control the behavior of a `Resource`.

  - SeeAlso: `Service.configure(...)`
*/
public struct Configuration

…and its Sourcekitten output:

"...\/siesta\/Source\/Configuration.swift" : {
  "key.substructure" : [
    {
      "key.kind" : "source.lang.swift.decl.struct",
      "key.offset" : 257,
      "key.parsed_declaration" : "public struct Configuration",
      "key.doc.comment" : "Options which control the behavior of a `Resource`.\n\n- SeeAlso: `Service.configure(...)`",
      "key.namelength" : 13,
      ...
      "key.attributes" : [
        {
          "key.attribute" : "source.decl.attribute.__raw_doc_comment"
        }
      ],
      "key.doc.full_as_xml" : "<Class file=\"...\/siesta\/Source\/Configuration.swift\" line=\"14\" column=\"15\"><Name>Configuration<\/Name><USR>s:V6Siesta13Configuration<\/USR><Declaration>public struct Configuration<\/Declaration><Abstract><Para>Options which control the behavior of a <codeVoice>Resource<\/codeVoice>.<\/Para><\/Abstract><Discussion><See><Para><codeVoice>Service.configure(...)<\/codeVoice><\/Para><\/See><\/Discussion><\/Class>",
      "key.doc.name" : "Configuration",
      "key.substructure" : [
        ...
      ],
      "key.doc.usr" : "s:V6Siesta13Configuration",
      "key.doc.discussion" : [
        {
          "See" : "",
          "kind" : ""
        }
      ]
    }
  ],
  "key.offset" : 0,
  "key.diagnostic_stage" : "source.diagnostic.stage.swift.parse",
  "key.length" : 2758
}

By contrast, the SeeAlso content appears in only two places:

  1. key.doc.comment
  2. inside key.doc.full_as_xml

There is no equivalent to key.doc.parameters for SeeAlso. So maybe it’s a SourceKit issue?

Looking at this output, I can't help but wonder whether it wouldn’t be better to use key.doc.full_as_xml for everything. You could not only stop the special tag filtering in question with this bug, but also ditch the Markdown parsing altogether. Have you considered that already?

@jpsim
Copy link
Collaborator

jpsim commented Aug 23, 2015

key.doc.parameters is created by SourceKitten by parsing key.doc.full_as_xml, but this is special-cased. The same could be done with every "special" list item by applying the same approach.

Looking at this output, I can't help but wonder whether it wouldn’t be better to use key.doc.full_as_xml for everything. You could not only stop the special tag filtering in question with this bug, but also ditch the Markdown parsing altogether. Have you considered that already?

That certainly would make jazzy a lot simpler, but this was avoided in the past because SourceKit's markdown rendering was pretty horrible. But I have a feeling it's gotten much better in recent Xcode 7 betas.

@pcantrell
Copy link
Collaborator Author

I can't help but wonder whether it wouldn’t be better to use key.doc.full_as_xml for everything.

That certainly would make jazzy a lot simpler, but this was avoided in the past because SourceKit's markdown rendering was pretty horrible. But I have a feeling it's gotten much better in recent Xcode 7 betas.

On casual inspection, it looks pretty good in XC7b5.

Would you be interested in seeing a PR for a rough draft of this?

@jpsim
Copy link
Collaborator

jpsim commented Aug 23, 2015

Would you be interested in seeing a PR for a rough draft of this?

Absolutely!

@pcantrell
Copy link
Collaborator Author

FYI, haven't forgotten about sending a PR draft of a key.doc.full_as_xml approach. Just waiting for the swift-2.0 merge to be completed and my other PRs to get done, so I don't go into a local merge hell.

@jpsim
Copy link
Collaborator

jpsim commented Nov 24, 2015

Done in #323.

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

No branches or pull requests

3 participants