Skip to content

Add cycle back feature for presets of column/window width/height.#1670

Merged
YaLTeR merged 2 commits intoniri-wm:mainfrom
two-horned:main
Aug 29, 2025
Merged

Add cycle back feature for presets of column/window width/height.#1670
YaLTeR merged 2 commits intoniri-wm:mainfrom
two-horned:main

Conversation

@two-horned
Copy link
Contributor

@two-horned two-horned commented May 25, 2025

Hello,

this is kind of a DUPLICATE since there's already draft #1367 that attacks issue / discussion #1000, but
it hasn't seen any updates since more than a month! So I hereby request to use this solution.

I simply made the existing functions that switch to the next preset const generics.
The FORWARDS constant can be set to either true or false, and depending on the value
we cycle for- or backwards through the presets.

Code compiles and passes tests. Wiki has been (slightly) updated.

Best Regards
S.K.

@two-horned
Copy link
Contributor Author

Last merge-commit simply synced my fork to avoid future conflicts.

Copy link
Member

@YaLTeR YaLTeR left a comment

Choose a reason for hiding this comment

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

Thanks, some small changes.

// We use const generic IS_WIDTH, because we cannot specifically pass the option as parameter
// without cloning it. This is because we'd need to pass immutable and mutable references,
// which is not possible.
fn toggle_window<const FORWARDS: bool, const IS_WIDTH: bool>(&mut self, id: Option<&W::Id>) {
Copy link
Member

Choose a reason for hiding this comment

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

This does not need to be deduplicated with the function below, at least not in this PR. Not to mention you forgot to update a bunch of the code... So please revert this change, which also gets rid of the IS_WIDTH generic along with the problem in your comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This piece of code was essential to understand cycling forwards/backwards of presets.
Code generation and logic is identical to the previous version, just more readable.

If you want to remove the const generic and use a flag instead I am happy to change that, but it's kinda weird to reintroduce duplicate code again.

Copy link
Member

Choose a reason for hiding this comment

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

Do you mind reintroducing the duplicate functions? It'll result in a smaller PR diff overall too. It's not a big deal, and we can do refactoring separately down the line.

@two-horned two-horned requested a review from YaLTeR August 28, 2025 02:48
@YaLTeR
Copy link
Member

YaLTeR commented Aug 28, 2025

All in all: happy to see the first review after a little more than 3 months

Sorry, but there's a lot of open PRs, and I'm also fairly busy outside niri (and have been traveling).

Anyway, I think we can get this in before the release this Saturday.

@YaLTeR YaLTeR self-assigned this Aug 29, 2025
@YaLTeR
Copy link
Member

YaLTeR commented Aug 29, 2025

Alright, I fixed it and finished it up. (Sidenote, the de-duplicated code did in fact still contain mistakes like using width in both width and height cases, even after the latest push.)

Also while testing, managed to find a small bug in preset height handling for floating windows (in a separate commit).

@YaLTeR YaLTeR merged commit dfe463e into niri-wm:main Aug 29, 2025
13 checks passed
@YaLTeR
Copy link
Member

YaLTeR commented Aug 29, 2025

Thanks

@two-horned
Copy link
Contributor Author

Hey, just recently saw your messages.
Thanks for merging this feature.

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.

2 participants