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 docstring to constants in Base.Filesystem #53247

Merged
merged 6 commits into from
Feb 10, 2024

Conversation

lgoettgens
Copy link
Contributor

A first step towards #52725 for Base.Filesystem.

The type of combined docstring was inspired by

julia/base/libdl.jl

Lines 33 to 46 in 72d3abe

"""
RTLD_DEEPBIND
RTLD_FIRST
RTLD_GLOBAL
RTLD_LAZY
RTLD_LOCAL
RTLD_NODELETE
RTLD_NOLOAD
RTLD_NOW
Enum constant for [`dlopen`](@ref). See your platform man page for details, if
applicable.
"""
(RTLD_DEEPBIND, RTLD_FIRST, RTLD_GLOBAL, RTLD_LAZY, RTLD_LOCAL, RTLD_NODELETE, RTLD_NOLOAD, RTLD_NOW)

cc @stevengj

JL_O_WRONLY

Enum constant for the `open` syscall, where `JL_O_*` corresponds to the `O_*` constant.
See your platform man page for details, if applicable.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The details actually come from here https://docs.libuv.org/en/v1.x/fs.html#file-open-constants if you want to link or copy that here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The page doesn't explain all of them, so I rather not link it here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which is missing? I am also a maintainer of that documentation, so it would help to know if one is missing from it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ASYNC, CLOEXEC, LARGEFILE, PATH, RSYNC, TMPFILE are missing from the libuv docs.
And there are some in the libuv docs that are not there in julia.
But if you say that this is the ultimate docu for this, I'll add a link to the docstring.

Suggested change
See your platform man page for details, if applicable.
See [the libuv docs](https://docs.libuv.org/en/v1.x/fs.html#file-open-constants) for more details.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably delete ASYNC, CLOEXEC, LARGEFILE, and RSYNC exports from our list, since we neither define nor support any of those either. Looks like O_PATH and O_TMPFILE are the only (linux-specific) ones we might pass through successfully, so we could put an asterisk next to those noting they are linux-only.

@vtjnash vtjnash added the docs This change adds or pertains to documentation label Feb 8, 2024
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Feb 8, 2024
JL_O_TRUNC
JL_O_WRONLY

Enum constant for the `open` syscall, where `JL_O_*` corresponds to the `O_*` constant.
Copy link
Member

@stevengj stevengj Feb 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not an "enum" in the sense of a Julia @enum value. They are just UInt32 constants.

(For some reason, JL_O_TMPFILE is a UInt16, though?? Is this a bug?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, a lot of these are defined incorrectly now, including being the wrong type in some cases, and that they don't follow the UV names that are now consistently defined for them instead our current ad-hoc definitions

@vtjnash
Copy link
Member

vtjnash commented Feb 10, 2024

Just a subtle issue that only a more primitive docs macro is available now which can only handle simpler forms of docs. I have edited it to the simpler form of this, which I think should work, based on testing:

julia> Core.atdoc = Base.CoreDocs.docm

julia> @doc "foo docs" (:foo,)

julia> Core.atdoc = Base.Docs.docm

julia> Base.Docs.loaddocs(Core.Compiler.CoreDocs.DOCS)

@vtjnash
Copy link
Member

vtjnash commented Feb 10, 2024

The tests seem to indicate a few a still missing:

  Expression: undoc == [:File, :Filesystem, :cptree, :futime, :rename, :sendfile, :unlink]
2024-02-10 08:06:10 UTC	   Evaluated: [:File, :Filesystem, :JL_O_FSYNC, :JL_O_NDELAY, :JL_O_NOFOLLOW, :cptree, :futime, :rename, :sendfile, :unlink] == [:File, :Filesystem, :cptree, :futime, :rename, :sendfile, :unlink]

@vtjnash vtjnash removed the merge me PR is reviewed. Merge when all tests are passing label Feb 10, 2024
@vtjnash vtjnash merged commit 604609a into JuliaLang:master Feb 10, 2024
4 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants