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

Add command-line option to set the minimum access control to document #69

Closed
jpsim opened this issue Nov 1, 2014 · 9 comments
Closed

Comments

@jpsim
Copy link
Collaborator

jpsim commented Nov 1, 2014

--minimum-access-control [public | internal | private] with public as default

@beltex
Copy link
Contributor

beltex commented Dec 14, 2014

So currently it is expected behaviour that private entities (enums, structs, etc.) will show up in the doc by default correct?

@segiddins
Copy link
Collaborator

@beltex yes, that is correct.

@beltex
Copy link
Contributor

beltex commented Dec 14, 2014

Cool, thanks @segiddins!

@segiddins
Copy link
Collaborator

@jpsim the easiest way to do this would be if SourceKitten could just add an ACL to its output for each token, but I don't know if SourceKit exposes that information.

@jpsim jpsim self-assigned this Dec 14, 2014
@jpsim
Copy link
Collaborator Author

jpsim commented Dec 14, 2014

@segiddins sourcekitten already exposes ACL information, i.e.:

"key.attributes" : [
  {
    "key.attribute" : "source.decl.attribute.private"
  },
  {
    "key.attribute" : "source.decl.attribute.__raw_doc_comment"
  }
]

However, simply ignoring tokens whose access control levels are under a given threshold would make the documentation coverage number invalid, since it considers all tokens. There are two possible approaches, I think:

  1. make sourcekitten return all detected tokens (documented or not) and filter them from jazzy
  2. pass minimum ACL level to sourcekitten

I'm not sure which one is best, so I'll just pick the easiest one and see if it works 😉.

@segiddins
Copy link
Collaborator

I say approach number 1

-Samuel E. Giddins

On Dec 14, 2014, at 4:35 PM, JP Simard notifications@github.com wrote:

@segiddins sourcekitten already exposes ACL information, i.e.:

"key.attributes" : [
{
"key.attribute" : "source.decl.attribute.private"
},
{
"key.attribute" : "source.decl.attribute.__raw_doc_comment"
}
]
However, simply ignoring tokens whose access control levels are under a given threshold would make the documentation coverage number invalid, since it considers all tokens. There are two possible approaches, I think:

make sourcekitten return all detected tokens (documented or not) and filter them from jazzy
pass minimum ACL level to sourcekitten
I'm not sure which one is best, so I'll just pick the easiest one and see if it works .


Reply to this email directly or view it on GitHub.

@segiddins
Copy link
Collaborator

We only seem to get when its private, and not internal or public.

-Samuel E. Giddins

On Dec 14, 2014, at 4:35 PM, JP Simard notifications@github.com wrote:

@segiddins sourcekitten already exposes ACL information, i.e.:

"key.attributes" : [
{
"key.attribute" : "source.decl.attribute.private"
},
{
"key.attribute" : "source.decl.attribute.__raw_doc_comment"
}
]
However, simply ignoring tokens whose access control levels are under a given threshold would make the documentation coverage number invalid, since it considers all tokens. There are two possible approaches, I think:

make sourcekitten return all detected tokens (documented or not) and filter them from jazzy
pass minimum ACL level to sourcekitten
I'm not sure which one is best, so I'll just pick the easiest one and see if it works .


Reply to this email directly or view it on GitHub.

@jpsim
Copy link
Collaborator Author

jpsim commented Dec 15, 2014

Actually, it's much worse than that. public and private both have attributes of source.decl.attribute.private (WTF, Apple) while internal has no ACL-related attribute.

I've spoken to SourceKit people about it and there's not much motivation to fix that, so we should use an alternative mode of ACL detection.

@jpsim jpsim closed this as completed in 5efe7d5 Dec 17, 2014
jpsim added a commit that referenced this issue Dec 17, 2014
Add config option for minimum ACL to document (closes #69)
@beltex
Copy link
Contributor

beltex commented Dec 19, 2014

Works like a charm, thanks @jpsim & @segiddins! :)

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

4 participants