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

doc.comment blank for many declarations, causing missing Jazzy docs #142

Closed
pcantrell opened this issue Jan 21, 2016 · 12 comments
Closed

doc.comment blank for many declarations, causing missing Jazzy docs #142

pcantrell opened this issue Jan 21, 2016 · 12 comments
Labels

Comments

@pcantrell
Copy link

Updating the Siesta docs, I noticed that a bunch of documentation had disappeared. I traced this to Sourcekitten omitting doc.comment from many of the declarations. I haven’t verified this, but at a glance, it seems like this might be happening on Swift classes that extend NSObject.

You can see the problem in the Service and Resource classes in Siesta. It shows in the integration specs as they’re currently checked in.

This bug was introduced in jazzy when Sourcekitten was updated to 0.7.3. I didn’t chase it down the rabbit hole any further than that.

@pcantrell
Copy link
Author

OK, I’ve narrowed it down to a pretty small case, and it’s formatting-dependent. Sourcekitten fails to generate doc.comment for this:

/**
I am a comment.
*/
@objc
public class KittenTest: NSObject { }

…but this works:

/**
I am a comment.
*/
@objc public class KittenTest: NSObject { }

…and so does this:

/// I am a comment.
@objc
public class KittenTest: NSObject { }

@jpsim
Copy link
Owner

jpsim commented Jan 21, 2016

Yeah, this is definitely a bug, it's due to the logic used to detect which declarations are documented, it only checks the line directly above (and the logic for /// might be a bit different than /** */).

I wish there was a more straightforward/correct way to get a documentation comment for a declaration using SourceKit. 😞

@jpsim jpsim added the bug label Jan 21, 2016
@pcantrell
Copy link
Author

FWIW, whatever broke it is a recent addition (0.7.3-ish).

@jpsim
Copy link
Owner

jpsim commented Jan 21, 2016

It's not quite as simple as reverting the change... I know what broke this and I added it because it also fixes other things. We need an approach that fixes all of these 😄

@norio-nomura
Copy link
Collaborator

It seems we need to fix here.

@jpsim
Copy link
Owner

jpsim commented Jan 22, 2016

No, the approach I'm taking there is entirely wrong. I'd rather use the actual Swift parser to detect the doc comment, because it may be many lines above.

@norio-nomura
Copy link
Collaborator

I understand.

@pcantrell
Copy link
Author

The “full xml” property in Sourcekitten’s output does work in all cases.

(In some mythical future day when I have more than an hour of free time, I still mean to do a massive, terrifying PR which switches jazzy to using that and rips out its markdown parsing entirely. I say this now just to alarm JP.)

@jpsim
Copy link
Owner

jpsim commented Jan 22, 2016

Consider JP alarmed 😉

@norio-nomura
Copy link
Collaborator

It seems the document in dictionary["key.doc.full_as_xml"] does not contains line endings. Will using that be breaking change?

@pcantrell
Copy link
Author

The idea is that key.doc.full_as_xml already has markdown parsing applied — but in the exact manner that Xcode does it, not Jazzy’s approximation of that — and thus line endings no longer matter. There are a bunch of jazzy issues related to parsing differences, and in theory this would fix them. In practice it may be a terrible idea, but I keep wanting to try it & see what happens to the integration specs.

@norio-nomura
Copy link
Collaborator

Using existence of document in dictionary["key.doc.full_as_xml"] will fix in sourcekitten doc instead of currently using:

guard commentEndLine == tokenStartLine || commentEndLine == tokenStartLine?.predecessor() else {

in getDocumentationCommentBody(_:).
But getDocumentationCommentBody(_:) is also used in SwiftLint and dictionary["key.doc.full_as_xml"] does not exist on that case.
Should we fix partially only for sourcekitten doc?
I have no better idea for fixing both of cases.

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