Home/lazarus
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix: preserve selection on delete/archive in dashboard and search #7

Closed
clanker wants to merge 17 commits from fix/selection-on-delete into master
pull from: fix/selection-on-delete
merge into: Home:master
Conversation 0 Commits 17 Files changed 7 +301 -113
clanker commented 2026-07-24 20:43:05 +00:00
Owner
Copy link

Problem

When deleting or archiving a thread in the dashboard or search panels, the selection disappeared and pressing j/k would jump to the top of the list.

Root Cause

  • Dashboard: _debounced_refresh() always called _select_first_thread(), ignoring the current position
  • Search: refresh() fell back to _select_first_row() when the deleted thread ID was no longer in the model

Fix

  • DashboardPanel: Added _refresh_pending_row attribute — set before triggering update_single_thread in delete/archive/archive-to-local paths. _debounced_refresh() now reads this and calls _select_near_row() to land on the item that took the deleted item's place (or the last item if it was the last one)
  • DashboardPanel: Added _select_near_row() and _select_last_thread() helpers
  • SearchPanel: refresh() now falls back to row-position clamping (_select_near_row) instead of always selecting first row when thread ID is gone
  • SearchPanel: Added _select_near_row() helper

Behavior After Fix

  • Delete last item → selection moves to new last item
  • Delete middle item → selection moves to the item that slides into its position
  • Archive works the same way
  • Marked bulk operations still refresh normally
## Problem When deleting or archiving a thread in the dashboard or search panels, the selection disappeared and pressing `j`/`k` would jump to the top of the list. ## Root Cause - **Dashboard**: `_debounced_refresh()` always called `_select_first_thread()`, ignoring the current position - **Search**: `refresh()` fell back to `_select_first_row()` when the deleted thread ID was no longer in the model ## Fix - **DashboardPanel**: Added `_refresh_pending_row` attribute — set before triggering `update_single_thread` in delete/archive/archive-to-local paths. `_debounced_refresh()` now reads this and calls `_select_near_row()` to land on the item that took the deleted item's place (or the last item if it was the last one) - **DashboardPanel**: Added `_select_near_row()` and `_select_last_thread()` helpers - **SearchPanel**: `refresh()` now falls back to row-position clamping (`_select_near_row`) instead of always selecting first row when thread ID is gone - **SearchPanel**: Added `_select_near_row()` helper ## Behavior After Fix - Delete last item → selection moves to new last item - Delete middle item → selection moves to the item that slides into its position - Archive works the same way - Marked bulk operations still refresh normally
- DashboardPanel: add _refresh_pending_row to remember target position
  before async debounced refresh; _debounced_refresh now selects the
  nearest non-header row at or after the saved position instead of
  always jumping to first thread

- DashboardPanel: add _select_near_row() and _select_last_thread() helpers

- SearchPanel: refresh() fallback now restores to current row position
  (clamped to valid range) instead of always selecting first row

- SearchPanel: add _select_near_row() helper
- DashboardPanel delete/archive/archive-to-local now advance selection
  BEFORE the notmuch operation, then refresh the model synchronously
  (stopping the debounce timer) and restore the advanced position

- SearchPanel delete/archive/archive-to-local now call next_thread()
  before the notmuch operation so selection lands on the next item

- Add _advance_past_current() helper to DashboardPanel

- Remove unused _refresh_pending_row mechanism
- Move notmuch new into _BulkMoveWorker.run(), after file moves
  complete but before batch_done is emitted.  This guarantees the
  notmuch database is updated only after files are actually moved.

- Remove the premature notmuch new --no-hooks calls from
  move_to_trash() and move_to_archive().  Previously these ran on
  the main thread before the background worker had processed any
  moves, making them no-ops.

- Run notmuch new WITH hooks (no --no-hooks).  The post-new hook
  does important work: Trash folder tag cleanup and mbsync flag
  sync.  Since notmuch new now runs in the background thread it
  doesn't block the UI, so hooks are safe.

- Wire batch_done -> Dodo.refresh_panels via connect_batch_done()
  so the UI refreshes after file moves + reindex complete.

- Store the callback and re-connect if the worker is re-created
  after a timeout, preventing lost connections.
- Add _unique_dest() helper that appends .1, .2, etc. when the
  destination already exists, preventing overwrites when two files
  produce the same basename after UID stripping (#21)

- _BulkMoveWorker now tracks moved/failed counts and emits them via
  batch_done(int, int) so failures are surfaced to the user

- App now shows a warning status bar message when any file moves fail

- _BulkMoveWorker.__init__ initialises _moved/_failed counters
- Add log_level and log_file settings to settings.py
- Replace ad-hoc --verbose basicConfig with _setup_logging() method
  that runs early in Dodo.__init__ (before config.py loads)
- Supports stderr + optional file handler, timestamped format
- --verbose/-v flag still overrides level to INFO for the session
Previously _setup_logging() ran before config.py was exec'd, so
settings.log_level and settings.log_file were always their defaults.
User config had no effect.

Now: minimal stderr logger at startup (captures early errors), then
full reconfiguration via _setup_logging() after config.py loads.
Uses root logger setLevel + addHandler instead of basicConfig
(basicConfig is no-op when handlers already exist).
When a source file no longer exists, check if it's already at the
destination before counting it as a failure.  This handles:
- Files moved by a previous batch (double-enqueue)
- Files moved by a previous dodo session (old race condition)
- Genuine missing files (logged as warning, counted as failed)
Run 'notmuch new --no-hooks' at the start of move_to_trash() and
move_to_archive() so the notmuch database is current before we query
file paths.  Without this, paths returned by 'notmuch search
--output=files' may be stale (mbsync removed files but notmuch hasn't
reindexed), causing 'source missing, dest not found' failures.

Uses --no-hooks to avoid triggering post-new hook (which may run
mbsync) during the synchronous pre-flight check.  The worker still
runs a full 'notmuch new' (with hooks) after moves complete.
- Add _sync_notmuch() helper with return-code checking and logging
- Filter out files that don't exist on disk before adding them to the
  move queue.  Stale paths (removed by mbsync but still in notmuch DB)
  are logged and skipped instead of being sent to the worker where
  they'd fail with 'source missing, dest not found'.

- Count skipped stale files separately and log at INFO level
Remove _BulkMoveWorker QThread, queue, batch_done signal, singleton,
connect_batch_done, _sync_notmuch, and all associated wiring.

Rationale: os.rename within the same filesystem is a directory-entry
update (~microseconds per file).  Even 500 files takes ~50ms.  The
only material cost is notmuch new at the end (~1-2s), which is the
same whether it runs on the main thread or in a background worker.

The background thread created more problems than it solved:
- Race: notmuch new ran before moves completed
- Stale file paths from out-of-sync DB
- Double-refresh (batch_done + caller's refresh_panels)
- Lost signal connections when worker timed out
- Complex queue/sentinel/callback machinery

Now: tag, move, reindex — synchronous, sequential, correct by
construction.  Callers already call refresh_panels() after
move_to_* returns, so the UI updates immediately.

-124 lines net.
Add 'notmuch new --no-hooks' as step 1 in both move_to_trash and
move_to_archive.  This syncs the database with the filesystem before
we query file paths, eliminating 'stale path' failures when mbsync
has renamed files since the last notmuch new.

The '--no-hooks' flag prevents the post-new hook from triggering
mbsync during the pre-flight check.  The final 'notmuch new' at step 5
runs with hooks to handle Trash cleanup and flag sync.
This reverts commit f066762828.
When os.path.exists returns False for a path from notmuch, list the
parent directory and log the closest matching filenames.  This will
reveal whether:
- The directory is empty/deleted
- Files have been renamed (different UID suffix)
- There's a filesystem encoding issue
The root cause: mbsync syncs mail flags (e.g. Seen) from the server
and renames files from :2, → :2,S.  Notmuch still has the old path
until 'notmuch new' runs.  For large batches, the chance that mbsync
touched some files since the last notmuch new is high.

Instead of skipping files that don't exist at their exact notmuch
path, search the parent directory for a file with the same basename
stem (everything before :2,).  This handles flag-sync renames
transparently without needing a pre-flight notmuch new.
Tags from notmuch are alphabetically sorted, putting 'marked' somewhere
in the middle.  Sort it to the front so it's always visible at a
glance when scanning marked threads.
SearchPanel.refresh() already advances selection correctly via
_select_near_row() fallback (when deleted thread_id is not found).
The explicit next_thread() call was causing a double-advance:
once before the operation, then again during refresh restoration.
- Add PR summary table (6 merged + 1 open)
- Document mbsync flag-sync rename issue and directory-glob fix
- Document synchronous file move simplification
- Document selection-on-delete behavior differences
- Document _unique_dest collision avoidance
- Document logging configuration
- Document 'marked' tag sort order
- Update actions.py description and bulk-move notes
RulyTafzil closed this pull request 2026-08-07 23:09:33 +00:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Home/lazarus!7
No description provided.