Introduce documentedVisibilities setting

[IgnatBeresnev]

No description provided.

[vmishenev]

• Why public? In this case I expect I can make public entities undocumented.
• Please add the option in the integration tests

[IgnatBeresnev]

Why public? In this case I expect I can make public entities undocumented.

I don't see any sensible and valid general use-cases where I as a user would want to document ONLY private/protected/internal code and exclude public (within the same module/package). I can see reasons why you need control over protected/internal/private, but I don't see any to exclude publics, maybe you can provide some examples.

I can, however, see the user forget to include public into

documentedVisibilities.set(setOf(Visibility.PROTECTED))

and then wonder why it doesn't generate documentation for public code (and maybe even not notice it at first). That's the primary reason I decided to make it the default

If you want to exclude a package/module altogether, I believe there's other mechanisms for that in Dokka and it should be used instead of excluding public visibility via documentedVisibilities.

Please add the option in the integration tests

Can you please tell me which tests you have in mind? I think I've added it to all the places I could find that deal with something similar.

[vmishenev]

I mean the existence of Visibility.PUBLIC is ambiguous. Then, on the other hand, documentedVisibilities.set(setOf(Visibility.PROTECTED)) can be considered as only for protected members.
In my opinion it would be more obvious if another name, e.g. documentedExtraVisibilities, was used.

Can you please tell me which tests you have in mind?

For example
https://github.com/Kotlin/dokka/blob/master/integration-tests/gradle/projects/it-basic/build.gradle.kts

[IgnatBeresnev]

I mean the existence of Visibility.PUBLIC is ambiguous.

Yeah, I thought about that and I agree it raises some questions, but at the same time I think it would allow for a clearer configuration where the author needs to show/stress their intentions by using the default value, like in the example below

named("main") {
    documentedVisibilities.set(
        setOf(
            Visibility.PUBLIC,
            Visibility.PRIVATE,
            Visibility.PROTECTED,
            Visibility.INTERNAL
        )
    )
    perPackageOption {
        matchingRegex.set(".*api.*")
        documentedVisibilities.set(
            setOf(
                Visibility.PUBLIC
            )
        )
    }
}

I thought about it more and out of all the options I see (all of them have trade offs, including renaming), I least dislike the idea of removing hardcoded generation for publics and generating documentation for whatever is set in documentedVisibilities. The default will be setOf(Visibility.PUBLIC).

I guess the worst that can happen in this case is the user can forge to include public if they override defaults, but at least the API will be concise.

What do you think about that?

[vmishenev]

Compare two configurations:

 ...
documentedVisibilities.set(
        setOf(
            Visibility.PRIVATE,
        )
    ) 

 ...
documentedVisibilities.set(
        setOf(
            Visibility.PUBLIC,
            Visibility.PRIVATE
        )
    ) 

Is it easy to tell the difference between them without looking in our documentation?
The configuration must be self-documenting and obvious. It is difficult to guess the default inclusion Visibility.PUBLIC into an user-filled set by the only configuration.

Why do you dislike the idea of renaming to documentedExtraVisibilities (for example)?

[IgnatBeresnev]

It is difficult to guess the default inclusion Visibility.PUBLIC into an user-filled set by the only configuration.

As I said earlier, I agree and that's why I proposed to remove the hardcoded inclusion of public up above.

Why do you dislike the idea of renaming to documentedExtraVisibilities (for example)?

Renaming doesn't really solve it completely. It gives a hint something's off, but it raises the same question - what's the default?

• If something's extra, then something is base. It's safe to assume public would in in the base documented visibilities, but what about protected? You'd have to look at enum values or read documentation for the definition of extra anyway
• *extraVisibilities to me sounds like there are more visibilities than the ones defined by stdlib (like protected internal). extraDocumentedVisibilities is better, but one might think that these visibilities would be documented more than others (like extra documentation). I initially had includeVisibilities as the name, but it still doesn't solve the issue that you brought up
• If we go by extra / additional then having Visibility.PUBLIC would make little sense and we'd have to remove it (since it wouldn't be 'additional', it'd be base), which limits the use of this enum (it won't be complete) if we or plugin creators need similar settings. We could rename the enum, of course, but ExtraVisibilities.PRIVATE looks strange

As I mentioned, I think the option with least questions would be the straightforward approach:

If you write this, you only get private documented.

documentedVisibilities.set(
        setOf(
            Visibility.PRIVATE,
        )
    ) 

If you write this, then it's public + private that are documented.

documentedVisibilities.set(
        setOf(
            Visibility.PUBLIC,
            Visibility.PRIVATE
        )
    ) 

If you don't set anything, default is documentedVisibilities = setOf(Visibility.PUBLIC). Although I would consider adding protected to the default as well. It's part of public API and javadoc task generates documentation for protected members.