-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Document private items for binary crates by default #7593
Conversation
r? @Eh2406 (rust_highfive has picked a reviewer for you, use r? to override) |
Thanks for the PR! We discussed this briefly, and I don't think anyone had any objections to the general plan. @Eh2406 will try to follow up with a review. Just a side note, this should probably have a small test to validate the behavior. |
Looks good to me with a test! |
Ooops, that was a typo! I of course meant Regarding tests: will add them soon, hopefully in roughly 10 hours, but certainly this week still. |
I think it is fine without a flag. If it seems like something people really want, we can add it later. |
I added tests now. Are the two tests sufficient? Or not, what else to test? This PR is now ready, I'd say :) |
@bors r+ |
📌 Commit 00b21c8 has been approved by |
Document private items for binary crates by default I suggested this change in default behavior [a long time ago](#1520 (comment)) and [a year ago again](#1520 (comment)). Both time, everyone seemed to agree that this is a good idea, so I thought I could just implement it. This PR already changed the default behavior, but there are a few things we should talk about/I should do before merging: - [x] ~~Do we *really* want this changed default behavior? If not, *why* not?~~ -> [yip](#7593 (comment)) - [x] Is changing this default behavior OK regarding backwards compatibility? -> [apparently](#7593 (comment)) - [x] ~~We should also probably add a `--document-only-public-items` flag or something like that if we change the default behavior. Otherwise users have no way to not document private items for binary crates. Right?~~ -> [We can do it later](#7593 (comment)) - [x] I should probably add some tests for this new behavior -> I did - [ ] I don't like that I added `rustdoc_document_private_items` to `CompileOptions`, but this was the sanest way I could think of without rewriting a whole lot. Is this OK or how else would one do it? The flag would belong to `DocOptions`. But `compile` does not have access to that. Any ideas? Btw: If we also add `--document-only-private-items`, I would change the type from `bool` to `Option<bool>`.
☀️ Test successful - checks-azure |
Yeah :) Not to be attention-seeking, but I think this deserves |
Update cargo, rls, books. ## nomicon 1 commits in 58e36e0e08dec5a379ac568827c058e25990d6cd..041c46e692a2592853aeca132c8dfe8eb5a79a9e 2019-10-30 08:14:24 -0500 to 2019-11-20 16:46:45 +0100 - Update unsafe-code-guidelines link (rust-lang/nomicon#175) ## cargo 15 commits in 8280633db680dec5bfe1de25156d1a1d53e6d190..750cb1482e4d0e74822cded7ab8b3c677ed8b041 2019-11-11 23:17:05 +0000 to 2019-11-23 23:06:36 +0000 - Some random comments and docstrings. (rust-lang/cargo#7625) - Add value OUT_DIR to build-script-executed JSON message (rust-lang/cargo#7622) - Update documentation for custom target dependencies. (rust-lang/cargo#7623) - Document private items for binary crates by default (rust-lang/cargo#7593) - Extend documentation on security concerns of crate names in a registry. (rust-lang/cargo#7616) - Stabilize install-upgrade. (rust-lang/cargo#7560) - Turn the new lock file format on by default (rust-lang/cargo#7579) - bump im-rc version (rust-lang/cargo#7609) - Ignore file lock errors if unsupported, on Windows (rust-lang/cargo#7602) - Add hack for fwdansi change. (rust-lang/cargo#7607) - Document Cargo's JSON output. (rust-lang/cargo#7595) - Remove "cargo login" from user input when asking for login token. (rust-lang/cargo#7588) - Fix all Clippy suggestions (but not add it to CI 🙃) (rust-lang/cargo#7574) - Add kind/platform info to `cargo metadata` (rust-lang/cargo#7132) - Update core-foundation requirement from 0.6.0 to 0.7.0 (rust-lang/cargo#7585) ## reference 2 commits in 45558c4..9e843ae 2019-11-08 14:47:35 +0100 to 2019-11-24 17:44:04 +0100 - Minor never type additions. (rust-lang/reference#723) - Update associated-items.md. "it"->is (rust-lang/reference#721) ## book 3 commits in e79dd62aa63396714278d484d91d48826737f47f..81ebaa2a3f88d4d106516c489682e64cacba4f60 2019-10-30 07:33:12 -0500 to 2019-11-15 08:30:04 -0800 - small fix ch04-03 & code block typo ch07-02 (rust-lang/book#2138) - Adapt content of Chapter 16.3 in order to be consistent with improved compiler message (rust-lang/book#1779) - [Rust 1.35] Remove FnBox and use builtin impl FnOnce for Box<FnOnce()> instead. (rust-lang/book#1906) ## rls 3 commits in 5db91c7b94ca81eead6b25bcf6196b869a44ece0..9ec2b8cb57c87517bcb506ac302eae339ffa2025 2019-10-30 16:04:39 +0100 to 2019-11-24 23:16:11 +0100 - Fix test for latest nightly. (rust-lang/rls#1595) - doc: contributing: Remove outdated LSP extension (rust-lang/rls#1594) - Update cargo. (rust-lang/rls#1591) ## rust-by-example 1 commits in dcee312c66267eb5a2f6f1561354003950e29105..4835e025826729827a94fdeb7cb85fed288d08bb 2019-10-31 11:26:53 -0300 to 2019-11-14 09:20:43 -0300 - crates: fix suggested value for --crate-type flag (rust-lang/rust-by-example#1292) ## edition-guide 1 commits in f553fb26c60c4623ea88a1cfe731eafe0643ce34..6601cab4666596494a569f94aa63b7b3230e9769 2019-10-30 08:27:42 -0500 to 2019-11-22 12:08:58 -0500 - Remove final nursery reference
Version 1.41.0 (2020-01-30) =========================== Language -------- - [You can now pass type parameters to foreign items when implementing traits.][65879] E.g. You can now write `impl<T> From<Foo> for Vec<T> {}`. - [You can now arbitrarily nest receiver types in the `self` position.][64325] E.g. you can now write `fn foo(self: Box<Box<Self>>) {}`. Previously only `Self`, `&Self`, `&mut Self`, `Arc<Self>`, `Rc<Self>`, and `Box<Self>` were allowed. - [You can now use any valid identifier in a `format_args` macro.][66847] Previously identifiers starting with an underscore were not allowed. - [Visibility modifiers (e.g. `pub`) are now syntactically allowed on trait items and enum variants.][66183] These are still rejected semantically, but can be seen and parsed by procedural macros and conditional compilation. Compiler -------- - [Rustc will now warn if you have unused loop `'label`s.][66325] - [Removed support for the `i686-unknown-dragonfly` target.][67255] - [Added tier 3 support\* for the `riscv64gc-unknown-linux-gnu` target.][66661] - [You can now pass an arguments file passing the `@path` syntax to rustc.][66172] Note that the format differs somewhat from what is found in other tooling; please see [the documentation][argfile-docs] for more information. - [You can now provide `--extern` flag without a path, indicating that it is available from the search path or specified with an `-L` flag.][64882] \* Refer to Rust's [platform support page][forge-platform-support] for more information on Rust's tiered platform support. [argfile-docs]: https://doc.rust-lang.org/nightly/rustc/command-line-arguments.html#path-load-command-line-flags-from-a-path Libraries --------- - [The `core::panic` module is now stable.][66771] It was already stable through `std`. - [`NonZero*` numerics now implement `From<NonZero*>` if it's a smaller integer width.][66277] E.g. `NonZeroU16` now implements `From<NonZeroU8>`. - [`MaybeUninit<T>` now implements `fmt::Debug`.][65013] Stabilized APIs --------------- - [`Result::map_or`] - [`Result::map_or_else`] - [`std::rc::Weak::weak_count`] - [`std::rc::Weak::strong_count`] - [`std::sync::Weak::weak_count`] - [`std::sync::Weak::strong_count`] Cargo ----- - [Cargo will now document all the private items for binary crates by default.][cargo/7593] - [`cargo-install` will now reinstall the package if it detects that it is out of date.][cargo/7560] - [Cargo.lock now uses a more git friendly format that should help to reduce merge conflicts.][cargo/7579] - [You can now override specific dependencies's build settings][cargo/7591] E.g. `[profile.dev.overrides.image] opt-level = 2` sets the `image` crate's optimisation level to `2` for debug builds. You can also use `[profile.<profile>.build_overrides]` to override build scripts and their dependencies. Misc ---- - [You can now specify `edition` in documentation code blocks to compile the block for that edition.][66238] E.g. `edition2018` tells rustdoc that the code sample should be compiled the 2018 edition of Rust. - [You can now provide custom themes to rustdoc with `--theme`, and check the current theme with `--check-theme`.][54733] - [You can use `#[cfg(doc)]` to compile an item when building documentation.][61351] Compatibility Notes ------------------- - [As previously announced 1.41.0 will be the last tier 1 release for 32-bit Apple targets.][apple-32bit-drop] This means that the source code is still available to build, but the targets are no longer being tested and release binaries for those platforms will no longer be distributed by the Rust project. Please refer to the linked blog post for more information. [54733]: rust-lang/rust#54733 [61351]: rust-lang/rust#61351 [67255]: rust-lang/rust#67255 [66661]: rust-lang/rust#66661 [66771]: rust-lang/rust#66771 [66847]: rust-lang/rust#66847 [66238]: rust-lang/rust#66238 [66277]: rust-lang/rust#66277 [66325]: rust-lang/rust#66325 [66172]: rust-lang/rust#66172 [66183]: rust-lang/rust#66183 [65879]: rust-lang/rust#65879 [65013]: rust-lang/rust#65013 [64882]: rust-lang/rust#64882 [64325]: rust-lang/rust#64325 [cargo/7560]: rust-lang/cargo#7560 [cargo/7579]: rust-lang/cargo#7579 [cargo/7591]: rust-lang/cargo#7591 [cargo/7593]: rust-lang/cargo#7593 [`Result::map_or_else`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.map_or_else [`Result::map_or`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.map_or [`std::rc::Weak::weak_count`]: https://doc.rust-lang.org/std/rc/struct.Weak.html#method.weak_count [`std::rc::Weak::strong_count`]: https://doc.rust-lang.org/std/rc/struct.Weak.html#method.strong_count [`std::sync::Weak::weak_count`]: https://doc.rust-lang.org/std/sync/struct.Weak.html#method.weak_count [`std::sync::Weak::strong_count`]: https://doc.rust-lang.org/std/sync/struct.Weak.html#method.strong_count [apple-32bit-drop]: https://blog.rust-lang.org/2020/01/03/reducing-support-for-32-bit-apple-targets.html
Thanks for doing this work! To be precise, I think this change causes |
@davepacheco I think the original description is right, though cargo and rustc extremely confusingly use different terms for the same concept.
If you go by https://doc.rust-lang.org/1.44.0/cargo/reference/cargo-targets.html, this change documents private items for binary targets in all packages by default. If you go by https://doc.rust-lang.org/1.44.0/book/ch07-01-packages-and-crates.html, this change documents private items for binary crates in all packages by default, with targets being a pretty different thing (https://doc.rust-lang.org/1.44.0/rustc/targets/index.html). In either case, a collection of Rust crates/targets described by one Cargo.toml is called a Cargo package. I think the use of "targets" for crates has been falling out of favor because targets already means the other thing, except maybe inside the Cargo codebase. |
Thanks for that clarification! Yeah, I got here from the |
Summary: This matches Cargo's behavior. See [rust-lang/cargo#7593](rust-lang/cargo#7593). Documentation for `rust_binary` targets is otherwise useless, since usually there is nothing in them declared `pub`. Reviewed By: zertosh Differential Revision: D36563393 fbshipit-source-id: 9ee64fb3f77caf7376a93eabfb309dc0adc1b45d
Fixes rust-lang#7963. Previously, --document-private-items was only a 'truthy' flag, and was enabled in rust-lang#7593 for binary targets by default. However, this prevented any means of *disabling* this functionality for binary targets, hence rust-lang#7963. This change does a few things. It first adds a new argument parser type called `yesno_flag()` that is a wrapper around a few `clap::Arg` settings to use the built-in `BoolishValueParser` in a way that allows for the `--flag` to return `true`, as well as an optional `=no` or `=yes` (or `=1`, `=off`, etc.) to explicitly set it to `true` or `false`. Then, the internal field for passing the private member documentation flag to rustdoc was changed to an `Option<bool>` to treat `None` as the automatic project type detection, and a `Some` value to override the behavior outright. This change should be entirely backwards compatible.
Fixes rust-lang#7963. Previously, --document-private-items was only a 'truthy' flag, and was enabled in rust-lang#7593 for binary targets by default. However, this prevented any means of *disabling* this functionality for binary targets, hence rust-lang#7963. This change does a few things. It first adds a new argument parser type called `yesno_flag()` that is a wrapper around a few `clap::Arg` settings to use the built-in `BoolishValueParser` in a way that allows for the `--flag` to return `true`, as well as an optional `=no` or `=yes` (or `=1`, `=off`, etc.) to explicitly set it to `true` or `false`. Then, the internal field for passing the private member documentation flag to rustdoc was changed to an `Option<bool>` to treat `None` as the automatic project type detection, and a `Some` value to override the behavior outright. This change should be entirely backwards compatible.
Fixes rust-lang#7963. Previously, --document-private-items was only a 'truthy' flag, and was enabled in rust-lang#7593 for binary targets by default. However, this prevented any means of *disabling* this functionality for binary targets, hence rust-lang#7963. This change does a few things. It first adds a new argument parser type called `yesno_flag()` that is a wrapper around a few `clap::Arg` settings to use the built-in `BoolishValueParser` in a way that allows for the `--flag` to return `true`, as well as an optional `=no` or `=yes` (or `=1`, `=off`, etc.) to explicitly set it to `true` or `false`. Then, the internal field for passing the private member documentation flag to rustdoc was changed to an `Option<bool>` to treat `None` as the automatic project type detection, and a `Some` value to override the behavior outright. This change should be entirely backwards compatible.
Just as a note, I'm running into the case where I don't want private items documented for a binary. In fact, I think more refinement should be made to allow the types of visibility to be specified (e.g. crate public vs. module public) - but a discussion should happen there. Initial PR at #10979. |
I suggested this change in default behavior a long time ago and a year ago again. Both time, everyone seemed to agree that this is a good idea, so I thought I could just implement it.
This PR already changed the default behavior, but there are a few things we should talk about/I should do before merging:
Do we really want this changed default behavior? If not, why not?-> yipWe should also probably add a-> We can do it later--document-only-public-items
flag or something like that if we change the default behavior. Otherwise users have no way to not document private items for binary crates. Right?rustdoc_document_private_items
toCompileOptions
, but this was the sanest way I could think of without rewriting a whole lot. Is this OK or how else would one do it? The flag would belong toDocOptions
. Butcompile
does not have access to that. Any ideas? Btw: If we also add--document-only-private-items
, I would change the type frombool
toOption<bool>
.