feat(tui): allow modifying column visibility and order in settings

[fujiapple852]

User description

Closes #1026

---

Type

enhancement

---

Description

• Introduced enhanced column management in TUI settings, allowing users to toggle column visibility and reorder columns.
• Implemented key bindings for new column management features.
• Updated settings and table rendering to support dynamic column visibility and ordering.

---

Changes walkthrough

Relevant files

Enhancement

frontend.rs Implement Column Visibility and Order Modification in TUI Settings src/frontend.rs Added key bindings for toggling column visibility and moving columns up and down in settings. +6/-0      columns.rs Enhance Column Management with Visibility and Ordering Features src/frontend/columns.rs Introduced Column struct with typ and status fields for better column management. Added methods for toggling column visibility and reordering columns. Implemented From for Columns to include both enabled and disabled columns. Modified column display logic to only show columns marked as shown. +202/-99 settings.rs Extend Settings Rendering to Include Column Visibility and Order src/frontend/render/settings.rs Added rendering for new settings related to column visibility and order. Implemented formatting for column settings, including toggle and move actions. Updated settings formatting to include new column settings section. +44/-14  table.rs Update Table Rendering to Support Column Visibility            src/frontend/render/table.rs Updated table rendering to use new Column struct and visibility status. Adjusted cell rendering logic to match updated column management. +24/-25  tui_app.rs Add Column Visibility and Order Modification Methods to TUI App src/frontend/tui_app.rs Added methods for toggling column visibility and moving columns in the TUI app. Updated navigation logic in settings to account for dynamic column settings. +42/-3

---

✨ PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[github-actions]

PR Description updated to latest commit (4ea86b0)

[github-actions]

PR Review

(Review updated until commit aca8f24)

🧪 Relevant tests	No
🔍 Possible issues	Index Manipulation: The methods move_down and move_up in src/frontend/columns.rs do not perform bounds checking before manipulating the index. This could lead to panic if the index is out of bounds.
	Inconsistent Naming: The naming convention for methods related to column manipulation (e.g., toggle, move_down, move_up) in src/frontend/columns.rs is inconsistent with Rust's naming conventions, which prefer snake_case over camelCase.
🔒 Security concerns	No

Code feedback:

---

relevant file	src/frontend/columns.rs
suggestion	Consider adding bounds checking in the move_down and move_up methods to prevent potential panics due to index out of bounds errors. This can be achieved by checking if the index is within the valid range before performing the removal and insertion operations. [important]
relevant line	let removed = self.0.remove(index);

---

relevant file	src/frontend/columns.rs
suggestion	Rename methods toggle, move_down, and move_up to toggle_column, move_column_down, and move_column_up respectively, to adhere to Rust's snake_case naming convention and improve code readability. [medium]
relevant line	pub fn toggle(&mut self, index: usize) {

---

relevant file	src/frontend/tui_app.rs
suggestion	To improve code maintainability and readability, consider refactoring the repeated logic for determining the count of settings items in next_settings_item and previous_settings_item into a separate method. This method can return the count based on the current selected settings tab. [medium]
relevant line	let count = if self.settings_tab_selected == SETTINGS_TAB_COLUMNS {

---

relevant file	src/frontend/render/settings.rs
suggestion	For better code organization and readability, consider moving the logic related to formatting column settings (currently in format_columns_settings) into the Columns struct as a method. This keeps all column-related logic in one place and makes the format_all_settings function cleaner. [medium]
relevant line	fn format_columns_settings(app: &TuiApp) -> Vec {

---

---

✨ Review tool usage guide:

---

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[github-actions]

PR Code Suggestions

Category	Suggestions
Bug	Add bounds check in move_down method to prevent out-of-bounds panic. Consider handling the case where index + 1 exceeds the bounds of self.0 in move_down method to avoid panics. src/frontend.rs [59-60] -let removed = self.0.remove(index); -self.0.insert(index + 1, removed); +if index < self.0.len() - 1 { + let removed = self.0.remove(index); + self.0.insert(index + 1, removed); +}
	Add bounds check in move_up method to prevent underflow panic. Consider handling the case where index == 0 in move_up method to avoid panics due to underflow. src/frontend.rs [63-64] -let removed = self.0.remove(index); -self.0.insert(index - 1, removed); +if index > 0 { + let removed = self.0.remove(index); + self.0.insert(index - 1, removed); +}
Enhancement	Use enumerate and find for column operations to improve readability and safety. Use enumerate and find to directly access the index and column in toggle, move_down, and move_up methods by column type or other identifiers instead of using index. src/frontend.rs [50-55] -if self.0[index].status == ColumnStatus::Shown { - self.0[index].status = ColumnStatus::Hidden; -} else { - self.0[index].status = ColumnStatus::Shown; +if let Some((index, column)) = self.0.iter_mut().enumerate().find(|(_, c)| c.typ == ColumnType::YourColumnType) { + if column.status == ColumnStatus::Shown { + column.status = ColumnStatus::Hidden; + } else { + column.status = ColumnStatus::Shown; + } }
	Add error handling for index out of bounds in column move operations. Implement error handling for move_down and move_up methods to notify when the move operation cannot be performed, such as when the index is out of bounds. src/frontend.rs [59-60] +if index >= self.0.len() { + return Err("Index out of bounds"); +} let removed = self.0.remove(index); self.0.insert(index + 1, removed); +Ok(())
	Refactor toggle method to use match statement for clarity and performance. For better performance and readability, consider refactoring the toggle method to use a match statement instead of if-else. src/frontend.rs [50-55] -if self.0[index].status == ColumnStatus::Shown { - self.0[index].status = ColumnStatus::Hidden; -} else { - self.0[index].status = ColumnStatus::Shown; -} +self.0[index].status = match self.0[index].status { + ColumnStatus::Shown => ColumnStatus::Hidden, + ColumnStatus::Hidden => ColumnStatus::Shown, +};
Maintainability	Use more descriptive variable names. Consider using a more descriptive variable name than c in the lambda function for better code readability. Descriptive names make the code easier to understand and maintain. src/frontend/render/settings.rs [438] -.map(|c| SettingsItem::new(c.typ.to_string(), c.status.to_string())) +.map(|column| SettingsItem::new(column.typ.to_string(), column.status.to_string()))
	Encapsulate repeated condition checks into a method. The repeated if self.settings_tab_selected == 6 checks in toggle_column_visibility, move_column_down, and move_column_up methods suggest that this logic could be encapsulated within a separate method or structure handling column operations, improving code organization and reducing duplication. src/frontend/tui_app.rs [288] -if self.settings_tab_selected == 6 { +if self.is_columns_tab_selected() {
Best practice	Implement the Display trait for enum formatting. Instead of calling to_string() directly on the ColumnType enum, consider implementing the Display trait for ColumnType. This approach is more idiomatic in Rust and allows for more flexibility in formatting. src/frontend/render/table.rs [77] -Cell::from(c.typ.to_string()).style(Style::default().fg(theme.hops_table_header_text_color)) +Cell::from(c.typ).style(Style::default().fg(theme.hops_table_header_text_color))
	Use direct pattern matching in function parameters. For better code readability and maintainability, consider using pattern matching for the ColumnType enum directly in the new_cell function parameters instead of using a separate variable. This makes the function signature clearer and the match statement more concise. src/frontend/render/table.rs [131] fn new_cell( - column: ColumnType, + column: &ColumnType,
Performance	Use Vec::with_capacity for known sizes. To improve performance, consider using with_capacity when creating a new Vec with a known size to avoid reallocations. src/frontend/render/settings.rs [72-74] -Row::new(vec![ +Row::new(Vec::with_capacity(2)[

---

✨ Improve tool usage guide:

---

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.
When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

Enabling\disabling automation When you first install the app, the default mode for the improve tool is: pr_commands = ["/improve --pr_code_suggestions.summarize=true", ...] meaning the improve tool will run automatically on every PR, with summarization enabled. Delete this line to disable the tool from running automatically.

Utilizing extra instructions Extra instructions are very important for the improve tool, since they enable to guide the model to suggestions that are more relevant to the specific needs of the project. Be specific, clear, and concise in the instructions. With extra instructions, you are the prompter. Specify relevant aspects that you want the model to focus on. Examples for extra instructions: [pr_code_suggestions] # /improve # extra_instructions=""" Emphasize the following aspects: - Does the code logic cover relevant edge cases? - Is the code logic clear and easy to understand? - Is the code logic efficient? ... """ Use triple quotes to write multi-line instructions. Use bullet points to make the instructions more readable.

A note on code suggestions quality While the current AI for code is getting better and better (GPT-4), it's not flawless. Not all the suggestions will be perfect, and a user should not accept all of them automatically. Suggestions are not meant to be simplistic. Instead, they aim to give deep feedback and raise questions, ideas and thoughts to the user, who can then use his judgment, experience, and understanding of the code base. Recommended to use the 'extra_instructions' field to guide the model to suggestions that are more relevant to the specific needs of the project, or use the custom suggestions 💎 tool With large PRs, best quality will be obtained by using 'improve --extended' mode.

More PR-Agent commands To invoke the PR-Agent, add a comment using one of the following commands: /review: Request a review of your Pull Request. /describe: Update the PR title and description based on the contents of the PR. /improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback. /ask <QUESTION>: Ask a question about the PR. /update_changelog: Update the changelog based on the PR's contents. /add_docs 💎: Generate docstring for new components introduced in the PR. /generate_labels 💎: Generate labels for the PR based on the PR's contents. /analyze 💎: Automatically analyzes the PR, and presents changes walkthrough for each component. See the tools guide for more details. To list the possible configuration parameters, add a /config comment.

See the improve usage page for a more comprehensive guide on using this tool.

[github-actions]

PR Description updated to latest commit (aca8f24)

[github-actions]

Persistent review updated to latest commit aca8f24

[github-actions]

PR Code Suggestions

Category	Suggestions
Possible issue	Add bounds checking to prevent potential out-of-bounds access. To prevent potential out-of-bounds access which could cause a panic, it's recommended to check if the index is within bounds before attempting to remove or insert elements in the Vec. src/frontend/columns.rs [64-65] -let removed = self.0.remove(index); -self.0.insert(index + 1, removed); +if index < self.0.len() { + let removed = self.0.remove(index); + self.0.insert(index + 1, removed); +}
	Add bounds checking to prevent potential out-of-bounds access when moving a column up. To avoid potential panics from out-of-bounds access when moving a column up, ensure the index is greater than 0 before removing and inserting. src/frontend/columns.rs [68-70] -let removed = self.0.remove(index); -self.0.insert(index - 1, removed); +if index > 0 { + let removed = self.0.remove(index); + self.0.insert(index - 1, removed); +}
Enhancement	Use a match statement for toggling column status to improve readability. To enhance code readability and maintainability, consider using a match statement instead of an if-else chain for toggling the column status. src/frontend/columns.rs [56-60] -if self.0[index].status == ColumnStatus::Shown { - self.0[index].status = ColumnStatus::Hidden; -} else { - self.0[index].status = ColumnStatus::Shown; -} +self.0[index].status = match self.0[index].status { + ColumnStatus::Shown => ColumnStatus::Hidden, + ColumnStatus::Hidden => ColumnStatus::Shown, +};
	Implement error handling for index out of bounds in column movement methods. Consider implementing error handling for the move_down and move_up methods to gracefully handle cases where the index is out of bounds, instead of silently failing. src/frontend/columns.rs [64-65] -let removed = self.0.remove(index); -self.0.insert(index + 1, removed); +if index < self.0.len() { + let removed = self.0.remove(index); + self.0.insert(index + 1, removed); +} else { + // Handle error: index out of bounds +}
	Use iter() instead of map() for simple iteration. Consider using iter() instead of map() for iterating over items without transforming them. This change enhances readability by clearly indicating that the loop's purpose is iteration, not transformation. src/frontend/render/settings.rs [73] -let rows = items.iter().map(|item| { +let rows = items.iter().each(|item| {
Performance	Optimize the Display implementation for Columns by avoiding unnecessary cloning. To improve the efficiency and clarity of the Display implementation for Columns, consider using filter and map directly on the iterator instead of cloning the vector. src/frontend/columns.rs [88-99] let output: Vec<char> = self .0 - .clone() - .into_iter() - .filter_map(|c| { - if c.status == ColumnStatus::Shown { - Some(c.typ.into()) - } else { - None - } - }) + .iter() + .filter_map(|c| if c.status == ColumnStatus::Shown { Some(c.typ.into()) } else { None }) .collect();
Maintainability	Use a struct for grouping related data in format_all_settings return value. To improve code readability and maintainability, consider using a struct or named tuple for grouping item, value, and Vec together in the format_all_settings function return value. src/frontend/render/settings.rs [124] -fn format_all_settings(app: &TuiApp) -> Vec<(&'static str, String, Vec<SettingsItem>)> { +struct SettingsCategory { + name: &'static str, + description: String, + items: Vec<SettingsItem>, +} +fn format_all_settings(app: &TuiApp) -> Vec<SettingsCategory> {
	Abstract cell rendering logic based on column type into a separate function. To enhance code readability and maintainability, consider abstracting the logic for rendering cells based on column type into a separate function. This approach can help in managing changes to rendering logic more efficiently. src/frontend/render/table.rs [139-164] match column { - ColumnType::Ttl => render_usize_cell(hop.ttl().into()), - ColumnType::Host => { - let (host_cell, _) = if is_selected_hop && app.show_hop_details { - render_hostname_with_details(app, hop, dns, geoip_lookup, config) - } else { - render_hostname(app, hop, dns, geoip_lookup) - }; - host_cell - } + _ => render_cell_by_column_type(column, is_selected_hop, app, hop, dns, geoip_lookup, config) +}
	Refactor repeated logic for getting settings items count into a separate method. To avoid repetition and improve code maintainability, consider refactoring the logic for determining the count of settings items into a separate function. This function can then be reused in both next_settings_item and previous_settings_item methods. src/frontend/tui_app.rs [256-259] -let count = if self.settings_tab_selected == SETTINGS_TAB_COLUMNS { - self.tui_config.tui_columns.all_columns_count() -} else { - SETTINGS_TABS[self.settings_tab_selected].1 -}; +let count = self.get_settings_items_count(); +// And define a new method in TuiApp: +fn get_settings_items_count(&self) -> usize { + if self.settings_tab_selected == SETTINGS_TAB_COLUMNS { + self.tui_config.tui_columns.all_columns_count() + } else { + SETTINGS_TABS[self.settings_tab_selected].1 + } +}
Best practice	Ensure consistent data types for SettingsItem fields. For consistency and to avoid potential future errors, consider using the same data type for item in SettingsItem. It's recommended to use String instead of &'static str to allow more flexibility in item values. src/frontend/render/table.rs [471-472] struct SettingsItem { - item: String, + item: String, // This change has already been made, suggesting to ensure consistency in future modifications. value: String, }

---

✨ Improve tool usage guide:

---

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

• When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

• With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.