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

Make hasdoc public #52708

Merged
merged 5 commits into from
Jan 18, 2024
Merged

Make hasdoc public #52708

merged 5 commits into from
Jan 18, 2024

Conversation

jariji
Copy link
Contributor

@jariji jariji commented Jan 3, 2024

Making Docs.hasdoc public per @LilithHafner #52139 (comment).

base/docs/Docs.jl Outdated Show resolved Hide resolved
@stevengj stevengj added the docsystem The documentation building system label Jan 3, 2024
@stevengj
Copy link
Member

stevengj commented Jan 3, 2024

Should it simply be exported?

(Note that even if you add export hasdoc here, it still won't be re-exported by Base by default.)

@jariji
Copy link
Contributor Author

jariji commented Jan 3, 2024

Now exporting undocumented_names from Docs too, per #52708 (comment).

s/exported/public/
@stevengj
Copy link
Member

stevengj commented Jan 3, 2024

I also fixed the docstring to refer to "public" symbols rather than "exported" symbols, following #52413 (comment) by @LilithHafner

@LilithHafner
Copy link
Member

I don't see much advantage of exporting from a submodule (historically, I'm guessing the primary uses were to mark the symbols public and when a symbol was previously defined in Base and moved to a submodule)

@jariji
Copy link
Contributor Author

jariji commented Jan 3, 2024

In favor of export:

  • people who do using Base.Docs can get the convenience they want.
  • export is the most common way of making something public, so without a particular reason against, it seems good to keep with the normal practice. It's nice to have a single way of doing things that works as often as possible.

@stevengj
Copy link
Member

stevengj commented Jan 3, 2024

If we are going to export doc and @var and @text_str (some of which aren't even documented), it doesn't make sense to me not to export hasdoc and documented_symbols (which are much more likely to be used outside of Base). And, after all, people can do using Base.Docs or using .Docs` as @jariji pointed out.

@stevengj
Copy link
Member

stevengj commented Jan 3, 2024

(After this, I think we need to make a concerted effort to get @test isempty(undocumented_symbols(Foo)) to pass for stdlib modules and documented submodules. See #52723 and #52725.)

@stevengj
Copy link
Member

Bump.

@LilithHafner
Copy link
Member

The declared semantics of undocumented_names (based on isidentifier) don't seem viable to me right now (e.g. it excludes macros and operators) so we shouldn't publicize them.

c.f. #52413 (comment)

The original commit (make hasdoc public) is definitely mergable, though.

@stevengj
Copy link
Member

stevengj commented Jan 17, 2024

@LilithHafner, undocumented_names was already changed (at your request) to include all public symbols, including macros and operators, that don't begin with # (to exclude generated internal symbols, ala gensym), don't you remember? #52743

(This PR just needs to be updated merge the latest master branch and fix merge conflicts.)

@LilithHafner
Copy link
Member

don't you remember?

Clearly not, lol. You are quite right.

@LilithHafner LilithHafner added the merge me PR is reviewed. Merge when all tests are passing label Jan 17, 2024
@IanButterworth IanButterworth merged commit 8f2f178 into JuliaLang:master Jan 18, 2024
8 checks passed
@IanButterworth IanButterworth removed the merge me PR is reviewed. Merge when all tests are passing label Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docsystem The documentation building system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants