diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index b95bdaa081..2a4152ccd0 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::env::consts::EXE_SUFFIX; use std::fmt; use std::io::Write; @@ -5,7 +6,7 @@ use std::path::{Path, PathBuf}; use std::process::ExitStatus; use std::str::FromStr; -use anyhow::{anyhow, Error, Result}; +use anyhow::{anyhow, Context, Error, Result}; use clap::{builder::PossibleValue, Args, CommandFactory, Parser, Subcommand, ValueEnum}; use clap_complete::Shell; use itertools::Itertools; @@ -1425,11 +1426,7 @@ macro_rules! docs_data { } impl DocPage { - fn name(&self) -> Option<&'static str> { - Some(self.path()?.rsplit_once('/')?.0) - } - - fn path(&self) -> Option<&'static str> { + fn path_str(&self) -> Option<&'static str> { $( if self.$ident { return Some($path); } )+ None } @@ -1466,6 +1463,39 @@ docs_data![ (unstable_book, "The Unstable Book", "unstable-book/index.html"), ]; +impl DocPage { + fn path(&self) -> Option<&'static Path> { + self.path_str().map(Path::new) + } + + fn name(&self) -> Option<&'static str> { + Some(self.path_str()?.rsplit_once('/')?.0) + } + + fn resolve<'t>(&self, root: &Path, topic: &'t str) -> Option<(PathBuf, Option<&'t str>)> { + // Use `.parent()` to chop off the default top-level `index.html`. + let mut base = root.join(Path::new(self.path()?).parent()?); + base.extend(topic.split("::")); + let base_index_html = base.join("index.html"); + + if base_index_html.is_file() { + return Some((base_index_html, None)); + } + + let base_html = base.with_extension("html"); + if base_html.is_file() { + return Some((base_html, None)); + } + + let parent_html = base.parent()?.with_extension("html"); + if parent_html.is_file() { + return Some((parent_html, topic.rsplit_once("::").map(|(_, s)| s))); + } + + None + } +} + async fn doc( cfg: &Cfg<'_>, path_only: bool, @@ -1500,32 +1530,40 @@ async fn doc( } }; - let topical_path: PathBuf; - - let doc_url = if let Some(topic) = topic { - topical_path = topical_doc::local_path(&toolchain.doc_path("").unwrap(), topic)?; - topical_path.to_str().unwrap() - } else { - topic = doc_page.name(); - doc_page.path().unwrap_or("index.html") + let (doc_path, fragment) = match (topic, doc_page.name()) { + (Some(topic), Some(name)) => { + let (doc_path, fragment) = doc_page + .resolve(&toolchain.doc_path("")?, topic) + .context(format!("no document for {name} on {topic}"))?; + (Cow::Owned(doc_path), fragment) + } + (Some(topic), None) => { + let doc_path = topical_doc::local_path(&toolchain.doc_path("").unwrap(), topic)?; + (Cow::Owned(doc_path), None) + } + (None, name) => { + topic = name; + let doc_path = doc_page.path().unwrap_or(Path::new("index.html")); + (Cow::Borrowed(doc_path), None) + } }; if path_only { - let doc_path = toolchain.doc_path(doc_url)?; + let doc_path = toolchain.doc_path(&doc_path)?; writeln!(cfg.process.stdout().lock(), "{}", doc_path.display())?; - Ok(utils::ExitCode(0)) + return Ok(utils::ExitCode(0)); + } + + if let Some(name) = topic { + writeln!( + cfg.process.stderr().lock(), + "Opening docs named `{name}` in your browser" + )?; } else { - if let Some(name) = topic { - writeln!( - cfg.process.stderr().lock(), - "Opening docs named `{name}` in your browser" - )?; - } else { - writeln!(cfg.process.stderr().lock(), "Opening docs in your browser")?; - } - toolchain.open_docs(doc_url)?; - Ok(utils::ExitCode(0)) + writeln!(cfg.process.stderr().lock(), "Opening docs in your browser")?; } + toolchain.open_docs(&doc_path, fragment)?; + Ok(utils::ExitCode(0)) } #[cfg(not(windows))] diff --git a/src/cli/topical_doc.rs b/src/cli/topical_doc.rs index b0a181f405..091c56e101 100644 --- a/src/cli/topical_doc.rs +++ b/src/cli/topical_doc.rs @@ -2,7 +2,7 @@ use std::ffi::OsString; use std::fs; use std::path::{Path, PathBuf}; -use anyhow::{anyhow, bail, Context, Result}; +use anyhow::{anyhow, Context, Result}; struct DocData<'a> { topic: &'a str, @@ -19,13 +19,10 @@ fn index_html(doc: &DocData<'_>, wpath: &Path) -> Option { } fn dir_into_vec(dir: &Path) -> Result> { - let entries = fs::read_dir(dir).with_context(|| format!("Failed to read_dir {dir:?}"))?; - let mut v = Vec::new(); - for entry in entries { - let entry = entry?; - v.push(entry.file_name()); - } - Ok(v) + fs::read_dir(dir) + .with_context(|| format!("Failed to read_dir {dir:?}"))? + .map(|f| Ok(f?.file_name())) + .collect() } fn search_path(doc: &DocData<'_>, wpath: &Path, keywords: &[&str]) -> Result { @@ -118,18 +115,14 @@ pub(crate) fn local_path(root: &Path, topic: &str) -> Result { // topic.split.count cannot be 0 let subpath_os_path = match topic_vec.len() { 1 => match topic { - "std" | "core" | "alloc" => match index_html(&doc, &work_path) { - Some(f) => f, - None => bail!(format!("No document for '{}'", doc.topic)), - }, - _ => { - let std = PathBuf::from("std"); - let search_keywords = match forced_keyword { - Some(k) => k, - None => keywords_top, - }; - search_path(&doc, &std, &search_keywords)? + "std" | "core" | "alloc" => { + index_html(&doc, &work_path).context(anyhow!("No document for '{}'", doc.topic))? } + _ => search_path( + &doc, + Path::new("std"), + &forced_keyword.unwrap_or(keywords_top), + )?, }, 2 => match index_html(&doc, &work_path) { Some(f) => f, diff --git a/src/test/mock/topical_doc_data.rs b/src/test/mock/topical_doc_data.rs index e8f93a37be..3c7fc93013 100644 --- a/src/test/mock/topical_doc_data.rs +++ b/src/test/mock/topical_doc_data.rs @@ -3,23 +3,29 @@ use std::path::PathBuf; // Paths are written as a string in the UNIX format to make it easy // to maintain. -static TEST_CASES: &[&[&str]] = &[ - &["core", "core/index.html"], - &["core::arch", "core/arch/index.html"], - &["fn", "std/keyword.fn.html"], - &["keyword:fn", "std/keyword.fn.html"], - &["primitive:fn", "std/primitive.fn.html"], - &["macro:file!", "std/macro.file!.html"], - &["macro:file", "std/macro.file.html"], - &["std::fs", "std/fs/index.html"], - &["std::fs::read_dir", "std/fs/fn.read_dir.html"], - &["std::io::Bytes", "std/io/struct.Bytes.html"], - &["std::iter::Sum", "std/iter/trait.Sum.html"], - &["std::io::error::Result", "std/io/error/type.Result.html"], - &["usize", "std/primitive.usize.html"], - &["eprintln", "std/macro.eprintln.html"], - &["alloc::format", "alloc/macro.format.html"], - &["std::mem::MaybeUninit", "std/mem/union.MaybeUninit.html"], +static TEST_CASES: &[(&[&str], &str)] = &[ + (&["core"], "core/index.html"), + (&["core::arch"], "core/arch/index.html"), + (&["fn"], "std/keyword.fn.html"), + (&["keyword:fn"], "std/keyword.fn.html"), + (&["primitive:fn"], "std/primitive.fn.html"), + (&["macro:file!"], "std/macro.file!.html"), + (&["macro:file"], "std/macro.file.html"), + (&["std::fs"], "std/fs/index.html"), + (&["std::fs::read_dir"], "std/fs/fn.read_dir.html"), + (&["std::io::Bytes"], "std/io/struct.Bytes.html"), + (&["std::iter::Sum"], "std/iter/trait.Sum.html"), + (&["std::io::error::Result"], "std/io/error/type.Result.html"), + (&["usize"], "std/primitive.usize.html"), + (&["eprintln"], "std/macro.eprintln.html"), + (&["alloc::format"], "alloc/macro.format.html"), + (&["std::mem::MaybeUninit"], "std/mem/union.MaybeUninit.html"), + (&["--rustc", "lints"], "rustc/lints/index.html"), + (&["--rustdoc", "lints"], "rustdoc/lints.html"), + ( + &["lints::broken_intra_doc_links", "--rustdoc"], + "rustdoc/lints.html", + ), ]; fn repath(origin: &str) -> String { @@ -30,8 +36,8 @@ fn repath(origin: &str) -> String { repathed.into_os_string().into_string().unwrap() } -pub fn test_cases<'a>() -> impl Iterator { - TEST_CASES.iter().map(|x| (x[0], repath(x[1]))) +pub fn test_cases<'a>() -> impl Iterator { + TEST_CASES.iter().map(|(args, path)| (*args, repath(path))) } pub fn unique_paths() -> impl Iterator { @@ -39,6 +45,6 @@ pub fn unique_paths() -> impl Iterator { let mut unique_paths = HashSet::new(); TEST_CASES .iter() - .filter(move |e| unique_paths.insert(e[1])) - .map(|e| repath(e[1])) + .filter(move |(_, p)| unique_paths.insert(p)) + .map(|(_, p)| repath(p)) } diff --git a/src/toolchain.rs b/src/toolchain.rs index 8e80838b1a..1253f73e14 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -10,9 +10,10 @@ use std::{ time::Duration, }; -use anyhow::{anyhow, bail}; +use anyhow::{anyhow, bail, Context}; use fs_at::OpenOptions; use tracing::info; +use url::Url; use wait_timeout::ChildExt; use crate::{ @@ -408,19 +409,30 @@ impl<'a> Toolchain<'a> { buf } - pub fn doc_path(&self, relative: &str) -> anyhow::Result { - let parts = vec!["share", "doc", "rust", "html"]; - let mut doc_dir = self.path.clone(); - for part in parts { - doc_dir.push(part); + pub fn doc_path(&self, relative: impl AsRef) -> anyhow::Result { + let relative = relative.as_ref(); + if relative.is_absolute() { + return Ok(relative.to_owned()); } + + let mut doc_dir = self.path.clone(); + doc_dir.extend(["share", "doc", "rust", "html"]); doc_dir.push(relative); Ok(doc_dir) } - pub fn open_docs(&self, relative: &str) -> anyhow::Result<()> { - utils::open_browser(&self.doc_path(relative)?) + pub fn open_docs( + &self, + relative: impl AsRef, + fragment: Option<&str>, + ) -> anyhow::Result<()> { + let relative = relative.as_ref(); + let mut doc_url = Url::from_file_path(self.doc_path(relative)?) + .ok() + .with_context(|| anyhow!("invalid doc file absolute path `{}`", relative.display()))?; + doc_url.set_fragment(fragment); + utils::open_browser(doc_url.to_string()) } /// Remove the toolchain from disk diff --git a/src/utils/mod.rs b/src/utils/mod.rs index dae45d4b73..e1e144cfa2 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -1,6 +1,7 @@ //! Utility functions for Rustup use std::env; +use std::ffi::OsStr; use std::fs::{self, File}; use std::io::{self, BufReader, Write}; use std::ops::{BitAnd, BitAndAssign}; @@ -459,7 +460,7 @@ pub(crate) fn read_dir(name: &'static str, path: &Path) -> Result { }) } -pub(crate) fn open_browser(path: &Path) -> Result<()> { +pub(crate) fn open_browser(path: impl AsRef) -> Result<()> { opener::open_browser(path).context("couldn't open browser") } diff --git a/tests/suite/cli_rustup.rs b/tests/suite/cli_rustup.rs index 687eaf7ff4..bae23ad5b7 100644 --- a/tests/suite/cli_rustup.rs +++ b/tests/suite/cli_rustup.rs @@ -2640,8 +2640,12 @@ async fn docs_topical_with_path() { .expect_ok(&["rustup", "toolchain", "install", "nightly"]) .await; - for (topic, path) in mock::topical_doc_data::test_cases() { - let mut cmd = clitools::cmd(&cx.config, "rustup", ["doc", "--path", topic]); + for (args, path) in mock::topical_doc_data::test_cases() { + let mut cmd = clitools::cmd( + &cx.config, + "rustup", + ["doc", "--path"].iter().chain(args.iter()), + ); clitools::env(&cx.config, &mut cmd); let out = cmd.output().unwrap(); @@ -2649,7 +2653,7 @@ async fn docs_topical_with_path() { let out_str = String::from_utf8(out.stdout).unwrap(); assert!( out_str.contains(&path), - "comparing path\ntopic: '{topic}'\nexpected path: '{path}'\noutput: {out_str}\n\n\n", + "comparing path\nargs: '{args:?}'\nexpected path: '{path}'\noutput: {out_str}\n\n\n", ); } }