Skip to content

Add editing p2poolv2 config file#20

Merged
pool2win merged 10 commits into
p2poolv2:mainfrom
R27-pixel:settings
Apr 28, 2026
Merged

Add editing p2poolv2 config file#20
pool2win merged 10 commits into
p2poolv2:mainfrom
R27-pixel:settings

Conversation

@R27-pixel
Copy link
Copy Markdown
Contributor

@R27-pixel R27-pixel commented Apr 11, 2026

Open, edit, save config.toml file

The imported P2poolv2_Config is deeply nested and not suitable for TUI lists.
The adapter flattens it into rows and maps edits back.

Demo

Cargo toml-pdm-VisualStudioCode2026-04-1114-08-47-ezgif com-video-to-gif-converter

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 11, 2026

Codecov Report

❌ Patch coverage is 88.01809% with 159 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/p2poolv2_config.rs 87.62% 76 Missing ⚠️
src/main.rs 80.71% 59 Missing ⚠️
src/components/p2pool_config_view.rs 95.00% 20 Missing ⚠️
src/app.rs 42.85% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

@R27-pixel R27-pixel marked this pull request as draft April 11, 2026 11:48
@R27-pixel R27-pixel marked this pull request as ready for review April 16, 2026 16:50
@pool2win pool2win requested a review from Copilot April 17, 2026 12:22
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds support for opening, editing, and saving a p2poolv2 config.toml from the TUI by introducing a “flattened rows” adapter around the deeply-nested p2poolv2_config::Config, plus a dedicated config view and new app actions to commit edits and persist them back to disk.

Changes:

  • Introduces a p2poolv2_config adapter that flattens Config into editable rows and applies edits back to the nested struct.
  • Updates the TUI to navigate and edit P2Pool config entries, including sensitive-field masking.
  • Adds save logic intended to patch the original TOML in-place (preserving comments/formatting) and expands integration tests around P2Pool config flows.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/p2poolv2_config.rs New adapter layer (flatten_config / apply_edit) + schema metadata + unit tests.
src/components/p2pool_config_view.rs Reworks the P2Pool config screen into a list/detail editor with edit mode and masking.
src/main.rs Wires P2Pool config screen input handling, commit/save actions, and TOML patch-based saving + tests.
src/app.rs Adds new AppActions and stores P2PoolConfigView in app state.
src/lib.rs Exposes p2poolv2_config module publicly.
Cargo.toml / Cargo.lock Adds TOML editing + Bitcoin network parsing deps and updates lockfile accordingly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Cargo.toml Outdated
ratatui = "0.29.0"
p2poolv2_config = { git = "https://github.com/p2poolv2/p2poolv2", package = "p2poolv2_config" }
bitcoin = "0.32.5"
toml = "0.8"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed the unused toml dependency.

Comment thread src/components/p2pool_config_view.rs Outdated
Comment on lines +158 to +161
let panels = Layout::default()
.direction(Direction::Horizontal)
.constraints([Constraint::Percentage(45), Constraint::Percentage(55)])
.split(area);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

updated the config view to render warning and save messages so validation errors and save feedback are visible.

Comment thread src/main.rs
Comment on lines +343 to +348
if let Some(table) = doc.get_mut(&section).and_then(|v| v.as_table_mut()) {
// Only update keys that already exist in the file to avoid
// injecting fields the user intentionally omitted
if table.contains_key(key) {
table[key] = toml_edit::value(entry.value.clone());
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

updated
now instead of always calling toml_edit::value(entry.value.clone()) (which always produces a string), we now call typed_toml_item_like which inspects the existing item's type in the document and parses the new value into that same type.

Comment thread src/main.rs Outdated
Comment on lines +714 to +727
// Needs a real sample config — adjust path to your sample
if let Ok(cfg) = P2PoolConfig::load("../config.sample.toml") {
let mut app = App::new();
app.p2pool_config = Some(cfg);
// Find port index
let entries = flatten_config(app.p2pool_config.as_ref().unwrap());
let port_idx = entries.iter().position(|e| e.key == "port").unwrap();
handle_action(
AppAction::CommitP2PoolEdit(port_idx, "notanumber".to_string()),
&mut app,
)
.unwrap();
assert!(app.p2pool_config_view.warning_message.is_some());
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

updated
now config is written inline so the test is fully hermetic, expect instead of if let Ok so a parse failure is a real test failure

Comment thread src/main.rs Outdated
Comment on lines +960 to +965
if let Ok(cfg) = P2PoolConfig::load(path.to_str().unwrap()) {
let mut app = App::new();
app.p2pool_config = Some(cfg);
app.p2pool_config_view.warning_message = Some("old warning".to_string());

let entries = flatten_config(app.p2pool_config.as_ref().unwrap());
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

replaced all if let ok with expect any future breakage in the fixture TOML will cause a expect panic rather than a silent pass.

Comment thread src/main.rs Outdated
Comment on lines +1066 to +1067
// Point at a directory that doesn't exist so writing fails
app.p2pool_conf_path = Some(std::path::PathBuf::from("/nonexistent/config.toml"));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed!!

Comment thread src/p2poolv2_config.rs Outdated
Comment on lines +888 to +893
fn apply_edit_unknown_field_hits_fallback_branch() {
use super::apply_edit;

let mut cfg = make_config();

// Out of bounds → triggers error path
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated!
refactored the apply_edit slightly to extract the dispatch logic so tests can inject a fake entry directly, and split this into separate tests for index bounds and unknown-field handling.

Comment thread src/main.rs Outdated
Copy link
Copy Markdown
Contributor

@framicheli framicheli Apr 24, 2026

Choose a reason for hiding this comment

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

Pressing "q" while editing a P2PoolV2 config field quits the application. We should add P2PoolConfig here to avoid this behaviour.

let text_input_active =
      (app.current_screen == CurrentScreen::BitcoinConfig
          && !app.bitcoin_config_view.sidebar_focused
          && app.bitcoin_config_view.editing)
      || (app.current_screen == CurrentScreen::P2PoolConfig
          && !app.p2pool_config_view.sidebar_focused
          && app.p2pool_config_view.editing);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated!!

Comment thread src/main.rs
.map_err(|e| anyhow::anyhow!("Failed to parse P2Pool config TOML: {}", e))?;

// Walk every flattened entry and patch the matching TOML key
for entry in flatten_config(cfg) {
Copy link
Copy Markdown
Contributor

@framicheli framicheli Apr 24, 2026

Choose a reason for hiding this comment

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

dial_peers field edits are not saved, flatten_config encodes the field as a CSV string like this n.dial_peers.join(","). When at line 551 typed_toml_item_like is called we get the "Unsupported TOML value type for key: {}" because existing is a TOML array and typed_toml_item_like doesn't handle arrays.
We could fix it adding a new if statement to typed_toml_item_like in order to handle arrays: if let Some(arr) = existing.as_array()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

resolved!

Comment thread src/main.rs Outdated
}
}
app.current_screen = CurrentScreen::P2PoolConfig;
app.settings.p2pool_conf_path = Some(path.clone());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These lines run even if the config fails to load. On next startup bootstrap_from_settings tries to load this path, fails and prints an eprintln!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks! updated!!

Comment thread src/main.rs Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CurrentScreen::P2PoolConfig has its own explicit arm, no need here.

_ => sidebar_nav(key.code, app),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated!!

Comment thread src/app.rs
}
}

// Logic to switch between sidebar items
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should add also P2PoolConfig here, like it's done for BitcoinConfig, in order to cleanup messages and editing state when navigating away from it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks!! Updated !!

@pool2win pool2win merged commit ad1966b into p2poolv2:main Apr 28, 2026
3 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.

4 participants