Skip to content

feat: snap to column boundaries option for Niri layout#228

Draft
adelin-b wants to merge 35 commits into
BarutSRB:mainfrom
adelin-b:feat/snap-to-column-boundaries
Draft

feat: snap to column boundaries option for Niri layout#228
adelin-b wants to merge 35 commits into
BarutSRB:mainfrom
adelin-b:feat/snap-to-column-boundaries

Conversation

@adelin-b
Copy link
Copy Markdown
Contributor

@adelin-b adelin-b commented Apr 11, 2026

Summary

Adds a new niriSnapToColumnBoundaries setting that makes trackpad gesture scrolling snap to clean column edges, preventing columns from settling in partially-visible positions.

Problem

When using Niri layout with centerFocusedColumn: never, the viewport can settle at positions where columns are partially visible on the sides. The current snap logic creates left/right padding snap points per column, allowing "in-between" resting positions.

Solution

When snapToColumnBoundaries is enabled, the snap point generator creates one snap point per column at its left edge instead of two padded snap points. This ensures the viewport always starts at a column boundary — columns are either fully visible or fully hidden.

How it works

In findSnapPointsAndTarget(), when the setting is active:

// Before (default): two padded snap points per column
let leftSnap = colX - padding
let rightSnap = colX + colW + padding - vw

// After (snap to boundaries): one clean snap point per column  
snapPoints.append((colX, idx))

The active column detection also uses zero padding to match the tighter alignment.

Files changed (12)

File Change
SettingsStore.swift New niriSnapToColumnBoundaries property
SettingsExport.swift JSON export/import
MonitorNiriSettings.swift Per-monitor override + ResolvedNiriSettings
NiriLayoutEngine.swift Engine property + config wiring
NiriLayoutEngine+Monitors.swift Global resolved settings
ViewportState+Gestures.swift Core snap point logic (the actual fix)
MouseEventHandler.swift Pass setting to endGesture()
NiriLayoutHandler.swift Config passthrough
WMController.swift Settings application
SettingsView.swift Global toggle + per-monitor override
NiriLayoutEngineTests.swift Test helper update
SettingsStoreTests.swift Test helper update

Design

  • Default: false (fully backward compatible)
  • Per-monitor overridable (like other Niri settings)
  • Only affects gesture snap — keyboard focus navigation is unchanged
  • When centerFocusedColumn: .always, this setting has no effect (centering already prevents partial columns)

Test plan

  • Enable setting → trackpad scroll snaps to column edges, no partial columns visible
  • Disable setting → original behavior with padded snap points
  • Per-monitor override works independently
  • Keyboard focus (alt+h/l) unaffected
  • JSON export includes setting, import applies it
  • Setting persists across restart

Summary by CodeRabbit

  • New Features
    • Added "Snap to Column Boundaries" setting with global and per-monitor override options.
    • New setting is persisted across sessions and fully integrated with configuration export/import functionality.

Barut and others added 30 commits April 6, 2026 14:43
Replace the Niri axis solver, viewport geometry helpers, and monitor restore assignment matcher with Zig implementations behind a small C ABI surface.

Add a dedicated COmniWMKernels header target, a reproducible kernel build script, focused Swift and Zig regression coverage, and the documentation/build updates needed to rebuild and test the new library.
Stabilize the layout and CLI regression coverage after the Zig kernel port.

Enable animations explicitly in the shared layout-plan fixture, make the scratchpad async reveal test use the controller-local AX frame override after setup refreshes settle, add deterministic viewport settling in the Niri animation-toggle regressions, widen the injected CLI watch timeouts under aggregate load, and ignore Zig build cache output.
Pin shared build metadata for the macOS target, Zig toolchain, and Ghostty archive digest.

Add reusable preflight helpers plus canonical make targets for build, test, verify, and release-check workflows.

Refactor Zig and packaging scripts to consume the pinned metadata and fail fast on provenance or configuration mismatches.
Introduce per-target SwiftLint configs and target-specific thresholds so
the
stricter root lint policy can be rolled out without forcing legacy-heavy
modules to fail immediately.

Clean up the surfaced violations across Core, UI, IPC, Quake Terminal,
and
tests by replacing unsafe casts and `try!` usage, removing unnecessary
`async`, tightening access control, and marking unsupported
`init(coder:)`
paths unavailable.

Refresh the contributor docs with the supported `make build`, `make
test`,
`make verify`, and `make release-check` workflows, and align the AX/Niri
regressions with the cleanup.
Rewrite the scratchpad async fronting test to use the real async AX frame-apply path instead of the synchronous frameApplyOverrideForTests hook.

Add a small layout-plan test helper that installs an AppAXContext and disables the default synchronous override for async-sensitive tests.

This keeps the production scratchpad code unchanged while stabilizing the previously failing scratchpad test and the full Swift suite.
Move the pure Dwindle frame solver behind the COmniWMKernels C ABI and have Swift flatten the workspace tree into a kernel snapshot before reapplying solved frames to existing nodes.

This removes the Swift gap and min-size recursion, adds the new Zig solver entry point, and expands Dwindle regression coverage for single-window, fullscreen, gap, placeholder, and min-size behaviors.

Tested with: swift test --filter DwindleLayoutEngineTests
Fix typo in README for 'Scratchpad'
Stop using pending reveal transactions for workspaceInactive reveals so cross-workspace activation no longer re-hides windows or re-suppresses frame writes after transient verification misses.

Keep delayed reveal verification for floating and scratchpad-style async restores, and add regression coverage for the layout refresh, inactive-workspace activation, and workspace-bar focus paths.

Fixes BarutSRB#204
Fixes BarutSRB#207
Move Niri's deterministic bulk projection/layout slice out of Swift and into the Zig kernel library behind `omniwm_niri_layout_solve`.

Swift keeps ownership of the Niri tree, viewport state, monitor selection, and AppKit policy while flattening the current columns and windows into compact snapshot arrays, invoking one bulk solve, and applying the returned canonical/rendered rects, resolved spans, and hidden-edge classifications back onto the existing nodes.

This removes the old Swift-side projection math for container rects, window frames, visibility and overflow handling, and single-window aspect fitting, adds ABI coverage for empty and bounded caller-allocated buffers, and updates the architecture docs to document the Niri leaf-kernel boundary.

Tests:
- zig build test
- swift test --filter NiriLayoutEngineTests
- swift test --filter NiriConstraintSolverTests
- swift test --filter ViewportGeometryTests
- swift test --filter MonitorRestoreAssignmentsTests
- swift test --filter NiriLayoutKernelABITests
Move the deterministic reconcile reducer and restore-intent solve behind a compact Zig-backed C ABI.

Keep RuntimeStore, Planner, event normalization, trace recording, invariant validation, and runtime object mutation Swift-owned while StateReducer becomes a thin marshal/decode seam over omniwm_reconcile_plan and omniwm_reconcile_restore_intent.

Thread persisted hydration through the same solve, add direct ABI coverage plus Zig unit tests for reconcile behavior, and update architecture docs to document the new leaf-kernel boundary.

Testing:
- zig build test
- swift test --filter ReconcileStateTests
- swift test --filter ReconcileKernelABITests
- make kernels-test
- make test
Move keyboard focus border ownership and lifecycle decisions into the
BorderCoordinator reconcile loop so border visibility, ordering, and
fallback behavior stay consistent across focus changes, CGS events,
workspace transitions, and fullscreen/teardown paths.

- track managed and fallback owners with generation-scoped state,
  fallback leases, live-motion revalidation, and bounded trace records
  so stale frame/teardown events are ignored instead of reviving old
  borders
- derive ordering metadata and corner radius from validated window
  server state while keeping BorderManager and BorderWindow focused on
  rendering and preventing hidden config changes from replaying stale
  target state
- route WMController, AXEventHandler, layout refresh, workspace,
  service lifecycle, and Niri animation border updates through explicit
  reconcile sources and add regression tests plus architecture notes
  for the new behavior
Use one exact snappy spring preset for normal-mode animations.

Remove balanced/gentle presets and per-call spring tuning.

Make Reduce Motion resolve to the exact reducedMotion preset.

Switch Niri movement, workspace switching, overview animation, and window-close motion to plain snappy.

Add tests for spring config resolution, Niri config wiring, and close-animation settling.

Update architecture docs to reflect the new preset model.
Replace Dwindle's cubic window motion with a dedicated no-bounce spring
configuration and thread display refresh rate through workspace
snapshots
so motion settles consistently across monitors.

Snap canonical and animated Dwindle frames to physical pixels, seed new
split insertions from the split edge, and make fullscreen exclusive per
workspace to avoid overlapping fullscreen leaves.

Unify Dwindle settings application through the handler snapshot context,
remove the unused cubic animation path, and harden AX frame writes by
rejecting invalid target geometry without retrying.

Add regression coverage for Dwindle animation lifecycle, pixel snapping,
refresh-rate plumbing, insertion seeds, fullscreen exclusivity, and AX
invalid-frame rejection.

Verified with:
- swift test --filter
'DwindleLayoutEngineTests|SpringAnimationTests|AXManagerTests'
- swift test --skip-build
Record hotkeys directly from the responder chain instead of relying on a
local event monitor.

This lets the recorder capture command-based key equivalents before
AppKit swallows them, which fixes top-row workspace shortcuts on
Czech/QWERTZ and other non-QWERTY layouts.

Add regression tests that verify command plus top-row keys are stored by
physical key code, including through performKeyEquivalent. Fixes BarutSRB#171
Refresh resolved Niri monitor settings whenever global Niri config changes so monitors inheriting defaults update immediately from the settings window.

Route monitor-specific Niri refreshes through the same helper and add regression coverage for live center-focused-column and single-window-aspect-ratio propagation.
Accept AXStandardWindow subrole windows during AX enumeration even when the AX role is non-standard or missing, so Emacs-style windows remain tracked across active-space changes and wake/unlock full rescans.

Fixes BarutSRB#197
Route quake terminal hide/show through the shared focus pipeline so manual close restores the latest valid focus target instead of replaying a stale app snapshot.

Add regression coverage for manual close, focus-loss auto-hide, and external focus fallback paths.

Fixes BarutSRB#212
Move the deterministic Overview projection path out of Swift and into the
existing Zig kernels library.

Replace the generic workspace and Niri overview projection math in
OverviewLayoutCalculator with a bulk omniwm_overview_projection_solve FFI
call. The new kernel owns frame normalization, scale fitting, projected
window and column geometry, drop-zone generation, total content height,
and scroll bounds, while Swift keeps snapshot extraction, search,
navigation, thumbnails, and result application.

Extend the checked-in C ABI with explicit overview snapshot and result
structs, add the new Zig solver module, and update the universal archive
build step to run ranlib after lipo so the rebuilt static library links
cleanly.

Add direct ABI regression coverage plus extra Overview projection
characterization tests, and verify the change with:
- make kernels-build
- make kernels-test
- swift test --filter OverviewProjectionKernelABITests
- swift test --filter Overview
Replace the deterministic WindowRuleEngine base-decision tree with a compact Zig-backed window decision kernel behind omniwm_window_decision_solve.

Keep rule compilation, regex matching, title gating, metadata ownership, and manual overrides in Swift while reattaching workspace and rule-effect metadata after the kernel decode.

Add ABI coverage, decision regression tests, higher-level admission/manual-override coverage, and architecture notes for the new leaf-kernel boundary.
Seed native fullscreen restore snapshots before command-driven, direct-activation, and full-rescan fullscreen transitions so restored windows can replay their managed geometry after exiting AppKit fullscreen.

Keep restore records alive through the first relayout commit, suppress Niri and Dwindle animations during that restore pass, and force-apply the captured frame before clearing lifecycle state.

Expand AX event, refresh routing, and layout refresh tests to cover command-driven, direct-activation, replacement-token, and restore-plan finalization flows.
Normalize refresh, focus, and activation controller inputs into an explicit orchestration snapshot/event/plan boundary and route deterministic planning through OrchestrationCore.

Thin WMController, LayoutRefreshController, and AXEventHandler to adapter/executor roles, keep runtime ownership and platform effects in Swift, and add seam-level regression tests for refresh coalescing, focus supersede/defer, and native fullscreen restore activation planning.
Replace the Swift orchestration reducer with a Swift encoder/decoder
around
the new `omniwm_orchestration_step` C ABI. The Zig kernel now owns the
deterministic refresh and managed-focus state transitions, while Swift
continues to own macOS-facing effects like AX focus, borders, workspace
activation, refresh tasks, and retry scheduling.

Add the orchestration ABI surface to `COmniWMKernels`, including
flattened
refresh/focus snapshots, activation observations, window-removal
payloads,
decisions, ordered actions, and ABI layout reporting. The Swift wrapper
grows
caller-owned output buffers on `BUFFER_TOO_SMALL` and decodes the
returned
snapshot, decision, and action plan back into existing OmniWM types.

Mirror kernel-owned managed-focus state back into
`FocusBridgeCoordinator`
and `WorkspaceManager`, and route activation handling through normalized
kernel events. This moves retry budget progression, retry exhaustion,
owned-application fallback, native fullscreen restore activation, and
managed
activation confirmation into the reducer boundary.

Preserve refresh merge behavior across the port, including affected
workspace
sets, window-removal payloads, post-layout attachments, visibility
follow-ups,
and cancelled-refresh restart state. Expand orchestration and ABI tests
to
cover the new kernel boundary and update architecture docs for the
kernel-backed orchestration flow.
Barut and others added 5 commits April 10, 2026 11:15
Drop the Niri workspace-switch slide state so workspace changes use the existing immediate relayout path without starting Niri scroll or Dwindle animation state.

Update routing and Niri layout tests for the no-animation behavior, and include the native-fullscreen no-snapshot restore coverage already present in the working tree.
Capture managed restore snapshots with frame, topology, Niri sizing
state,
and replacement metadata so native fullscreen exits can restore the
prior
managed layout more faithfully.

Use the cached restore snapshots when fullscreen records are created,
suspended, or restored, and carry them across replacement-window rekeys.
Match temporarily unavailable fullscreen records with exact replacement
metadata when multiple same-pid candidates exist instead of relying on
the
active workspace alone.

Finalize native fullscreen restore records only after the expected frame
write is confirmed, while still handling hidden/offscreen restore paths.
This keeps failed restore writes from prematurely clearing the record.

Add regression coverage for Niri sizing restoration, hidden column
stability,
same-app replacement matching, Dwindle identity preservation across
workspace
mismatches, and delayed restore finalization.
Add the omniwm_niri_topology_plan C ABI and Zig implementation for workspace-local Niri topology/navigation decisions, including removal reconciliation, insertion, focus, ensure-visible, column movement, and window movement planning.

Replace the old Swift planner logic with thin snapshot/marshalling/application code while keeping NiriNode ownership, geometry, effects, controller policy, AppKit, and AX behavior in Swift.

Cover the new kernel with direct ABI tests, Niri layout integration cases, and architecture documentation updates.
Move the deterministic workspace navigation/session planner behind a compact C ABI and a new Zig leaf kernel.

Keep WorkspaceManager, layout mutation execution, focus execution, and refresh/orchestration in Swift while replacing the old target-selection, transfer-planning, follow-focus, source-recovery, and affected-set solver logic.

Add direct ABI coverage and Zig unit tests, remove the dead Swift monitor/workspace traversal helpers, and update the architecture guide for the new boundary.
When enabled, trackpad gesture scrolling snaps to clean column
boundaries instead of allowing partial columns to peek from the edges.
Creates one snap point per column at its left edge position, ensuring
the viewport always starts at a column boundary.

New setting: niriSnapToColumnBoundaries (default: false)
- Global toggle in Settings > Niri > Snap to Column Boundaries
- Per-monitor override support
- JSON export/import support
- Backward compatible (defaults to off)
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 11, 2026

📝 Walkthrough

Walkthrough

A new snapToColumnBoundaries boolean configuration setting has been introduced across the layout engine's settings pipeline, persistence layer, gesture handling, and UI. The setting is configurable globally and per-monitor, controlling column snapping behavior during viewport gestures.

Changes

Cohort / File(s) Summary
Version Control
.gitignore
Added .omc/ directory to ignore list.
Settings Configuration Structures
Sources/OmniWM/Core/Config/MonitorNiriSettings.swift, Sources/OmniWM/Core/Config/SettingsExport.swift, Sources/OmniWM/Core/Config/SettingsStore.swift
Added snapToColumnBoundaries (optional for per-monitor, required for storage) with initialization, coding keys, and UserDefaults persistence; integrated into settings resolution pipeline.
Configuration Propagation Pipeline
Sources/OmniWM/Core/Controller/WMController.swift, Sources/OmniWM/Core/Controller/NiriLayoutHandler.swift, Sources/OmniWM/Core/Controller/MouseEventHandler.swift
Threaded snapToColumnBoundaries parameter through controller chain and gesture finalization to pass setting to layout engine.
Layout Engine & Gesture Logic
Sources/OmniWM/Core/Layout/Niri/NiriLayoutEngine.swift, Sources/OmniWM/Core/Layout/Niri/NiriLayoutEngine+Monitors.swift, Sources/OmniWM/Core/Layout/Niri/ViewportState+Gestures.swift
Added snapToColumnBoundaries property to engine; updated gesture finalization to conditionally generate snap points aligned to column boundaries when enabled, overriding padding-based snapping behavior.
User Interface
Sources/OmniWM/UI/SettingsView.swift
Added global and per-monitor toggle controls for "Snap to Column Boundaries" setting with appropriate state binding and update handlers.
Tests
Tests/OmniWMTests/NiriLayoutEngineTests.swift, Tests/OmniWMTests/SettingsStoreTests.swift
Updated test helpers and fixtures to support the new setting in resolved settings and export/import round-trip coverage.

Sequence Diagram

sequenceDiagram
    participant UI as SettingsView
    participant WM as WMController
    participant NLH as NiriLayoutHandler
    participant NLE as NiriLayoutEngine
    participant VS as ViewportState

    UI->>WM: updateNiriConfig(snapToColumnBoundaries: true)
    WM->>NLH: updateNiriConfig(snapToColumnBoundaries: true)
    NLH->>NLE: updateConfiguration(snapToColumnBoundaries: true)
    NLE->>NLE: self.snapToColumnBoundaries = true
    
    Note over VS: User performs gesture
    
    UI->>WM: (gesture event)
    WM->>VS: endGesture(snapToColumnBoundaries: true)
    VS->>VS: findSnapPointsAndTarget(snapToColumnBoundaries: true)
    VS->>VS: Generate column-boundary snaps<br/>(not padding-based)
    VS->>VS: Adjust scroll direction with 0.0 padding
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A new snap point comes alive,
Column boundaries help views thrive,
Through configs and gestures it flows,
With toggles in settings it glows,
Each viewport now dances with grace!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding a snap-to-column-boundaries option for the Niri layout engine, which aligns with all file changes across configuration, UI, engine logic, and tests.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch feat/snap-to-column-boundaries

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Sources/OmniWM/Core/Controller/MouseEventHandler.swift (1)

1131-1139: ⚠️ Potential issue | 🟠 Major

Use resolved monitor settings here; global values bypass per-monitor overrides.

At Line 1139, engine.snapToColumnBoundaries uses only the global value. This makes monitor overrides ineffective during gesture finalization. The same call currently uses global centering flags too, so monitor-resolved behavior can drift during snap completion.

🔧 Proposed fix
-        controller.workspaceManager.withNiriViewportState(for: wsId) { endState in
+        let resolved = engine.effectiveSettings(for: lockedContext.monitorId)
+        controller.workspaceManager.withNiriViewportState(for: wsId) { endState in
             endState.endGesture(
                 columns: columns,
                 gap: gap,
                 viewportWidth: insetFrame.width,
                 motion: controller.motionPolicy.snapshot(),
-                centerMode: engine.centerFocusedColumn,
-                alwaysCenterSingleColumn: engine.alwaysCenterSingleColumn,
-                snapToColumnBoundaries: engine.snapToColumnBoundaries
+                centerMode: resolved.centerFocusedColumn,
+                alwaysCenterSingleColumn: resolved.alwaysCenterSingleColumn,
+                snapToColumnBoundaries: resolved.snapToColumnBoundaries
             )
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/OmniWM/Core/Controller/MouseEventHandler.swift` around lines 1131 -
1139, The call to endState.endGesture is passing global engine flags
(engine.centerFocusedColumn, engine.alwaysCenterSingleColumn,
engine.snapToColumnBoundaries) which bypass per-monitor overrides; replace those
three arguments with the monitor-resolved values before calling endGesture
(e.g., obtain the resolved settings for this workspace/viewport from the
workspace/viewport state or a resolver method and pass
resolvedCenterFocusedColumn, resolvedAlwaysCenterSingleColumn,
resolvedSnapToColumnBoundaries instead of the engine.* globals) so the gesture
finalization uses per-monitor overrides.
🧹 Nitpick comments (2)
Sources/OmniWM/UI/SettingsView.swift (1)

417-420: Disable the toggle when it is a known no-op (Center Focused Column = Always).

This setting won’t take effect in that mode, so disabling it (and optionally adding helper text) would avoid confusing UX in both global and per-monitor sections.

♻️ Suggested UX guard
 Toggle("Snap to Column Boundaries", isOn: $settings.niriSnapToColumnBoundaries)
     .onChange(of: settings.niriSnapToColumnBoundaries) { _, newValue in
         controller.updateNiriConfig(snapToColumnBoundaries: newValue)
     }
+    .disabled(settings.niriCenterFocusedColumn == .always)
+let effectiveCenterFocusedColumn = ms.centerFocusedColumn ?? settings.niriCenterFocusedColumn
 OverridableToggle(
     label: "Snap to Column Boundaries",
     value: ms.snapToColumnBoundaries,
     globalValue: settings.niriSnapToColumnBoundaries,
     onChange: { newValue in updateSetting { $0.snapToColumnBoundaries = newValue } },
     onReset: { updateSetting { $0.snapToColumnBoundaries = nil } }
 )
+.disabled(effectiveCenterFocusedColumn == .always)

Also applies to: 595-601

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/OmniWM/UI/SettingsView.swift` around lines 417 - 420, Disable the
"Snap to Column Boundaries" Toggle when Center Focused Column is set to the
always mode: check the relevant setting (e.g. the Center Focused Column property
or enum value used in SettingsView) and bind the Toggle's .disabled(...) to that
predicate so the control is non-interactive when Center Focused Column ==
.always; keep the existing binding of isOn to
settings.niriSnapToColumnBoundaries and
controller.updateNiriConfig(snapToColumnBoundaries:) for changes, and apply the
same disabled behavior in the per-monitor duplicate Toggle (the other occurrence
referenced around the 595-601 range).
Tests/OmniWMTests/SettingsStoreTests.swift (1)

575-575: Exercise the non-default path (true) in at least one round-trip assertion.

Both touched spots currently validate wiring but not the meaningful “enabled” state. Consider setting this to true in one scenario and asserting it survives import/re-export.

Also applies to: 1673-1673

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Tests/OmniWMTests/SettingsStoreTests.swift` at line 575, The test currently
only validates the false path for the niriSnapToColumnBoundaries setting; update
one round-trip test in SettingsStoreTests to set niriSnapToColumnBoundaries:
true when constructing the settings, perform the import/export (roundTrip) using
the same helper used elsewhere in the file, and add an assertion that the
restored settings' niriSnapToColumnBoundaries is true to ensure the enabled
state survives serialization; locate the relevant round-trip assertion in
SettingsStoreTests (and the similar spot around the other mention) and mirror
the existing pattern but with the value true and a corresponding XCTAssert on
the deserialized object.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.gitignore:
- Line 40: The addition of `.omc/` to .gitignore appears unrelated to the
snap-to-column-boundaries feature; either explain its purpose in this PR or
remove it and add in a separate housekeeping PR: if `.omc/` is required for this
change (e.g., build/cache dir from a tool used by this feature), add a short
note in the PR description and a comment above the entry in .gitignore
referencing that tool; otherwise revert the `.omc/` entry from this branch and
open a dedicated PR that adds `.omc/` with an explanatory commit message.

---

Outside diff comments:
In `@Sources/OmniWM/Core/Controller/MouseEventHandler.swift`:
- Around line 1131-1139: The call to endState.endGesture is passing global
engine flags (engine.centerFocusedColumn, engine.alwaysCenterSingleColumn,
engine.snapToColumnBoundaries) which bypass per-monitor overrides; replace those
three arguments with the monitor-resolved values before calling endGesture
(e.g., obtain the resolved settings for this workspace/viewport from the
workspace/viewport state or a resolver method and pass
resolvedCenterFocusedColumn, resolvedAlwaysCenterSingleColumn,
resolvedSnapToColumnBoundaries instead of the engine.* globals) so the gesture
finalization uses per-monitor overrides.

---

Nitpick comments:
In `@Sources/OmniWM/UI/SettingsView.swift`:
- Around line 417-420: Disable the "Snap to Column Boundaries" Toggle when
Center Focused Column is set to the always mode: check the relevant setting
(e.g. the Center Focused Column property or enum value used in SettingsView) and
bind the Toggle's .disabled(...) to that predicate so the control is
non-interactive when Center Focused Column == .always; keep the existing binding
of isOn to settings.niriSnapToColumnBoundaries and
controller.updateNiriConfig(snapToColumnBoundaries:) for changes, and apply the
same disabled behavior in the per-monitor duplicate Toggle (the other occurrence
referenced around the 595-601 range).

In `@Tests/OmniWMTests/SettingsStoreTests.swift`:
- Line 575: The test currently only validates the false path for the
niriSnapToColumnBoundaries setting; update one round-trip test in
SettingsStoreTests to set niriSnapToColumnBoundaries: true when constructing the
settings, perform the import/export (roundTrip) using the same helper used
elsewhere in the file, and add an assertion that the restored settings'
niriSnapToColumnBoundaries is true to ensure the enabled state survives
serialization; locate the relevant round-trip assertion in SettingsStoreTests
(and the similar spot around the other mention) and mirror the existing pattern
but with the value true and a corresponding XCTAssert on the deserialized
object.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: af6d3e65-1943-4942-9b0c-6a8fda47caab

📥 Commits

Reviewing files that changed from the base of the PR and between 1194ebd and 0ce3325.

📒 Files selected for processing (13)
  • .gitignore
  • Sources/OmniWM/Core/Config/MonitorNiriSettings.swift
  • Sources/OmniWM/Core/Config/SettingsExport.swift
  • Sources/OmniWM/Core/Config/SettingsStore.swift
  • Sources/OmniWM/Core/Controller/MouseEventHandler.swift
  • Sources/OmniWM/Core/Controller/NiriLayoutHandler.swift
  • Sources/OmniWM/Core/Controller/WMController.swift
  • Sources/OmniWM/Core/Layout/Niri/NiriLayoutEngine+Monitors.swift
  • Sources/OmniWM/Core/Layout/Niri/NiriLayoutEngine.swift
  • Sources/OmniWM/Core/Layout/Niri/ViewportState+Gestures.swift
  • Sources/OmniWM/UI/SettingsView.swift
  • Tests/OmniWMTests/NiriLayoutEngineTests.swift
  • Tests/OmniWMTests/SettingsStoreTests.swift

Comment thread .gitignore

# Local documentation
STABILITY_IMPROVEMENTS.md
.omc/
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Clarify the relationship of .omc/ to this PR's objectives.

The addition of .omc/ to .gitignore appears unrelated to the snap-to-column-boundaries feature described in the PR objectives. Can you clarify what .omc/ represents and why it's being added in this PR? If it's unrelated to the feature work, consider moving it to a separate PR to keep changes focused.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.gitignore at line 40, The addition of `.omc/` to .gitignore appears
unrelated to the snap-to-column-boundaries feature; either explain its purpose
in this PR or remove it and add in a separate housekeeping PR: if `.omc/` is
required for this change (e.g., build/cache dir from a tool used by this
feature), add a short note in the PR description and a comment above the entry
in .gitignore referencing that tool; otherwise revert the `.omc/` entry from
this branch and open a dedicated PR that adds `.omc/` with an explanatory commit
message.

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.

3 participants