Better deal with multiple config files (#3774)

* Better deal with multiple config files

* Improve key handling

* Apply suggestion from review

esp-config/src/bin/esp-config/main.rs

@@ -13,8 +13,6 @@ use toml_edit::{DocumentMut, Formatted, Item, Table};
 
 mod tui;
 
-const DEFAULT_CONFIG_PATH: &str = ".cargo/config.toml";
-
 #[derive(Parser, Debug)]
 #[command(author, version, about, long_about = None)]
 struct Args {
@@ -26,7 +24,7 @@ struct Args {
     #[arg(short = 'C', long)]
     chip: Option<esp_metadata::Chip>,
 
-    /// Config file - using `.cargo/config.toml` by default
+    /// Config file - using `config.toml` by default
     #[arg(short = 'c', long)]
     config_file: Option<String>,
 }
@@ -53,8 +51,52 @@ fn main() -> Result<(), Box<dyn Error>> {
 
     let work_dir = args.path.clone().unwrap_or(".".into());
 
-    let config_file_path =
+    let config_file = if args.config_file.is_none() {
-        work_dir.join(args.config_file.as_deref().unwrap_or(DEFAULT_CONFIG_PATH));
+        // if there are multiple config files and none is selected via the command line option
+        // let the user choose (but don't offer the base config.toml)
+
+        let mut config_file = None;
+        let cargo_dir = work_dir.join(".cargo");
+
+        if cargo_dir.exists() {
+            let files: Vec<String> = cargo_dir
+                .read_dir()?
+                .filter_map(|e| e.ok())
+                .filter(|entry| {
+                    entry.path().is_file()
+                        && entry.path().extension().unwrap_or_default() == "toml"
+                        && entry.file_name().to_string_lossy() != "config.toml"
+                })
+                .map(|entry| entry.file_name().to_string_lossy().to_string())
+                .collect();
+
+            if files.len() > 0 {
+                let terminal = tui::init_terminal()?;
+                let mut chooser = tui::ConfigChooser::new(files);
+                config_file = chooser.run(terminal)?;
+                tui::restore_terminal()?;
+            } else {
+                config_file = Some("config.toml".to_string());
+            }
+        }
+
+        config_file
+    } else {
+        Some(
+            args.config_file
+                .as_deref()
+                .unwrap_or("config.toml")
+                .to_string(),
+        )
+    };
+
+    if config_file.is_none() {
+        return Ok(());
+    }
+
+    let config_file = config_file.unwrap();
+
+    let config_file_path = work_dir.join(".cargo").join(config_file);
     if !config_file_path.exists() {
         return Err(format!(
             "Config file {} does not exist or is not readable.",
@@ -63,7 +105,7 @@ fn main() -> Result<(), Box<dyn Error>> {
         .into());
     }
 
-    let configs = parse_configs(&work_dir, args.chip, args.config_file.as_deref())?;
+    let (hint_about_config_toml, configs) = parse_configs(&work_dir, args.chip, &config_file_path)?;
     let initial_configs = configs.clone();
     let previous_config = initial_configs.clone();
 
@@ -73,31 +115,25 @@ fn main() -> Result<(), Box<dyn Error>> {
     let terminal = tui::init_terminal()?;
 
     // create app and run it
-    let updated_cfg = tui::App::new(None, repository).run(terminal)?;
+    let updated_cfg = tui::App::new(if hint_about_config_toml {
+        Some("[env] section in base config.toml detected - avoid this and only add [env] sections to individual configs".to_string()) } else { None }, repository)
+        .run(terminal)?;
 
     tui::restore_terminal()?;
 
     // done with the TUI
     if let Some(updated_cfg) = updated_cfg {
-        apply_config(
+        apply_config(updated_cfg, previous_config, &config_file_path)?;
-            &work_dir,
-            updated_cfg,
-            previous_config,
-            args.config_file.clone(),
-        )?;
     }
 
     Ok(())
 }
 
 fn apply_config(
-    path: &Path,
     updated_cfg: Vec<CrateConfig>,
     previous_cfg: Vec<CrateConfig>,
-    cfg_file: Option<String>,
+    config_toml_path: &PathBuf,
 ) -> Result<(), Box<dyn Error>> {
-    let config_toml_path = path.join(cfg_file.as_deref().unwrap_or(DEFAULT_CONFIG_PATH));
-
     let mut config = std::fs::read_to_string(&config_toml_path)?
         .as_str()
         .parse::<DocumentMut>()?;
@@ -143,9 +179,36 @@ fn apply_config(
 fn parse_configs(
     path: &Path,
     chip_from_args: Option<esp_metadata::Chip>,
-    config_file: Option<&str>,
+    config_toml_path: &PathBuf,
-) -> Result<Vec<CrateConfig>, Box<dyn Error>> {
+) -> Result<(bool, Vec<CrateConfig>), Box<dyn Error>> {
-    let config_toml_path = path.join(config_file.unwrap_or(DEFAULT_CONFIG_PATH));
+    let mut hint_about_configs = false;
+
+    // check if we find multiple potential config files - if yes and if the base `config.toml`
+    // contains an [env] section let the user know this is not ideal
+    if let Some(config_toml_dir) = config_toml_path.parent() {
+        if config_toml_dir
+            .read_dir()?
+            .filter(|entry| {
+                if let Ok(entry) = entry {
+                    entry.path().is_file() && entry.path().extension().unwrap_or_default() == "toml"
+                } else {
+                    false
+                }
+            })
+            .count()
+            > 1
+        {
+            let base_toml = config_toml_dir.join("config.toml");
+            if base_toml.exists() {
+                let base_toml_content = std::fs::read_to_string(base_toml)?;
+                let base_toml = base_toml_content.as_str().parse::<DocumentMut>()?;
+                if base_toml.contains_key("env") {
+                    hint_about_configs = true;
+                }
+            }
+        }
+    }
+
     let config_toml_content = std::fs::read_to_string(config_toml_path)?;
     let config_toml = config_toml_content.as_str().parse::<DocumentMut>()?;
 
@@ -263,5 +326,5 @@ fn parse_configs(
         return Err("No config files found.".into());
     }
 
-    Ok(configs)
+    Ok((hint_about_configs, configs))
 }

esp-config/src/bin/esp-config/tui.rs

@@ -599,7 +599,6 @@ impl Widget for &mut App<'_> {
                 .style(self.colors.edit_invalid_style)
                 .block(
                     Block::bordered()
-                        .title("Validation Error")
                         .style(self.colors.border_error_style)
                         .padding(Padding::uniform(1)),
                 );
@@ -739,3 +738,119 @@ pub(super) fn validate_config(config: &CrateConfig) -> Result<(), String> {
     }
     Ok(())
 }
+
+pub struct ConfigChooser {
+    config_files: Vec<String>,
+    state: ListState,
+    colors: Colors,
+}
+
+impl ConfigChooser {
+    pub fn new(config_files: Vec<String>) -> Self {
+        let colors = match std::env::var("TERM_PROGRAM").as_deref() {
+            Ok("vscode") => Colors::RGB,
+            Ok("Apple_Terminal") => Colors::ANSI,
+            _ => Colors::RGB,
+        };
+
+        let state = ListState::default().with_selected(Some(0));
+
+        Self {
+            config_files,
+            state,
+            colors,
+        }
+    }
+
+    pub fn run(&mut self, mut terminal: Terminal<impl Backend>) -> AppResult<Option<String>> {
+        loop {
+            self.draw(&mut terminal)?;
+
+            if let Event::Key(key) = event::read()? {
+                if key.kind == KeyEventKind::Press {
+                    match key.code {
+                        KeyCode::Char('q') => return Ok(None),
+                        KeyCode::Esc => return Ok(None),
+                        KeyCode::Char('j') | KeyCode::Down => {
+                            self.state.select_next();
+                        }
+                        KeyCode::Char('k') | KeyCode::Up => {
+                            self.state.select_previous();
+                        }
+                        KeyCode::Enter => {
+                            let selected = self.state.selected().unwrap_or_default();
+                            return Ok(Some(self.config_files[selected].clone()));
+                        }
+                        _ => {}
+                    }
+                }
+            }
+        }
+    }
+
+    fn draw(&mut self, terminal: &mut Terminal<impl Backend>) -> AppResult<()> {
+        terminal.draw(|f| {
+            f.render_widget(self, f.area());
+        })?;
+
+        Ok(())
+    }
+}
+
+impl Widget for &mut ConfigChooser {
+    fn render(self, area: Rect, buf: &mut Buffer) {
+        let vertical = Layout::vertical([
+            Constraint::Length(2),
+            Constraint::Fill(1),
+            Constraint::Length(1),
+        ]);
+        let [header_area, rest_area, footer_area] = vertical.areas(area);
+
+        Paragraph::new("esp-config")
+            .bold()
+            .centered()
+            .render(header_area, buf);
+
+        Paragraph::new("Choose a config to edit")
+            .bold()
+            .centered()
+            .render(footer_area, buf);
+
+        // We create two blocks, one is for the header (outer) and the other is for the
+        // list (inner).
+        let outer_block = Block::default()
+            .borders(Borders::NONE)
+            .fg(self.colors.text_color)
+            .bg(self.colors.header_bg)
+            .title_alignment(Alignment::Center);
+        let inner_block = Block::default()
+            .borders(Borders::NONE)
+            .fg(self.colors.text_color)
+            .bg(self.colors.normal_row_color);
+
+        // We get the inner area from outer_block. We'll use this area later to render
+        // the table.
+        let outer_area = rest_area;
+        let inner_area = outer_block.inner(outer_area);
+
+        // We can render the header in outer_area.
+        outer_block.render(outer_area, buf);
+
+        // Iterate through all elements in the `items` and stylize them.
+        let items: Vec<ListItem> = self
+            .config_files
+            .iter()
+            .map(|value| ListItem::new(value.as_str()).style(Style::default()))
+            .collect();
+
+        // We can now render the item list
+        // (look carefully, we are using StatefulWidget's render.)
+        // ratatui::widgets::StatefulWidget::render as stateful_render
+        let state = &mut self.state;
+        let list_widget = List::new(items)
+            .block(inner_block)
+            .highlight_style(self.colors.selected_active_style)
+            .highlight_spacing(HighlightSpacing::Always);
+        StatefulWidget::render(list_widget, inner_area, buf, state);
+    }
+}