Skip to content

Commit

Permalink
Format all Markdown files with dprint (google#1157)
Browse files Browse the repository at this point in the history
This is the result of running `dprint fmt` after removing `src/` from
the list of excluded directories.

This also reformats the Rust code: we might want to tweak this a bit in
the future since some of the changes removes the hand-formatting. Of
course, this formatting can be seen as a mis-feature, so maybe this is
good overall.

Thanks to mdbook-i18n-helpers 0.2, the POT file is nearly unchanged
after this, meaning that all existing translations remain valid! A few
messages were changed because of stray whitespace characters:

     msgid ""
     "Slices always borrow from another object. In this example, `a` has to remain "
    -"'alive' (in scope) for at least as long as our slice. "
    +"'alive' (in scope) for at least as long as our slice."
     msgstr ""

The formatting is enforced in CI and we will have to see how annoying
this is in practice for the many contributors. If it becomes annoying,
we should look into fixing dprint/check#11 so that `dprint` can annotate
the lines that need fixing directly, then I think we can consider more
strict formatting checks.

I added more customization to `rustfmt.toml`. This is to better emulate
the dense style used in the course:

- `max_width = 85` allows lines to take up the full width available in
our code blocks (when taking margins and the line numbers into account).
- `wrap_comments = true` ensures that we don't show very long comments
in the code examples. I edited some comments to shorten them and avoid
unnecessary line breaks — please trim other unnecessarily long comments
when you see them! Remember we're writing code for slides 😄
- `use_small_heuristics = "Max"` allows for things like struct literals
and if-statements to take up the full line width configured above.

The formatting settings apply to all our Rust code right now — I think
we could improve this with dprint/dprint#711
which lets us add per-directory `dprint` configuration files. However,
the `inherit: true` setting is not yet implemented (as far as I can
tell), so a nested configuration file will have to copy most or all of
the top-level file.
  • Loading branch information
mgeisler committed Dec 30, 2023
1 parent f43e72e commit c9f66fd
Show file tree
Hide file tree
Showing 302 changed files with 3,093 additions and 2,648 deletions.
1 change: 0 additions & 1 deletion dprint.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
},
"excludes": [
"/book/",
"/src/",
"/theme/*.hbs",
"/third_party/",
"target/"
Expand Down
107 changes: 53 additions & 54 deletions mdbook-course/src/course.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,18 @@
//! Slide -- a single topic (may be represented by multiple mdBook chapters)
//! ```
//!
//! This structure is parsed from the format of the book using a combination of the order in which
//! chapters are listed in `SUMMARY.md` and annotations in the frontmatter of each chapter.
//! This structure is parsed from the format of the book using a combination of
//! the order in which chapters are listed in `SUMMARY.md` and annotations in
//! the frontmatter of each chapter.
//!
//! A book contains a sequence of BookItems, each of which can contain sub-items. A top-level item
//! can potentially introduce a new course, session, segment, and slide all in the same item. If
//! the item has a `course` property in its frontmatter, then it introduces a new course. If it has
//! a `session` property, then it introduces a new session. A top-level item always corresponds
//! 1-to-1 with a segment (as long as it is a chapter), and that item becomes the first slide in
//! that segment. Any other sub-items of the top-level item are treated as further slides in the
//! same segment.
//! A book contains a sequence of BookItems, each of which can contain
//! sub-items. A top-level item can potentially introduce a new course, session,
//! segment, and slide all in the same item. If the item has a `course` property
//! in its frontmatter, then it introduces a new course. If it has a `session`
//! property, then it introduces a new session. A top-level item always
//! corresponds 1-to-1 with a segment (as long as it is a chapter), and that
//! item becomes the first slide in that segment. Any other sub-items of the
//! top-level item are treated as further slides in the same segment.

use crate::frontmatter::{split_frontmatter, Frontmatter};
use crate::markdown::{duration, relative_link};
Expand All @@ -53,19 +55,19 @@ pub struct Courses {

/// A Course is the level of content at which students enroll.
///
/// Courses are identified by the `course` property in a session's frontmatter. All
/// sessions with the same value for `course` are grouped into a Course.
/// Courses are identified by the `course` property in a session's frontmatter.
/// All sessions with the same value for `course` are grouped into a Course.
#[derive(Default, Debug)]
pub struct Course {
pub name: String,
pub sessions: Vec<Session>,
}

/// A Session is a block of instructional time, containing segments. Typically a full day of
/// instruction contains two sessions: morning and afternoon.
/// A Session is a block of instructional time, containing segments. Typically a
/// full day of instruction contains two sessions: morning and afternoon.
///
/// A session is identified by the `session` property in the session's frontmatter. There can be
/// only one session with a given name in a course.
/// A session is identified by the `session` property in the session's
/// frontmatter. There can be only one session with a given name in a course.
#[derive(Default, Debug)]
pub struct Session {
pub name: String,
Expand Down Expand Up @@ -95,8 +97,8 @@ pub struct Slide {
}

impl Courses {
/// Extract the course structure from the book. As a side-effect, the frontmatter is stripped
/// from each slide.
/// Extract the course structure from the book. As a side-effect, the
/// frontmatter is stripped from each slide.
pub fn extract_structure(mut book: Book) -> anyhow::Result<(Self, Book)> {
let mut courses = Courses::default();
let mut current_course_name = None;
Expand All @@ -111,7 +113,8 @@ impl Courses {
let (frontmatter, content) = split_frontmatter(chapter)?;
chapter.content = content;

// If 'course' is given, use that course (if not 'none') and reset the session.
// If 'course' is given, use that course (if not 'none') and reset the
// session.
if let Some(course_name) = &frontmatter.course {
current_session_name = None;
if course_name == "none" {
Expand All @@ -133,7 +136,8 @@ impl Courses {
);
}

// If we have a course and session, then add this chapter to it as a segment.
// If we have a course and session, then add this chapter to it as a
// segment.
if let (Some(course_name), Some(session_name)) =
(&current_course_name, &current_session_name)
{
Expand All @@ -145,7 +149,8 @@ impl Courses {
Ok((courses, book))
}

/// Get a reference to a course, adding a new one if none by this name exists.
/// Get a reference to a course, adding a new one if none by this name
/// exists.
fn course_mut(&mut self, name: impl AsRef<str>) -> &mut Course {
let name = name.as_ref();
if let Some(found_idx) =
Expand All @@ -164,8 +169,8 @@ impl Courses {
self.courses.iter().find(|c| c.name == name)
}

/// Find the slide generated from the given Chapter within these courses, returning the "path"
/// to that slide.
/// Find the slide generated from the given Chapter within these courses,
/// returning the "path" to that slide.
pub fn find_slide(
&self,
chapter: &Chapter,
Expand Down Expand Up @@ -201,19 +206,15 @@ impl<'a> IntoIterator for &'a Courses {

impl Course {
fn new(name: impl Into<String>) -> Self {
Course {
name: name.into(),
..Default::default()
}
Course { name: name.into(), ..Default::default() }
}

/// Get a reference to a session, adding a new one if none by this name exists.
/// Get a reference to a session, adding a new one if none by this name
/// exists.
fn session_mut(&mut self, name: impl AsRef<str>) -> &mut Session {
let name = name.as_ref();
if let Some(found_idx) = self
.sessions
.iter()
.position(|session| &session.name == name)
if let Some(found_idx) =
self.sessions.iter().position(|session| &session.name == name)
{
return &mut self.sessions[found_idx];
}
Expand All @@ -222,15 +223,17 @@ impl Course {
self.sessions.last_mut().unwrap()
}

/// Return the total duration of this course, as the sum of all segment durations.
/// Return the total duration of this course, as the sum of all segment
/// durations.
///
/// This includes breaks between segments, but does not count time between between
/// sessions.
/// This includes breaks between segments, but does not count time between
/// between sessions.
pub fn minutes(&self) -> u64 {
self.into_iter().map(|s| s.minutes()).sum()
}

/// Generate a Markdown schedule for this course, for placement at the given path.
/// Generate a Markdown schedule for this course, for placement at the given
/// path.
pub fn schedule(&self, at_source_path: impl AsRef<Path>) -> String {
let mut outline = String::from("Course schedule:\n");
for session in self {
Expand All @@ -249,7 +252,10 @@ impl Course {
&mut outline,
" * [{}]({}) ({})",
segment.name,
relative_link(&at_source_path, &segment.slides[0].source_paths[0]),
relative_link(
&at_source_path,
&segment.slides[0].source_paths[0]
),
duration(segment.minutes())
)
.unwrap();
Expand All @@ -270,10 +276,7 @@ impl<'a> IntoIterator for &'a Course {

impl Session {
fn new(name: impl Into<String>) -> Self {
Session {
name: name.into(),
..Default::default()
}
Session { name: name.into(), ..Default::default() }
}

/// Add a new segment to the session, representing sub-items as slides.
Expand All @@ -297,7 +300,8 @@ impl Session {
Ok(())
}

/// Generate a Markdown outline for this session, for placement at the given path.
/// Generate a Markdown outline for this session, for placement at the given
/// path.
pub fn outline(&self, at_source_path: impl AsRef<Path>) -> String {
let mut outline = String::from("In this session:\n");
for segment in self {
Expand All @@ -324,7 +328,8 @@ impl Session {
if instructional_time == 0 {
return instructional_time;
}
let breaks = (self.into_iter().filter(|s| s.minutes() > 0).count() - 1) as u64
let breaks = (self.into_iter().filter(|s| s.minutes() > 0).count() - 1)
as u64
* BREAK_DURATION;
instructional_time + breaks
}
Expand All @@ -341,14 +346,11 @@ impl<'a> IntoIterator for &'a Session {

impl Segment {
fn new(name: impl Into<String>) -> Self {
Segment {
name: name.into(),
..Default::default()
}
Segment { name: name.into(), ..Default::default() }
}

/// Create a slide from a chapter. If `recurse` is true, sub-items of this chapter are
/// included in this slide as well.
/// Create a slide from a chapter. If `recurse` is true, sub-items of this
/// chapter are included in this slide as well.
fn add_slide(
&mut self,
frontmatter: Frontmatter,
Expand All @@ -364,8 +366,8 @@ impl Segment {
Ok(())
}

/// Return the total duration of this segment (the sum of the durations of the enclosed
/// slides).
/// Return the total duration of this segment (the sum of the durations of
/// the enclosed slides).
pub fn minutes(&self) -> u64 {
self.into_iter().map(|s| s.minutes()).sum()
}
Expand Down Expand Up @@ -406,10 +408,7 @@ impl<'a> IntoIterator for &'a Segment {

impl Slide {
fn new(frontmatter: Frontmatter, chapter: &Chapter) -> Self {
let mut slide = Self {
name: chapter.name.clone(),
..Default::default()
};
let mut slide = Self { name: chapter.name.clone(), ..Default::default() };
slide.add_frontmatter(&frontmatter);
slide.push_source_path(&chapter.source_path);
slide
Expand Down
8 changes: 5 additions & 3 deletions mdbook-course/src/frontmatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@ pub struct Frontmatter {
}

/// Split a chapter's contents into frontmatter and the remaining contents.
pub fn split_frontmatter(chapter: &Chapter) -> anyhow::Result<(Frontmatter, String)> {
pub fn split_frontmatter(
chapter: &Chapter,
) -> anyhow::Result<(Frontmatter, String)> {
if let Some((frontmatter, content)) = matter(&chapter.content) {
let frontmatter: Frontmatter =
serde_yaml::from_str(&frontmatter).with_context(|| {
let frontmatter: Frontmatter = serde_yaml::from_str(&frontmatter)
.with_context(|| {
format!("error parsing frontmatter in {:?}", chapter.source_path)
})?;

Expand Down
19 changes: 8 additions & 11 deletions mdbook-course/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ fn main() {
pretty_env_logger::init();
let app = Command::new("mdbook-course")
.about("mdbook preprocessor for Comprehensive Rust")
.subcommand(Command::new("supports").arg(Arg::new("renderer").required(true)));
.subcommand(
Command::new("supports").arg(Arg::new("renderer").required(true)),
);
let matches = app.get_matches();

if let Some(_) = matches.subcommand_matches("supports") {
Expand Down Expand Up @@ -65,18 +67,11 @@ fn timediff(actual: u64, target: u64) -> String {
}

fn print_summary(fundamentals: &Course) {
eprintln!(
"Fundamentals: {}",
timediff(fundamentals.minutes(), 8 * 3 * 60)
);
eprintln!("Fundamentals: {}", timediff(fundamentals.minutes(), 8 * 3 * 60));

eprintln!("Sessions:");
for session in fundamentals {
eprintln!(
" {}: {}",
session.name,
timediff(session.minutes(), 3 * 60)
);
eprintln!(" {}: {}", session.name, timediff(session.minutes(), 3 * 60));
for segment in session {
eprintln!(" {}: {}", segment.name, duration(segment.minutes()));
}
Expand All @@ -95,7 +90,9 @@ fn preprocess() -> anyhow::Result<()> {

book.for_each_mut(|chapter| {
if let BookItem::Chapter(chapter) = chapter {
if let Some((course, session, segment, slide)) = courses.find_slide(chapter) {
if let Some((course, session, segment, slide)) =
courses.find_slide(chapter)
{
timing_info::insert_timing_info(slide, chapter);
replacements::replace(
&courses,
Expand Down
14 changes: 10 additions & 4 deletions mdbook-course/src/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@

use std::path::Path;

/// Given a source_path for the markdown file being rendered and a source_path for the target,
/// generate a relative link.
/// Given a source_path for the markdown file being rendered and a source_path
/// for the target, generate a relative link.
pub fn relative_link(
doc_path: impl AsRef<Path>,
target_path: impl AsRef<Path>,
Expand Down Expand Up @@ -72,15 +72,21 @@ mod test {
#[test]
fn relative_link_subdir() {
assert_eq!(
relative_link(Path::new("hello-world.md"), Path::new("hello-world/foo.md")),
relative_link(
Path::new("hello-world.md"),
Path::new("hello-world/foo.md")
),
"./hello-world/foo.md".to_string()
);
}

#[test]
fn relative_link_parent_dir() {
assert_eq!(
relative_link(Path::new("references/foo.md"), Path::new("hello-world.md")),
relative_link(
Path::new("references/foo.md"),
Path::new("hello-world.md")
),
"../hello-world.md".to_string()
);
}
Expand Down
6 changes: 1 addition & 5 deletions mdbook-course/src/timing_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,7 @@ pub fn insert_timing_info(slide: &Slide, chapter: &mut Chapter) {
{
// Include the minutes in the speaker notes.
let minutes = slide.minutes;
let plural = if slide.minutes == 1 {
"minute"
} else {
"minutes"
};
let plural = if slide.minutes == 1 { "minute" } else { "minutes" };
let mut subslides = "";
if slide.source_paths.len() > 1 {
subslides = "and its sub-slides ";
Expand Down
8 changes: 4 additions & 4 deletions mdbook-exerciser/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ const FILENAME_END: &str = " -->";
pub fn process(output_directory: &Path, input_contents: &str) -> anyhow::Result<()> {
let parser = Parser::new(input_contents);

// Find a specially-formatted comment followed by a code block, and then call `write_output`
// with the contents of the code block, to write to a file named by the comment. Code blocks
// without matching comments will be ignored, as will comments which are not followed by a code
// block.
// Find a specially-formatted comment followed by a code block, and then call
// `write_output` with the contents of the code block, to write to a file
// named by the comment. Code blocks without matching comments will be
// ignored, as will comments which are not followed by a code block.
let mut next_filename: Option<String> = None;
let mut current_file: Option<File> = None;
for event in parser {
Expand Down
8 changes: 5 additions & 3 deletions mdbook-exerciser/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ fn main() -> anyhow::Result<()> {
let output_directory = Path::new(
config
.get("output-directory")
.context("Missing output.exerciser.output-directory configuration value")?
.context(
"Missing output.exerciser.output-directory configuration value",
)?
.as_str()
.context("Expected a string for output.exerciser.output-directory")?,
);
Expand All @@ -55,8 +57,8 @@ fn process_all(book: &Book, output_directory: &Path) -> anyhow::Result<()> {
if let BookItem::Chapter(chapter) = item {
trace!("Chapter {:?} / {:?}", chapter.path, chapter.source_path);
if let Some(chapter_path) = &chapter.path {
// Put the exercises in a subdirectory named after the chapter file, without its
// parent directories.
// Put the exercises in a subdirectory named after the chapter file,
// without its parent directories.
let chapter_output_directory =
output_directory.join(chapter_path.file_stem().with_context(
|| format!("Chapter {:?} has no file stem", chapter_path),
Expand Down
2 changes: 1 addition & 1 deletion po/pt-BR.po
Original file line number Diff line number Diff line change
Expand Up @@ -10081,7 +10081,7 @@ msgstr "\"Falha ao ler\""

#: src/error-handling/error-contexts.md:19
msgid "\"Found no username in {path}\""
msgstr "\"Nome de usuário não encontrado em {caminho}\""
msgstr "\"Nome de usuário não encontrado em {path}\""

#: src/error-handling/error-contexts.md:28
msgid "\"Error: {err:?}\""
Expand Down
Loading

0 comments on commit c9f66fd

Please sign in to comment.