search

YaLTeR / niri

A scrollable-tiling Wayland compositor.
https://matrix.to/#/#niri:matrix.org
GNU General Public License v3.0
4.14k stars 123 forks source link

Allow creation of new workspace above top most workspace #745

Open FluxTape opened 1 month ago

FluxTape commented 1 month ago

Add the layout option allow-workspace-above-first to allow adding new workspaces above the top most workspace. When this option is enabled the first and last workspace on a monitor are always empty (compared to just the last one) allowing the movement of windows/columns above the first workspace.

FluxTape commented 1 month ago

I've made the necessary changes to verify_invariants() and fixed any issues that popped up. Anything else that needs changing?

FluxTape commented 2 weeks ago

Implemented all your suggestions, let me know if there are any more issues

YaLTeR commented 2 weeks ago

I'll take it from here, thanks.

YaLTeR commented 2 weeks ago

If you don't mind, I will postpone this to after the upcoming release. The logic has some remaining bugs, which are not difficult to fix, but they cannot get caught by the current testing system. I want to extend the testing system a bit to make sure they are caught and that there are no other similar bugs left.

YaLTeR commented 2 weeks ago

So that I don't forget, the current bugs are:

  1. Interactive move uses add_tile() but assumes the workspace idx doesn't change, which it does if a workspace is added at the top. Possibly similar bugs elsewhere in the code. Not caught because the testing system needs to be able to advance animations to complete a workspace switch up.
  2. Toggling the option at runtime doesn't add or remove the topmost workspace. Not caught because there are no tests that change the options.
FluxTape commented 1 week ago

So that I don't forget, the current bugs are:

1. Interactive move uses add_tile() but assumes the workspace idx doesn't change, which it does if a workspace is added at the top. Possibly similar bugs elsewhere in the code. Not caught because the testing system needs to be able to advance animations to complete a workspace switch up.

2. Toggling the option at runtime doesn't add or remove the topmost workspace. Not caught because there are no tests that change the options.

To fix 2) it should be enough to add 

        if self.options.empty_workspace_above_first != options.empty_workspace_above_first
            && self.workspaces.len() > 1
        {
            if options.empty_workspace_above_first {
                self.add_workspace_top();
            } else if self.workspace_switch.is_none() {
                self.workspaces.remove(0);
                self.active_workspace_idx = self.active_workspace_idx.saturating_sub(1);
            }
        }

to the top of update_config(..) in monitor.rs

YaLTeR commented 2 days ago

Working on advancing animations in the test system in #825.