Skip to content

Commit f6eb832

Browse files
joskeclaude
andauthored
Refactor: extract shared helpers, deduplicate views (#35)
* Quick-win refactors: extract helpers, fix grammar, use Side enum - Extract draw_gutter_band helper to deduplicate cairo band drawing - Extract is_binary helper in file I/O code - Extract find_window helper replacing inline root/downcast pattern - Extract git_show helper to deduplicate head_content/stage_content - Replace is_left:bool with Side enum in chunk_map/diff_state - Fix "1 changes" grammar bug → "1 change" Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Wire nav buttons to GActions, fix merge toolbar label jitter - Replace duplicated button connect_clicked handlers with thin wrappers that delegate to the corresponding GActions (-272 lines) - Add grab_focus() to action handlers so keyboard focus follows navigation (clone-first pattern to avoid RefCell double-borrow) - Set fixed width_chars on chunk/conflict labels in merge toolbar to prevent layout shift when label text changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Extract shared key bindings, chunk map helper, fix viewport indicator - Extract map_key_to_action + KeyBindings struct to deduplicate ~120 lines of key handler logic between diff_view and merge_view - Extract create_chunk_map helper to deduplicate chunk map setup - Simplify scroll redraw wiring with loops - Fix chunk map viewport indicator using widget foreground color instead of editor scheme to detect dark/light GTK theme Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Deduplicate dir_window focus handlers and copy actions - Parameterize focus handlers with (own_view, other_view, is_left) loop - Parameterize copy actions with (name, src_dir, dst_dir, status) loop - Replace inline root/downcast patterns with find_window helper Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Split build_new_comparison_tab, extract tab/scroll helpers - Extract make_closeable_tab_label and display_name helpers - Extract open_merge_comparison_tab (merge_view.rs) and open_dir_comparison_tab (dir_window.rs) paralleling open_file_diff_paths - Simplify build_new_comparison_tab from 332 to ~160 lines - Move setup_scroll_sync_3way from merge_view.rs to scroll_sync.rs - Extract sync_hscrolls helper, deduplicating horizontal sync wiring Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add context menu row targeting test case to TESTING.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix clippy warnings, formatting, and missing blank-path guard - Move DIFF_KEYS and MERGE_KEYS statics to module level (clippy items_after_statements) - Add is_blank_path guard in merge view open-externally action - Run cargo +nightly fmt on files committed without formatting Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix reference cycles in scroll sync and page-removed handler - sync_hscrolls: capture Adjustments instead of full ScrolledWindows to avoid holding strong refs to pane widgets in signal closures - open_dir_comparison_tab: use weak ref for widget in page-removed handler and disconnect the signal after it fires to prevent handler accumulation and widget leaks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add dbus-run-session to test-release and document required UI test packages Wrap pytest in dbus-run-session so AT-SPI works in containers. List all system packages needed to run the UI integration tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix collapse-all segfault and disable context menu on missing-side files - Collapse-all iterated with a pre-computed bound; collapsing rows removed children and shrank the model, causing out-of-bounds access. Fixed by iterating in reverse. - Context menu actions (Delete, Copy, Open Externally, Copy Path) were available on the pane where a file doesn't exist, leading to wrong-side operations. Now all per-file actions are disabled on the missing side. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add collapse/expand all toolbar buttons, canonicalize dir header tooltips - Add linked collapse-all / expand-all button pair to directory toolbar - Canonicalize directory paths so pane header tooltips show full absolute paths - Update TESTING.md with collapse/expand, missing-side context menu test cases Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add copy-path buttons to directory pane headers Frameless edit-copy-symbolic button next to each header label copies the full absolute directory path to the clipboard. Path tracks swaps. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Left-align modification time columns in directory comparison Bold text on "changed" rows made right-aligned dates shift relative to non-bold rows. Switch date columns to left-alignment (sizes stay right-aligned) and widen from 180px to 200px. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Make directory comparison columns resizable Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Show conflict chunks in red on merge view chunk map strips Pass conflict flags through to chunk map drawing so chunks that overlap between left and right sides render with band_conflict() color instead of the regular blue/green. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix Replace All modifying read-only buffers in merge view Replace All now skips non-editable TextViews, so in 3-way merge it only replaces in the middle (editable) buffer. Updated TESTING.md to match. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Return focus to active pane when closing find bar Clone the TextView out of the RefCell before calling grab_focus() to avoid a borrow conflict — grab_focus() triggers the focus-enter handler which needs borrow_mut() on the same RefCell. Also fix Replace All modifying read-only buffers in merge view: now skips non-editable TextViews. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add Extra column, Unstage action, and stage-conflict warning to VCS view - Parse staged/partially-staged state from porcelain XY bytes - Skip staged detection for conflict entries (XY is conflict type, not index/worktree status) - Add "Extra" column showing Staged/Partially staged - Add "Unstage" context menu action (git restore --staged) - Stage on conflicted file shows confirmation dialog - Enable/disable Stage vs Unstage based on file state Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * bump * fmt * bump to 0.7.1 * Fix clippy warnings and Copilot review comments - Add #[must_use] to unstage_file - Add #[allow(too_many_arguments)] to draw_chunk_map - Add -- argument terminator in stage_file and unstage_file - Distinguish fully staged from partially staged for Stage action Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 36b42f8 commit f6eb832

17 files changed

Lines changed: 1041 additions & 1270 deletions

Cargo.lock

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "mergers"
3-
version = "0.7.0"
3+
version = "0.7.1"
44
edition = "2024"
55
description = "A visual diff and merge tool for files and directories"
66
license = "GPL-2.0"

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ $(VENV): tests/ui_integration/requirements.txt
1717
test-release: $(VENV)
1818
cargo build --release
1919
xvfb-run -a cargo test
20-
xvfb-run -a $(PYTEST) tests/ui_integration/ -v
20+
dbus-run-session -- xvfb-run -a $(PYTEST) tests/ui_integration/ -v
2121

2222
fmt:
2323
cargo +nightly fmt

README.md

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,23 @@ make test-release
194194
make test-release
195195
```
196196

197-
`make test-release` runs `cargo test` followed by `pytest tests/ui_integration/ -v` under `xvfb-run`.
197+
`make test-release` runs `cargo test` followed by `pytest tests/ui_integration/ -v` under `dbus-run-session` + `xvfb-run`.
198+
199+
**Required system packages (Debian/Ubuntu)**
200+
201+
```bash
202+
sudo apt install \
203+
libgtksourceview-5-dev \
204+
xvfb \
205+
fonts-dejavu-core \
206+
at-spi2-core \
207+
gir1.2-atspi-2.0 \
208+
gir1.2-gtk-3.0 \
209+
dbus-x11 \
210+
xdotool \
211+
python3-pip \
212+
python3-gi
213+
```
198214

199215
**Running UI tests manually (with a display)**
200216

TESTING.md

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@
6767
- [ ] Escape closes find bar from source pane (not just from find entry)
6868
- [ ] Replace: replaces current selection if it matches
6969
- [ ] Replace All: replaces in both buffers
70-
- [ ] Close (X / Escape): hides bar, clears highlights
70+
- [ ] Close (X / Escape): hides bar, clears highlights, returns focus to pane
71+
- [ ] After closing find bar, Ctrl+F / Ctrl+H work immediately
7172

7273
### Save
7374
- [ ] Ctrl+S: saves focused pane; button goes insensitive
@@ -129,6 +130,8 @@
129130
- [ ] Tree view with expandable directories
130131
- [ ] Columns: Name (with icon), Size, Modification time
131132
- [ ] Directory headers above each pane with full path as tooltip
133+
- [ ] Copy path button next to each header: copies full absolute path to clipboard
134+
- [ ] Copy path after swap: copies the swapped (current) path, not the original
132135
- [ ] Status colors: blue (different), orange (left-only), green (right-only), dim italic (missing side)
133136

134137
### Navigation
@@ -145,14 +148,19 @@
145148
- [ ] Copy to left (Alt+Left): copies right to left; confirms if overwriting
146149
- [ ] Copy to right (Alt+Right): copies left to right; confirms if overwriting
147150
- [ ] Delete (Delete key): trashes selected file with confirmation
151+
- [ ] Collapse all: collapses every expanded directory row
152+
- [ ] Expand all: expands every directory row
148153
- [ ] Swap panes: rescans, updates headers and window title
149154
- [ ] Preferences (Ctrl+,)
150155

151156
### Context Menu
152157
- [ ] Right-click selects the clicked row first
158+
- [ ] Right-click targets correct row: select row A, right-click row B, action operates on B (not A)
153159
- [ ] "Open Diff": opens diff tab (disabled for directories)
154160
- [ ] "Copy to Left" / "Copy to Right": enabled for appropriate statuses
155-
- [ ] "Delete": always available
161+
- [ ] "Delete": enabled only when file exists on focused side
162+
- [ ] "Open Externally" / "Copy File Path": enabled only when file exists on focused side
163+
- [ ] Missing-side file: Copy, Delete, Open Externally, Copy Path all disabled
156164

157165
### Notebook Tabs
158166
- [ ] Directory tab labeled with "dir1 — dir2"
@@ -181,6 +189,7 @@
181189
- [ ] Hide hidden files off: dotfiles shown in tree
182190

183191
### Edge Cases
192+
- [ ] Collapse all with missing-side files expanded: no crash
184193
- [ ] Deeply nested directory trees
185194
- [ ] Copy directory recursively: all nested files copied
186195
- [ ] Dir -> file tab -> swap -> edit -> save dialog shows correct filename
@@ -219,7 +228,7 @@
219228
### Find / Replace
220229
- [ ] Ctrl+F/H: find bar highlights matches across all three buffers
221230
- [ ] Match count sums all three buffers
222-
- [ ] Replace All replaces in all three buffers
231+
- [ ] Replace All replaces only in editable middle buffer (not read-only left/right)
223232

224233
### Save
225234
- [ ] Ctrl+S: saves middle pane
@@ -242,8 +251,10 @@
242251

243252
### Display
244253
- [ ] Window title: "mergers -- reponame (git)"
245-
- [ ] ColumnView: Status and File columns
254+
- [ ] ColumnView: Status, File, and Extra columns
246255
- [ ] Status colors: Modified/Renamed = blue, Added/Untracked = green, Deleted = orange
256+
- [ ] Extra column: "Staged" for fully staged, "Partially staged" for staged+unstaged
257+
- [ ] Conflict files: Extra column empty (not falsely "Staged")
247258
- [ ] Repo path label and changed file count
248259

249260
### Opening Diffs
@@ -257,7 +268,9 @@
257268
- [ ] Right-click selects clicked row
258269
- [ ] "Open Diff": opens diff (disabled for Untracked)
259270
- [ ] "Discard Changes": runs `git checkout --` with confirmation (disabled for Untracked)
260-
- [ ] "Stage": runs `git add`
271+
- [ ] "Stage": runs `git add` (disabled when already staged)
272+
- [ ] "Stage" on conflicted file: confirmation dialog warning it marks conflict resolved
273+
- [ ] "Unstage": runs `git restore --staged` (enabled only when staged)
261274
- [ ] "Trash": moves to trash with confirmation (Untracked only)
262275

263276
### File Watcher

src/ui/common/chunk_map.rs

Lines changed: 76 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,36 @@
11
#[allow(clippy::wildcard_imports)]
22
use super::*;
33

4+
#[allow(clippy::too_many_arguments)]
45
pub fn draw_chunk_map(
6+
area: &DrawingArea,
57
cr: &gtk4::cairo::Context,
68
height: f64,
79
total_lines: i32,
810
scroll: &ScrolledWindow,
911
chunks: &[DiffChunk],
10-
is_left: bool,
12+
side: Side,
13+
conflict_flags: &[bool],
1114
) {
1215
if total_lines <= 0 || height <= 0.0 {
1316
return;
1417
}
1518

16-
for rect in &diff_state::compute_chunk_map_rects(chunks, total_lines as usize, height, is_left)
17-
{
18-
let (r, g, b) = match rect.tag {
19-
DiffTag::Replace => band_replace(),
20-
DiffTag::Delete | DiffTag::Insert => band_insert(),
21-
DiffTag::Equal => continue,
19+
for rect in &diff_state::compute_chunk_map_rects(
20+
chunks,
21+
total_lines as usize,
22+
height,
23+
side,
24+
conflict_flags,
25+
) {
26+
let (r, g, b) = if rect.conflict {
27+
band_conflict()
28+
} else {
29+
match rect.tag {
30+
DiffTag::Replace => band_replace(),
31+
DiffTag::Delete | DiffTag::Insert => band_insert(),
32+
DiffTag::Equal => continue,
33+
}
2234
};
2335
cr.set_source_rgba(r, g, b, 0.7);
2436
cr.rectangle(1.0, rect.y_start, 10.0, rect.height);
@@ -30,7 +42,11 @@ pub fn draw_chunk_map(
3042
if adj.upper() > 0.0 {
3143
let view_start = (adj.value() / adj.upper()) * height;
3244
let view_h = (adj.page_size() / adj.upper()) * height;
33-
let dark = is_dark_scheme();
45+
let fg = area.color();
46+
let fg_lum = 0.299 * f64::from(fg.red())
47+
+ 0.587 * f64::from(fg.green())
48+
+ 0.114 * f64::from(fg.blue());
49+
let dark = fg_lum > 0.5;
3450
let v = if dark { 1.0 } else { 0.0 };
3551
let fill_alpha = if dark { 0.20 } else { 0.25 };
3652
let border_alpha = if dark { 0.45 } else { 0.55 };
@@ -44,3 +60,55 @@ pub fn draw_chunk_map(
4460
let _ = cr.fill();
4561
}
4662
}
63+
64+
pub fn create_chunk_map(
65+
buf: &TextBuffer,
66+
scroll: &ScrolledWindow,
67+
chunks: &Rc<RefCell<Vec<DiffChunk>>>,
68+
side: Side,
69+
other_chunks: Option<&Rc<RefCell<Vec<DiffChunk>>>>,
70+
other_mid: Option<(Side, Side)>,
71+
) -> DrawingArea {
72+
let map = DrawingArea::new();
73+
map.set_content_width(12);
74+
map.set_vexpand(true);
75+
{
76+
let buf = buf.clone();
77+
let scroll = scroll.clone();
78+
let chunks = chunks.clone();
79+
let other_chunks = other_chunks.cloned();
80+
map.set_draw_func(move |area, cr, _w, h| {
81+
let flags: Vec<bool> =
82+
if let (Some(oc), Some((my_mid, oc_mid))) = (&other_chunks, other_mid) {
83+
conflict_flags(&chunks.borrow(), my_mid, &oc.borrow(), oc_mid)
84+
} else {
85+
Vec::new()
86+
};
87+
draw_chunk_map(
88+
area,
89+
cr,
90+
h as f64,
91+
buf.line_count(),
92+
&scroll,
93+
&chunks.borrow(),
94+
side,
95+
&flags,
96+
);
97+
});
98+
}
99+
{
100+
let gesture = GestureClick::new();
101+
let scroll = scroll.clone();
102+
let m = map.clone();
103+
gesture.connect_pressed(move |_, _, _x, y| {
104+
let h = m.height() as f64;
105+
if h > 0.0 {
106+
let adj = scroll.vadjustment();
107+
let target = (y / h) * adj.upper() - adj.page_size() / 2.0;
108+
adj.set_value(target.max(0.0));
109+
}
110+
});
111+
map.add_controller(gesture);
112+
}
113+
map
114+
}

src/ui/common/find_bar.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,9 @@ pub fn build_find_bar(
194194
});
195195
}
196196

197-
// ── Replace all ──────────────────────────────────────────────────
197+
// ── Replace all (only in editable buffers) ────────────────────────
198198
{
199-
let bufs = buffers.clone();
199+
let panes_for_replace = all_panes.clone();
200200
let find_e = find_entry.clone();
201201
let repl_e = replace_entry.clone();
202202
replace_all_btn.connect_clicked(move |_| {
@@ -206,7 +206,10 @@ pub fn build_find_bar(
206206
return;
207207
}
208208
let needle_lower = needle.to_lowercase();
209-
for buf in &bufs {
209+
for (tv, buf) in &panes_for_replace {
210+
if !tv.is_editable() {
211+
continue;
212+
}
210213
let text = buf
211214
.text(&buf.start_iter(), &buf.end_iter(), false)
212215
.to_string();
@@ -241,18 +244,22 @@ pub fn build_find_bar(
241244
{
242245
let fr = find_revealer.clone();
243246
let bufs = buffers.clone();
247+
let av = active_view.clone();
244248
find_close_btn.connect_clicked(move |_| {
245249
fr.set_reveal_child(false);
246250
for b in &bufs {
247251
clear_search_tags(b);
248252
}
253+
let tv = av.borrow().clone();
254+
tv.grab_focus();
249255
});
250256
}
251257

252258
// ── Escape / F3 in find and replace entries ──────────────────────
253259
for entry in [&find_entry, &replace_entry] {
254260
let fr = find_revealer.clone();
255261
let bufs = buffers.clone();
262+
let av = active_view.clone();
256263
let fnb = find_next_btn.clone();
257264
let fpb = find_prev_btn.clone();
258265
let key_ctl = EventControllerKey::new();
@@ -262,6 +269,8 @@ pub fn build_find_bar(
262269
for b in &bufs {
263270
clear_search_tags(b);
264271
}
272+
let tv = av.borrow().clone();
273+
tv.grab_focus();
265274
return gtk4::glib::Propagation::Stop;
266275
}
267276
if key == gtk4::gdk::Key::F3 {

0 commit comments

Comments
 (0)