From be0b1121e9ede0cc3b322aba8d8a2a9bb48572f6 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Wed, 29 Jun 2022 13:04:43 -0700 Subject: [PATCH] Replace `sort_modules_alphabetically` boolean with enum This fixes the long-standing FIXME there and makes the code easier to understand. The reference to modules in both the old and new names seems potentially wrong since I believe it applies to all items. --- src/librustdoc/config.rs | 23 +++++++++++++++-------- src/librustdoc/html/render/context.rs | 17 ++++++++++------- src/librustdoc/html/render/print_item.rs | 8 ++++++-- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 6e3651665c8..50d154dd278 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -218,12 +218,9 @@ pub(crate) struct RenderOptions { /// /// Be aware: This option can come both from the CLI and from crate attributes! pub(crate) playground_url: Option, - /// Whether to sort modules alphabetically on a module page instead of using declaration order. - /// `true` by default. - // - // FIXME(misdreavus): the flag name is `--sort-modules-by-appearance` but the meaning is - // inverted once read. - pub(crate) sort_modules_alphabetically: bool, + /// What sorting mode to use for module pages. + /// `ModuleSorting::Alphabetical` by default. + pub(crate) module_sorting: ModuleSorting, /// List of themes to extend the docs with. Original argument name is included to assist in /// displaying errors if it fails a theme check. pub(crate) themes: Vec, @@ -281,6 +278,12 @@ pub(crate) struct RenderOptions { pub(crate) no_emit_shared: bool, } +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub(crate) enum ModuleSorting { + DeclarationOrder, + Alphabetical, +} + #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub(crate) enum EmitType { Unversioned, @@ -650,7 +653,11 @@ impl Options { let proc_macro_crate = crate_types.contains(&CrateType::ProcMacro); let playground_url = matches.opt_str("playground-url"); let maybe_sysroot = matches.opt_str("sysroot").map(PathBuf::from); - let sort_modules_alphabetically = !matches.opt_present("sort-modules-by-appearance"); + let module_sorting = if matches.opt_present("sort-modules-by-appearance") { + ModuleSorting::DeclarationOrder + } else { + ModuleSorting::Alphabetical + }; let resource_suffix = matches.opt_str("resource-suffix").unwrap_or_default(); let enable_minification = !matches.opt_present("disable-minification"); let markdown_no_toc = matches.opt_present("markdown-no-toc"); @@ -731,7 +738,7 @@ impl Options { external_html, id_map, playground_url, - sort_modules_alphabetically, + module_sorting, themes, extension_css, extern_html_root_urls, diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index bfdc44c7e45..2ed7a6f1bb1 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -22,7 +22,7 @@ use super::{ }; use crate::clean::{self, types::ExternalLocation, ExternalCrate}; -use crate::config::RenderOptions; +use crate::config::{ModuleSorting, RenderOptions}; use crate::docfs::{DocFS, PathError}; use crate::error::Error; use crate::formats::cache::Cache; @@ -95,7 +95,7 @@ pub(crate) struct SharedContext<'tcx> { created_dirs: RefCell>, /// This flag indicates whether listings of modules (in the side bar and documentation itself) /// should be ordered alphabetically or in order of appearance (in the source code). - pub(super) sort_modules_alphabetically: bool, + pub(super) module_sorting: ModuleSorting, /// Additional CSS files to be added to the generated docs. pub(crate) style_files: Vec, /// Suffix to be added on resource files (if suffix is "-v2" then "light.css" becomes @@ -280,10 +280,13 @@ impl<'tcx> Context<'tcx> { } } - if self.shared.sort_modules_alphabetically { - for items in map.values_mut() { - items.sort(); + match self.shared.module_sorting { + ModuleSorting::Alphabetical => { + for items in map.values_mut() { + items.sort(); + } } + ModuleSorting::DeclarationOrder => {} } map } @@ -394,7 +397,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { external_html, id_map, playground_url, - sort_modules_alphabetically, + module_sorting, themes: style_files, default_settings, extension_css, @@ -476,7 +479,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { issue_tracker_base_url, layout, created_dirs: Default::default(), - sort_modules_alphabetically, + module_sorting, style_files, resource_suffix, static_root_path, diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index d115185562c..0fe99463f1d 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -23,6 +23,7 @@ use super::{ AssocItemLink, Context, ImplRenderingParameters, }; use crate::clean; +use crate::config::ModuleSorting; use crate::formats::item_type::ItemType; use crate::formats::{AssocItemRender, Impl, RenderMode}; use crate::html::escape::Escape; @@ -246,8 +247,11 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items: compare_names(lhs.as_str(), rhs.as_str()) } - if cx.shared.sort_modules_alphabetically { - indices.sort_by(|&i1, &i2| cmp(&items[i1], &items[i2], i1, i2, cx.tcx())); + match cx.shared.module_sorting { + ModuleSorting::Alphabetical => { + indices.sort_by(|&i1, &i2| cmp(&items[i1], &items[i2], i1, i2, cx.tcx())); + } + ModuleSorting::DeclarationOrder => {} } // This call is to remove re-export duplicates in cases such as: //