Skip to content

Conversation

@gwenn
Copy link
Contributor

@gwenn gwenn commented Mar 30, 2025

Apply:
https://users.rust-lang.org/t/how-to-document-optional-features-in-api-docs/64577/3
And update build script accordingly
Currently, there is a warning:

warning: unresolved link to `Perform::advance`
   --> src/lib.rs:139:15
    |
139 |     /// See [`Perform::advance`] for more details.
    |               ^^^^^^^^^^^^^^^^ the trait `Perform` has no associated item named `advance`
    |
    = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default

warning: `vte` (lib doc) generated 1 warning

But I am not sure if the link should be on Parser::advance or Perfom::terminated.

@chrisduerr
Copy link
Member

But I am not sure if the link should be on Parser::advance or Perfom::terminated.

Should be Self::advance.

Copy link
Member

@chrisduerr chrisduerr left a comment

Choose a reason for hiding this comment

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

Seems reasonable, I'll take a proper look once CI passes.

Generally if there's other features that are doc relevant, they should also be considered for this patch though. No reason to limit it to ansi.

warning: unresolved link to `Perform::advance`
@gwenn
Copy link
Contributor Author

gwenn commented Apr 1, 2025

Seems reasonable, I'll take a proper look once CI passes.

Done.

Generally if there's other features that are doc relevant, they should also be considered for this patch though. No reason to limit it to ansi.

I see only serde but it doesn't seem relevant to me.

@chrisduerr chrisduerr self-requested a review April 1, 2025 16:24
- rustdoc: |
$HOME/.cargo/bin/rustup toolchain install nightly -c rust-docs
cd vte
RUSTDOCFLAGS="--cfg docsrs -Dwarnings" $HOME/.cargo/bin/cargo +nightly doc --features=ansi --no-deps
Copy link
Member

Choose a reason for hiding this comment

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

Is there any reason to have the --cfg docsrs here? What exactly does it achieve? I'd assume rustdoc verification should be largely independent of target documentation platform.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I'd assume rustdoc verification should be largely independent of target documentation platform.

unless you have doc stuff behind docsrs feature, it shouldn't really matter.

Copy link
Member

Choose a reason for hiding this comment

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

unless you have doc stuff behind docsrs feature, it shouldn't really matter.

Which really shouldn't be the case tbh. I don't like singling out docsrs even though it's pretty much the "standard".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am almost sure that we can use any name instead of docsrs.
Because, it is used only to activate the nightly doc_auto_cfg feature.

@chrisduerr chrisduerr merged commit ca3dd62 into alacritty:master Apr 8, 2025
1 check passed
@gwenn gwenn deleted the docsrs branch April 9, 2025 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants