Skip to content

Add an opt-in Ignore whitespace control to local Review diffs #333

Closed

Description

@wheregmis

User problem and benefit

Reindentation or spacing changes can bury a small substantive edit in a large local patch. Threadlane's Review panel currently offers no way to suppress that noise without leaving the application.

Add an opt-in Ignore whitespace control to the local Review diff document, for both a selected file and All changes. It changes only the displayed patch, never repository content, commit selection, or agent context. Keep it off by default: whitespace can be meaningful in indentation-sensitive languages and string literals.

Source inspiration and evidence

Source-code/documentation research only; neither source application nor Threadlane was launched or visually inspected.

  • Synara, revision ff9e89dc16365181c7d112a5764f68989ed6f32e: its diff options menu has a checked Ignore whitespace-only changes option. Its checkpoint diff implementation conditionally passes --ignore-all-space to Git. Borrow the explicit, reversible presentation option—not its checkpoint infrastructure.
  • T3 Code, revision c2fa9fc911daeac97df4760f95fc57dca42b84c8: its diff toolbar exposes a pressed toggle with accessible Show/Hide whitespace changes labels. The query inputs carry that option to the backend; the Git argument builder adds --ignore-all-space. Borrow real Git filtering and explicit state, not DOM-side removal of patch lines.

This is a smaller, more immediately reusable improvement than adopting rich pasted-text attachments or a new diff renderer. Conversation/terminal search, attachment previews, file completion, review progress, and terminal-to-chat handoff were excluded because existing issues already cover them.

Threadlane baseline and reuse

Checked current main at bca44c42a6072612fde47f472bdd6f8469dadaee; these relevant files are identical to the available worktree:

Proposed UX and interaction states

  1. Scope and entry: in the local diff document header, add a standard kit Checkbox labeled Ignore whitespace, unchecked initially. It applies only to this Review panel's local patch view, remains selected across files/All changes in the same checkout, and resets on checkout/project change and app restart. Do not show it for editable files, GitHub PR diffs, history/stash previews, or editor-tab diff exports in this first increment. Keep existing Back/Close commands reachable. At narrow widths allow a compact second header row rather than clipping controls.
  2. Visible semantics: while enabled, retain the checked label and show Whitespace ignored · Display only near the patch. Help/accessibility description: Ignores whitespace when comparing lines. Whitespace can affect program behavior. File counts and commit selection are unchanged. Do not call this a semantic diff or claim the remaining patch is safe to commit. File lists, additions/deletions totals, and selected files continue to describe the unfiltered working tree.
  3. Loading/error: toggling reloads the same target in the background. Show Updating diff…; do not present the previous patch under the new filter label as if current. Allow a later toggle to supersede an earlier request. A failure shows Could not load diff with Retry and the option to turn filtering off; it must not masquerade as an empty patch. Local reads require no GitHub credentials/network and retain existing permission errors.
  4. Empty and non-text changes: a successful empty filtered patch says No text changes to show with whitespace ignored, with a Show whitespace changes button. Never substitute Working tree clean. Keep binary, rename, mode-change, and other metadata that Git returns; do not remove entire files based on a zero-text count. Untracked/new files remain visible as additions. Ordinary unfiltered empty/error behavior remains distinct.
  5. Keyboard and lifecycle: Tab reaches the checkbox; Space toggles with a visible focus ring and announced checked state. Activation must not move focus into the patch or trigger commit/discard. Reject late results after file/project changes, Back/Close, or a newer toggle. Use existing themed component sizing, semantic colors and roles; no hover-only control or color-only indication.

Scoped implementation plan

  1. Git leaf: extend crates/threadlane-git/src/git.rs with the smallest option-aware variants needed by the two review loaders; retain current public wrappers as unfiltered defaults for all existing callers. Reuse path validation, --no-ext-diff, and Git error handling. Pass --ignore-all-space through the appropriate tracked HEAD/staged/unstaged paths; do not strip whitespace or patch lines in Rust. Represent a successful empty result separately from an error and UI prose.
    • Important correctness boundary: an empty successful HEAD diff is a valid comparison result, not evidence of an unborn HEAD. Only use the staged/unstaged fallback when that comparison is unavailable. A temporary-repository experiment confirmed that git diff -w HEAD can be empty while git diff -w --cached is nonempty when staged and unstaged edits cancel. Blindly keeping the current empty-output fallback would display the wrong comparison.
    • Collect untracked additions independently of whether tracked output is nonempty; ensure both filter modes cover the same file universe. Keep this limited to correct composition of the touched helper, not a general Git refactor.
  2. State and reload: in crates/threadlane-ui-right-panel/src/view.rs, keep the option and active diff target in RightPanelView. Use a small explicit target (File(path) versus AllChanges) instead of inferring identity from the rendered title. Route toggles and PanelEvent::ReviewLoaded refresh through that target; the current refresh path parses the title as a file path, which cannot safely reload All changes. Include mode/target in request identity and guard pending results when applying them, not just when receiving them. Clear stale pending content when invalidating a request.
  3. UI composition: extend the existing document header and status/empty/error rendering in render_files/sync_pending_document; retain TextViewState and the background executor. Reuse the already-imported kit Checkbox/Button. Follow AGENTS.md and the project GPUI Kit design guide: visible state, region-scoped controls, theme/rem tokens, named keyboard controls, restrained hierarchy. No changes to the binary shell, daemon, protocol, persistent settings, or dependencies.
  4. Tests: extend crates/threadlane-git/src/tests.rs, crates/threadlane-ui-right-panel/src/tests.rs, and the existing in-module GPUI tests in src/view.rs. Preserve retain_review_selection and its tests. Mount interaction tests under initialized gpui_component::Root as required by project instructions.

Acceptance criteria and validation

  • File and All changes diffs can toggle whitespace filtering; mixed edits retain non-whitespace differences, and disabling it restores the full patch.
  • Filtered-empty, clean, loading, error and non-text/metadata outcomes remain distinguishable. Staged/unstaged cancellation, unborn HEAD, detached HEAD, and tracked-plus-untracked changes have regression fixtures.
  • Filtering never changes files, index, selected commit files, raw status counts, generated commit prompts, editor exports, or GitHub PR review state. Validate working-tree bytes and index before/after the read-only operations.
  • Rapid toggles, refresh, navigation and project/worktree changes cannot publish a stale mode/target. All changes refresh stays an aggregate diff. Closing during a request cannot reopen the document.
  • Keyboard-only use, accessible name/checked state, narrow panels, larger fonts, and light/dark themes remain usable; errors permit recovery without losing commit selection.

Implementation validation: cargo nextest run -p threadlane-git, cargo nextest run -p threadlane-ui-right-panel, cargo check -p threadlane-gpui, and git diff --check. Manually exercise a real window with whitespace-only and mixed patches, new/deleted/binary files, Unicode/spaced paths, rapid toggles and checkout switches; check VoiceOver and focus behavior. These implementation tests have not been run as part of this research-only proposal.

Nonduplication and tradeoffs

Reviewed all 87 existing open/closed issues, searching titles, bodies and comments by whitespace/spacing/indentation/formatting, diff/patch/review, filtering/hiding/noise, and related workflow concepts. Nearby scopes:

  • #271 / PR #274 add Environment Git status and navigation; #291 / PR #294 add commit/pull/push/branch actions. Their closing PR descriptions and relevant discussion do not cover patch whitespace filtering. The image-only issue references could not be visually inspected because the embedded browser was unavailable; the scope assessment uses those linked implementation records and current source instead.
  • #311 concerns GitHub-backed Viewed markers and Next unviewed, not local patch presentation.
  • #156 concerns GitHub authentication, PR lifecycle, checks, review feedback and transport. This proposal makes no GitHub mutation or PR-review change.

Git --ignore-all-space can hide meaningful indentation and whitespace inside strings: default-off, explicit scope and a persistent indicator are essential. Raw file totals will intentionally differ from the filtered display. Changing filter mode can alter hunk structure and scroll position; retaining the same target matters more than pretending old line offsets are stable. Large-patch rendering limits are unchanged; this is not a virtualization or AST-diff project.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions