Skip to content

Use tables to summarize course content #2005

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 19, 2024
Merged

Conversation

djmitche
Copy link
Collaborator

This is more friendly to translation (as it can share the translation of the title).

This fixes #1982.

@djmitche djmitche requested a review from mgeisler April 18, 2024 19:46
This is more friendly to translation (as it can share the translation of
the title).
Copy link
Collaborator

@mgeisler mgeisler left a comment

Choose a reason for hiding this comment

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

Thanks @djmitche!

pub fn outline(&self, at_source_path: impl AsRef<Path>) -> String {
let mut outline = String::from("In this segment:\n");
pub fn outline(&self) -> String {
let mut slides = Table::new(["Slide".into(), "Time".into()]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be slightly more accurate to call this a "duration"?

Suggested change
let mut slides = Table::new(["Slide".into(), "Time".into()]);
let mut slides = Table::new(["Slide".into(), "Duration".into()]);

outline
format!(
"Including {BREAK_DURATION} minute breaks, this session should take about {}. It contains:\n\n{}",
duration(self.minutes()),segments)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think your rustfmt has given up here 😄

Suggested change
duration(self.minutes()),segments)
duration(self.minutes()), segments)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it doesn't do much in macro invocations? I was surprised by this too.

pub fn outline(&self, at_source_path: impl AsRef<Path>) -> String {
let mut outline = String::from("In this session:\n");
pub fn outline(&self) -> String {
let mut segments = Table::new(["Segment".into(), "Time".into()]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps "duration" is better here?

Suggested change
let mut segments = Table::new(["Segment".into(), "Time".into()]);
let mut segments = Table::new(["Segment".into(), "Duration".into()]);

session.name,
duration(session.minutes())
)
.unwrap();
let mut segments = Table::new(["Segment".into(), "Time".into()]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let mut segments = Table::new(["Segment".into(), "Time".into()]);
let mut segments = Table::new(["Segment".into(), "Duration".into()]);

@@ -57,6 +58,46 @@ pub fn duration(mut minutes: u64) -> String {
}
}

/// Table implements Display to format a two-dimensional table as markdown,
/// following https://github.github.com/gfm/#tables-extension-.
pub struct Table<const N: usize> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice! If you have time, it would be nice to have a small unit test below to show how the output looks like.

@djmitche djmitche enabled auto-merge (squash) April 19, 2024 13:33
@djmitche djmitche merged commit bb44b1d into google:main Apr 19, 2024
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timing information enters the translations
2 participants