Repository navigation
feat(layout): add maximize_to_edges option - #383
AdityaKr015 wants to merge 8 commits into
Conversation
|
Blocking: enabling the policy can make ordinary maximized windows impossible to unmaximize At src/view/view_state.cpp:214-216, maximize_to_edges = true routes both maximize and unmaximize requests to setMaximizedToEdges(). For a column-maximized window, m_maximizedToEdges is false, so setMaximizedToEdges(false) returns immediately at lines 228-229 without clearing column maximization. Reproduced with a real Wayland client in an isolated headless compositor:
In each case, the window remained column-maximized at width 1200, rather than restoring to 588. Proposed fix: apply the configured policy only to maximize requests. Route unmaximize requests according to the active maximize mode. Add transition coverage for these cases; Nit: tests/unit/maximize.cpp:13 spells RequestsOnEdges as RequestsonEdges. |
Can you tell if everything alright now? |
|
Blocking: a restored maximize lands in a different state depending on client timing.
Nit: docs say floating windows get the edges behavior, but the code only does it for Nit: the key-table row is missing. maximize_to_edges was added to the TOML block at Nit: indentation. client_maximize_to_edges.sh:137: await_lines has two extra spaces of |
|
Blocking: a Throwaway harness probe (tiled 1280x720 output,
Proposed fix, pick one:
Either way, add a ruled before-map case and a ruled after-map case to Nit: at |
There was a problem hiding this comment.
Talia preliminary review
| Check | Result |
|---|---|
| Merge score | 🟡 3/5 — Minor changes needed before merging |
| Findings | 🟡 1 medium |
| Review depth | 4/5 — how thoroughly the changed code could be checked by reading it |
| Coverage | 12 of 12 changed files reviewed |
What Talia checked: Read the changed helper and its only call site, both opening-state paths (handleMap/handleCommit), the config registry wiring, unit/harness expectations, and the rule paths that apply default_maximize; code paths are self-consistent, and the remaining defect is the added window-rules doc claim, which is unchecked at runtime.
Findings
| # | Severity | Finding | Location |
|---|---|---|---|
| 1 | 🟡 P2 | window-rules doc claims [layout] maximize_to_edges chooses a rule-opened window's mode | docs/user/window-rules.md:63 |
Each finding is explained in a review comment on its line.
Run details
- Commit reviewed: d7be0e2
- Source lookups: 36 file reads, 28 searches
- Prompt version: 859b7490ca3d
Preliminary automated review: Talia read the changed code and related source; nothing was built, run or tested, and AI can make mistakes. It is not a scope acceptance or a promise to merge; a human reviewer makes the final call. See our ethos.
| Later identity changes refresh only the settings that update while a window is | ||
| open. | ||
|
|
||
| This rule decides *that* a window opens maximized [`maximize_to_edges`](layout.md#maximize-and-fullscreen) |
There was a problem hiding this comment.
🟡 P2 (medium) · window-rules doc claims [layout] maximize_to_edges chooses a rule-opened window's mode
The added sentence says `default_maximize` decides *that* a window opens maximized and `maximize_to_edges` decides *whether* that is the column or the edges. The implementation only consults `maximizeToEdges` for a window whose client itself requests/restores maximize: the edges branch in `src/view/view_lifecycle.cpp:378` requires `restoredMaximized`, and every rule-only path calls `setMaximized(true)` (column) (`src/view/view_lifecycle.cpp:381`, `:387` for `default_maximize_to_edges`, `src/view/view_rules.cpp:535`). The PR's own `docs/user/layout.md:378-379` says "A `default_maximize` window that never sees a client request still opens the column", so a user with only `default_maximize` rules and `maximize_to_edges = true` still gets the column, contradicting this sentence. Fix: state that the option governs the client's own/restored maximize request, and that a rule-only maximize stays on the column unless `default_maximize_to_edges` is set.
Talia preliminary review · maintainability
Summary
This PR adds
[layout] maximize_to_edges(defaultfalse). When enabled, a client's own maximize request fills the usable area without struts, gaps, or borders instead of the full column width.Motivation
Clicking an app's maximize button currently maximizes to full column width.
Type of Change
Related Issue
Closes #227
Testing
❯ just format && just test && just lintmaximize_to_edges = true: Firefox's maximize button fills the screen edges with no gap or border, unmaximize restores the previous tile.Manual Coverage>
Screenshots / Videos
Checklist
SCOPE.md, or its scope was agreed in an issue or on Discord first.CONTRIBUTING.md.just format, or this PR has no C++ changes.docs/andexamples/config.toml, or this PR does not change user-facing configuration or behavior.