restructure the formatting of team asks (#245)

* restructure the formatting of team asks

Break down by goal and have a table with the
kind of asks.

* sort by title, shorten link

* elaborate more in the RFC itself

---------

Co-authored-by: Niko Matsakis <nikomat@amazon.com>

Author: nikomatsakis

Cargo.lock

@@ -8,10 +8,12 @@ use mdbook::book::{Book, Chapter};
 use mdbook::preprocess::{Preprocessor, PreprocessorContext};
 use mdbook::BookItem;
 use regex::{Captures, Regex};
-use rust_project_goals::util::GithubUserInfo;
+use rust_project_goals::config::Configuration;
+use rust_project_goals::format_team_ask::format_team_asks;
+use rust_project_goals::util::{self, GithubUserInfo};
 
 use rust_project_goals::{
-    goal::{self, format_team_asks, GoalDocument, Status, TeamAsk},
+    goal::{self, GoalDocument, Status, TeamAsk},
     re, team,
 };
 
@@ -129,6 +131,7 @@ impl<'c> GoalPreprocessorWithContext<'c> {
             BookItem::Chapter(chapter) => {
                 self.replace_metadata_placeholders(chapter)?;
                 self.replace_team_asks(chapter)?;
+                self.replace_valid_team_asks(chapter)?;
                 self.replace_goal_lists(chapter)?;
                 self.replace_goal_count(chapter)?;
                 self.link_teams(chapter)?;
@@ -261,10 +264,30 @@ impl<'c> GoalPreprocessorWithContext<'c> {
         Ok(())
     }
 
+    /// Look for `<!-- TEAM ASKS -->` in the chapter content and replace it with the team asks.
+    fn replace_valid_team_asks(&mut self, chapter: &mut Chapter) -> anyhow::Result<()> {
+        if !re::VALID_TEAM_ASKS.is_match(&chapter.content) {
+            return Ok(());
+        }
+        let config = Configuration::get();
+        let rows = std::iter::once(vec!["Ask".to_string(), "aka".to_string(), "Description".to_string()])
+            .chain(config.team_asks.iter().map(|(name, details)| {
+                vec![
+                    format!("{name:?}"),
+                    details.short.to_string(),
+                    details.about.to_string(),
+                ]
+            }))
+            .collect::<Vec<Vec<String>>>();
+        let table = util::format_table(&rows);
+        let new_content = re::VALID_TEAM_ASKS.replace_all(&chapter.content, table);
+        chapter.content = new_content.to_string();
+        Ok(())
+    }
+
     /// Find the goal documents for the milestone in which this `chapter_path` resides.
     /// e.g., if invoked with `2024h2/xxx.md`, will find all goal documents in `2024h2`.
     fn goal_documents(&mut self, chapter_path: &Path) -> anyhow::Result<Arc<Vec<GoalDocument>>> {
-
         let Some(milestone_path) = chapter_path.parent() else {
             anyhow::bail!("cannot get goal documents from `{chapter_path:?}`")
         };

crates/mdbook-goals/src/mdbook_preprocessor.rs

@@ -16,3 +16,4 @@ walkdir = "2.5.0"
 rust_team_data = { git = "https://github.com/rust-lang/team" }
 rust-project-goals-json = { version = "0.1.0", path = "../rust-project-goals-json" }
 toml = "0.8.19"
+indexmap = "2.7.1"

crates/rust-project-goals/Cargo.toml

@@ -1,11 +1,23 @@
-use std::{collections::BTreeMap, path::PathBuf};
+use std::path::PathBuf;
 
 use anyhow::Context;
+use indexmap::IndexMap;
 use serde::Deserialize;
 
 #[derive(Deserialize)]
 pub struct Configuration {
-    pub team_asks: BTreeMap<String, String>,
+    /// Defines the valid "asks" of teams. The key is the ask, the value is an extended description.
+    /// IndexMap is used to preserve the ordering as defined in the TOML file.
+    pub team_asks: IndexMap<String, TeamAskDetails>,
+}
+
+#[derive(Deserialize)]
+pub struct TeamAskDetails {
+    /// A short descriptor of the team ask suitable for inclusion in a table
+    pub short: String,
+
+    /// Longer description
+    pub about: String,
 }
 
 impl Configuration {

crates/rust-project-goals/src/config.rs

@@ -0,0 +1,148 @@
+use std::collections::BTreeSet;
+
+use indexmap::IndexMap;
+
+use crate::{config::Configuration, goal::TeamAsk, team::TeamName, util};
+
+/// Format a set of team asks into a table, with asks separated by team and grouped by kind.
+///
+/// output looks like
+///
+/// ```ignore
+/// | Goal      | [DMS](#discussion-and-moral-support) | [SR](#standard reviews) |
+/// | :---      | :--                                  | :--                     |
+/// | Foo       | ✅                                   | ✅ (notes)               |
+/// | Bar (Baz) | ✅                                   | ✅ (\*1)                 |
+///
+/// \*1: ... longer notes that would not fit ...
+/// ```
+pub fn format_team_asks(asks_of_any_team: &[&TeamAsk]) -> anyhow::Result<String> {
+    use std::fmt::Write;
+
+    const CHECK: &str = "✅";
+
+    /// Arbitrary: max length of text before we insert a footnote
+    const FOOTNOTE_LEN: usize = 22;
+
+    let mut output = String::new();
+
+    let all_teams: BTreeSet<&TeamName> = asks_of_any_team
+        .iter()
+        .flat_map(|a| &a.teams)
+        .copied()
+        .collect();
+
+    // The set of configured team asks
+    let config = Configuration::get();
+
+    for team_name in all_teams {
+        let asks_of_this_team: Vec<_> = asks_of_any_team
+            .iter()
+            .filter(|a| a.teams.contains(&team_name))
+            .collect();
+
+        let team_data = team_name.data();
+        write!(output, "\n### {} team\n", team_data.name)?;
+
+        // We will accumulate footnotes when we encounter comments that are too long.
+        let mut footnotes = vec![];
+
+        // These are things like "discussion and moral support". They are extracted from
+        // the configuration. We prune out the ones that do not appear in the asks for a particular team.
+        let ask_headings = config
+            .team_asks
+            .keys()
+            .filter(|&ask_kind| {
+                asks_of_this_team
+                    .iter()
+                    .any(|a| &a.ask_description == ask_kind)
+            })
+            .collect::<Vec<_>>();
+        let empty_row = || {
+            (0..ask_headings.len())
+                .map(|_| "".to_string())
+                .collect::<Vec<_>>()
+        };
+
+        // Collect the asks by goal. The `rows` map goes from goal title to a row with entries
+        let mut goal_rows: IndexMap<String, Vec<String>> = IndexMap::default();
+        for ask in &asks_of_this_team {
+            let link = format!("{}", ask.link_path.display());
+
+            let goal_title = match &ask.goal_titles[..] {
+                [goal_title] => format!("[{goal_title}]({link}#ownership-and-team-asks)"),
+                [goal_title, subgoal_title] => {
+                    format!("[{subgoal_title}]({link}#ownership-and-team-asks) (part of [{goal_title}]({link}))")
+                }
+                _ => anyhow::bail!(
+                    "expected either 1 or 2 goal titles, not {:?}",
+                    ask.goal_titles
+                ),
+            };
+
+            let row = goal_rows.entry(goal_title).or_insert_with(empty_row);
+
+            let index = ask_headings
+                .iter()
+                .position(|&h| h == &ask.ask_description)
+                .unwrap();
+
+            let text = if !ask.notes.is_empty() {
+                &ask.notes
+            } else {
+                CHECK
+            };
+
+            let mut maybe_footnote = |text: &str| -> String {
+                if text.len() > FOOTNOTE_LEN {
+                    let footnote_index = footnotes.len() + 1;
+                    footnotes.push(format!("\\*{footnote_index}: {text} ([from here]({link}))", link = ask.link_path.display()));
+                    format!("\\*{footnote_index}")
+                } else {
+                    text.to_string()
+                }
+            };
+
+            if !row[index].is_empty() {
+                row[index] = format!("{} {}", row[index], maybe_footnote(text));
+            } else {
+                row[index] = maybe_footnote(text);
+            }
+        }
+
+        // Sort the goal rows by name (ignoring case).
+        goal_rows.sort_by_cached_key(|ask_names, _ask_rows| {  
+            ask_names.clone().to_uppercase()
+        });
+
+        // Create the table itself.
+        let table = {
+            let headings = std::iter::once("Goal".to_string())
+                .chain(
+                    ask_headings
+                        .iter()
+                        .map(|&ask_kind| format!(
+                            "[{team_ask_short}][valid_team_asks]", // HACK: This should not be hardcoded in the code.
+                            team_ask_short = config.team_asks[ask_kind].short,
+                        ))
+                ) // e.g. "discussion and moral support"
+                .collect::<Vec<String>>();
+
+            let rows = goal_rows.into_iter().map(|(goal_title, goal_columns)| {
+                std::iter::once(goal_title)
+                    .chain(goal_columns)
+                    .collect::<Vec<String>>()
+            });
+
+            std::iter::once(headings).chain(rows).collect::<Vec<_>>()
+        };
+
+        write!(output, "{}", util::format_table(&table))?;
+
+        for footnote in footnotes {
+            write!(output, "\n\n{}\n", footnote)?;
+        }
+    }
+
+    Ok(output)
+}