Skip to content

Commit

Permalink
Rollup merge of rust-lang#98685 - camelid:sorting-flag, r=GuillaumeGomez
Browse files Browse the repository at this point in the history
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.

r? `@GuillaumeGomez`
  • Loading branch information
matthiaskrgr authored Jun 29, 2022
2 parents c864468 + be0b112 commit 969b57d
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 17 deletions.
23 changes: 15 additions & 8 deletions src/librustdoc/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
/// 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<StylePath>,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -731,7 +738,7 @@ impl Options {
external_html,
id_map,
playground_url,
sort_modules_alphabetically,
module_sorting,
themes,
extension_css,
extern_html_root_urls,
Expand Down
17 changes: 10 additions & 7 deletions src/librustdoc/html/render/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -95,7 +95,7 @@ pub(crate) struct SharedContext<'tcx> {
created_dirs: RefCell<FxHashSet<PathBuf>>,
/// 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<StylePath>,
/// Suffix to be added on resource files (if suffix is "-v2" then "light.css" becomes
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 6 additions & 2 deletions src/librustdoc/html/render/print_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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:
//
Expand Down

0 comments on commit 969b57d

Please sign in to comment.