Make fields editable by pool2win · Pull Request #18 · p2poolv2/pdm

pool2win · 2026-03-31T09:51:09Z

A quick PR with claude to make fields editable. Needs proper review and we need to ask claude to add tests.

codecov · 2026-03-31T09:52:39Z

Codecov Report

❌ Patch coverage is 43.68601% with 165 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/components/bitcoin_config_view.rs	47.10%	128 Missing ⚠️
src/config.rs	27.50%	29 Missing ⚠️
src/main.rs	20.00%	8 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copilot

Pull request overview

This PR adds in-app editing for bitcoin.conf entries and introduces a save-to-disk flow for modified configuration values.

Changes:

Adds an editable Bitcoin config UI with navigation, inline value editing, and a save hotkey.
Introduces write_config() to serialize enabled config entries back to bitcoin.conf, grouped by category.
Wires new SaveConfig action and BitcoinConfigView state into the app/event loop.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

File	Description
`src/main.rs`	Routes Bitcoin config key handling to the config view and handles the new `SaveConfig` action.
`src/config.rs`	Adds category display helpers, a fixed category ordering, and a new `write_config()` serializer.
`src/components/bitcoin_config_view.rs`	Replaces the static list with sectioned rendering plus inline editing, cursor, scrolling, and save/back keybindings.
`src/app.rs`	Adds `BitcoinConfigView` to app state and introduces `AppAction::SaveConfig`.

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

Copilot · 2026-03-31T09:58:00Z

            let action = match app.current_screen {
                CurrentScreen::FileExplorer => app.explorer.handle_input(key),

+                CurrentScreen::BitcoinConfig if !app.bitcoin_data.is_empty() => {


CurrentScreen::BitcoinConfig input is only delegated to bitcoin_config_view when !app.bitcoin_data.is_empty(). This makes the screen behavior depend on data presence and can prevent expected keys (e.g. Esc/back or save) from working if the data ever ends up empty (including on parse/load failures or future schema changes). Prefer handling BitcoinConfig unconditionally and let the view decide what to do when there are 0 entries (e.g., allow Esc/back and Enter to open the explorer).

Suggested change

CurrentScreen::BitcoinConfig if !app.bitcoin_data.is_empty() => {

CurrentScreen::BitcoinConfig => {

Copilot · 2026-03-31T09:58:01Z

+        match key.code {
+            KeyCode::Char(c) => {
+                self.edit_buffer.insert(self.edit_cursor_pos, c);
+                self.edit_cursor_pos += 1;
+            }
+            KeyCode::Backspace => {
+                if self.edit_cursor_pos > 0 {
+                    self.edit_cursor_pos -= 1;
+                    self.edit_buffer.remove(self.edit_cursor_pos);
+                }


edit_cursor_pos is documented as a character position, but it’s used as a byte index for String::insert/String::remove. This will panic or corrupt cursor behavior when the buffer contains non-ASCII UTF-8 (cursor can land on a non-char boundary). Track the cursor as a byte offset, or store a character index and convert to a byte offset before slicing/inserting/removing.

Copilot · 2026-03-31T09:58:01Z

+                        let pos = view.edit_cursor_pos;
+                        let before_cursor = &buf[..pos];
+                        let (cursor_char, after_cursor) = if pos < buf.len()
+                        {
+                            let next =
+                                pos + buf[pos..].chars().next().unwrap().len_utf8();
+                            (
+                                buf[pos..next].to_string(),
+                                &buf[next..],


The edit rendering slices buf[..pos] and buf[pos..] where pos comes from edit_cursor_pos. With the current cursor logic this can easily become a non-UTF-8-boundary index and panic at runtime. Consider deriving before/cursor/after using char_indices() (or a helper that maps char index ↔ byte offset) so slicing is always on valid boundaries.

Suggested change

let pos = view.edit_cursor_pos;

let before_cursor = &buf[..pos];

let (cursor_char, after_cursor) = if pos < buf.len()

{

let next =

pos + buf[pos..].chars().next().unwrap().len_utf8();

(

buf[pos..next].to_string(),

&buf[next..],

let char_pos = view.edit_cursor_pos;

// Map character index to byte offsets to ensure valid UTF-8 slicing

let mut cursor_byte = buf.len();

let mut next_byte = buf.len();

let mut found = false;

for (i, (b, ch)) in buf.char_indices().enumerate() {

if i == char_pos {

cursor_byte = b;

next_byte = b + ch.len_utf8();

found = true;

break;

}

}

let before_cursor = &buf[..cursor_byte];

let (cursor_char, after_cursor) = if found {

(

buf[cursor_byte..next_byte].to_string(),

&buf[next_byte..],

Copilot · 2026-03-31T09:58:01Z

+        self.total_entries = entries.len();
+        if self.selected_index >= self.total_entries && self.total_entries > 0 {
+            self.selected_index = self.total_entries - 1;


total_entries is set to entries.len(), but selection/navigation is based on section_layout. If the layout ever omits entries (e.g. a new ConfigCategory not added to CATEGORY_ORDER), selection can point to an entry that isn’t renderable/editable. Consider setting total_entries to the sum of section_layout[*].entry_indices.len() to keep navigation consistent with what’s displayed.

Suggested change

self.total_entries = entries.len();

if self.selected_index >= self.total_entries && self.total_entries > 0 {

self.selected_index = self.total_entries - 1;

// Keep total_entries in sync with what is actually laid out in sections.

self.total_entries = self

.section_layout

.iter()

.map(|section| section.entry_indices.len())

.sum();

if self.selected_index >= self.total_entries {

if self.total_entries == 0 {

self.selected_index = 0;

} else {

self.selected_index = self.total_entries - 1;

}

Copilot · 2026-03-31T09:58:01Z

+    // Write "Other" entries (no schema)
+    let other_entries: Vec<_> = entries
+        .iter()
+        .filter(|e| e.enabled && e.schema.is_none())
+        .collect();
+
+    if !other_entries.is_empty() {
+        if !output.is_empty() {
+            writeln!(output)?;
+        }
+        writeln!(output, "# Other")?;
+        for entry in other_entries {
+            writeln!(output, "{}={}", entry.key, entry.value)?;
+        }
+    }


other_entries are derived from a HashSet (via config_keys) and then written in iteration order. This can make the saved bitcoin.conf nondeterministic between runs, causing noisy diffs and surprising reorderings. Consider sorting other_entries (and potentially per-category entries too) by key before writing.

Copilot · 2026-03-31T09:58:02Z

+            writeln!(output, "# {category}")?;
+            for entry in section_entries {
+                writeln!(output, "{}={}", entry.key, entry.value)?;
+            }
+        }


write_config writes schema entries as key=value without preserving INI section scoping (e.g. values originally under [test] become global). Since the app can now save, this becomes a destructive semantic change for users who rely on per-network sections. Consider either (a) preserving section information in ConfigEntry/parsing so it can be round-tripped, or (b) detecting multi-section configs and refusing to save / warning the user rather than rewriting them incorrectly.

Copilot · 2026-03-31T09:58:02Z

+        }
+    }
+
+    std::fs::write(path, output)


std::fs::write overwrites the config file directly. If the process is interrupted mid-write (crash, power loss), bitcoin.conf can be left truncated/corrupted. Consider writing to a temporary file in the same directory and renaming it (atomic replace) to make saves safer.

Suggested change

std::fs::write(path, output)

let tmp_path = path.with_extension("tmp");

std::fs::write(&tmp_path, &output)?;

std::fs::rename(&tmp_path, path)

Copilot · 2026-03-31T09:58:02Z

+/// Write config entries back to a bitcoin.conf file.
+/// Only enabled entries are written. Entries are grouped by category with comment headers.
+pub fn write_config(path: &Path, entries: &[ConfigEntry]) -> std::io::Result<()> {
+    use std::io::Write;
+


New write_config behavior isn’t covered by tests even though this module has extensive unit tests for parse_config and schema. Adding tests for write_config would help prevent regressions (e.g., category grouping, exclusion of disabled entries, deterministic ordering for "Other", and behavior when sectioned configs are present).

framicheli · 2026-04-02T19:05:25Z

Improved in #19

Make fields editable

b4e8bca

pool2win requested review from Copilot and framicheli March 31, 2026 09:51

Copilot started reviewing on behalf of pool2win March 31, 2026 09:52 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make fields editable#18

Make fields editable#18
pool2win wants to merge 1 commit into
mainfrom
make-fields-editable

pool2win commented Mar 31, 2026

Uh oh!

codecov Bot commented Mar 31, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

framicheli commented Apr 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	CurrentScreen::BitcoinConfig if !app.bitcoin_data.is_empty() => {
	CurrentScreen::BitcoinConfig => {

-                        let pos = view.edit_cursor_pos;
-                        let before_cursor = &buf[..pos];
-                        let (cursor_char, after_cursor) = if pos < buf.len()
-                        {
-                            let next =
-                                pos + buf[pos..].chars().next().unwrap().len_utf8();
-                            (
-                                buf[pos..next].to_string(),
-                                &buf[next..],
+                        let char_pos = view.edit_cursor_pos;
+                        // Map character index to byte offsets to ensure valid UTF-8 slicing
+                        let mut cursor_byte = buf.len();
+                        let mut next_byte = buf.len();
+                        let mut found = false;
+                        for (i, (b, ch)) in buf.char_indices().enumerate() {
+                            if i == char_pos {
+                                cursor_byte = b;
+                                next_byte = b + ch.len_utf8();
+                                found = true;
+                                break;
+                            }
+                        }
+                        let before_cursor = &buf[..cursor_byte];
+                        let (cursor_char, after_cursor) = if found {
+                            (
+                                buf[cursor_byte..next_byte].to_string(),
+                                &buf[next_byte..],

-        self.total_entries = entries.len();
-        if self.selected_index >= self.total_entries && self.total_entries > 0 {
-            self.selected_index = self.total_entries - 1;
+        // Keep total_entries in sync with what is actually laid out in sections.
+        self.total_entries = self
+            .section_layout
+            .iter()
+            .map(|section| section.entry_indices.len())
+            .sum();
+        if self.selected_index >= self.total_entries {
+            if self.total_entries == 0 {
+                self.selected_index = 0;
+            } else {
+                self.selected_index = self.total_entries - 1;
+            }

-    std::fs::write(path, output)
+    let tmp_path = path.with_extension("tmp");
+    std::fs::write(&tmp_path, &output)?;
+    std::fs::rename(&tmp_path, path)

Conversation

pool2win commented Mar 31, 2026

Uh oh!

codecov Bot commented Mar 31, 2026

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

framicheli commented Apr 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants