Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sway: fix workspace 10 missing from default config #4636

Merged
merged 1 commit into from
Feb 10, 2024

Conversation

emilylange
Copy link
Contributor

Description

workspace 10 is part of upstream's default config, but was missing in home-manager.

The initial "sway: add module" PR (#829, 02d6040) went through multiple iterations and had workspace 10 included for a brief moment. Until the author removed it in a force-push commenting

Have removed the last change which added bound ${modifer}+0 to workspace number 10 as this messed up workspace numbering in sway.

#829 (comment)

The reason might have been, that sway used to sort the workspaces in the order they appeared in the config.

Attribute sets in nix are sorted, but not "naturally sorted", meaning bindsym Mod1+0 workspace number 10 comes before bindsym Mod1+0 workspace number 1.

It's unclear if that's what really happened. A workaround would have been to use lib.lists.naturalSort in keybindingsStr.

But I cannot reproduce this anymore in any way.
I assume this has been fixed many years ago by now.

upstream config: https://github.com/swaywm/sway/blob/020a572ed615b8fe272c7566a27ee0abe73a58d7/config.in#L113-L134

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all or nix develop --ignore-environment .#all using Flakes.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

Maintainer CC

@Scrumplex @alexarice @sumnerevans @SebTM @oxalica

workspace 10 is part of upstream's default config, but was missing in home-manager.

The initial "sway: add module" PR (02d6040) went through multiple iterations and had workspace 10 included for a brief moment.
Until the author removed it in a force-push commenting

> Have removed the last change which added bound ${modifer}+0 to workspace number 10 as this messed up workspace numbering in sway.

The reason might have been, that sway used to sort the workspaces in the order they appeared in the config.

Attribute sets in nix are sorted, but not "naturally sorted", meaning `bindsym Mod1+0 workspace number 10` comes before `bindsym Mod1+0 workspace number 1`.

It's unclear if that's what really happened. A workaround would have been to use `lib.lists.naturalSort` in `keybindingsStr`.

But I cannot reproduce this anymore in any way.
I assume this has been fixed many years ago by now.

upstream config: https://github.com/swaywm/sway/blob/020a572ed615b8fe272c7566a27ee0abe73a58d7/config.in#L113-L134
Copy link

stale bot commented Feb 9, 2024

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@stale stale bot added the status: stale label Feb 9, 2024
Copy link
Collaborator

@SebTM SebTM left a comment

Choose a reason for hiding this comment

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

LGTM, another reason it might got left out is that the first defined workspace in keybindings is the workspace active when sway starts, but this can be controlled with defaultWorkspace already.

@emilylange
Copy link
Contributor Author

not stale (trying to get rid of the stale label and not sure if the review does not count for whatever reason -- might delete later, in case you wondered why you can see this comment in your mail inbox but not on github)

@stale stale bot removed the status: stale label Feb 9, 2024
@teto teto merged commit 4c0357f into nix-community:master Feb 10, 2024
@emilylange emilylange deleted the upstream-sway-default-config branch February 10, 2024 19:58
SimSaladin added a commit to SimSaladin/home-manager that referenced this pull request Feb 13, 2024
* commit 'bfd0ae29':
  emacs: use `overrideScope` instead of `overrideScope'`
  flake.lock: Update
  imapnotify: enable STARTTLS if enabled in email account config (nix-community#5013)
  sway: fix workspace 10 missing from default config (nix-community#4636)
  zellij: use full executable path
  jetbrains-remote: add module
  neomutt: fix crypt_use_gpgme in newer versions
  arrpc: add module
  swayosd: update executable
  home-manager: Check VISUAL before EDITOR for editor
  himalaya: adjust module for v1.0.0-beta
  nix-gc: add service
  mcfly: add interfaceView option
  vscode: add openvscode-server
  kitty: always export `KITTY_SHELL_INTEGRATION`
  vdirsyncer: create postHook script when non-empty
  fish: implement shellInitLast (after others)
  flake.lock: Update
  Translate using Weblate (Turkish)
  hyprland: fix hyprctl crash
  tests: reduce closure sizes
  nix: fix generation of nix.conf for nix >= 2.20
  xfconf: fix config loading
  docs: add style sheets and `scrubDerivations`
  tealdeer: add option to toggle update on activation
  home-manager: remove the export of `run`
  wob: add module
  firefox: fix darwin NativeMessagingHostsPath
  flake.lock: Update (nix-community#4964)
  network-manager-applet: add XDG data directory
  firefox: Reimplement FF native messaging
  home-manager: add extendModules attribute
  flake.lock: Update
  firefox: add default containers
  keepassx: remove module
  firefox: implement native messaging hosts
  treewide: deprecate `VERBOSE_ECHO`
  home-manager: avoid running empty `nix profile remove`
  treewide: deprecate `DRY_RUN_CMD` and `DRY_RUN_NULL`
  tealdeer: add cache update activation script
  zoxide: fix nushell 0.89 deprecation
  bemenu: allow floats in settings
  flake.lock: Update
  k9s: fix unnecessary test dependency
  k9s: v0.29/v0.30 compatibility
  mise: fix test
  mise: add module
  hyprland: change plugins settings generation
  mcfly: add mcfly-fzf integration
  hyprland: do not override existing plugins settings in config
  docs, tests: revert to fetchTarball for nmd and nmt
  gh: add github gist to default credential hosts
  sway: include cursor environment variables
  gradle: Don't enable programs.java
  gradle: re-add britter as maintainer
  flake.lock: Update
  flake.lock: Update
  flake.lock: Update
  firefox: restore compatibility for extraPolicies
  xsession: allow xplugd to restart on failure
  flake: update release notes URL
  home-manager: check profile exists in nixProfileRemove
  docs: use nmd from Nixpkgs
  tests: use nmt from Nixpkgs
  Remove some formatting exceptions
  listenbrainz-mpd: use sdnotify when possible
  gh: only run migration when required
  home-manager: internalize uninstall
  docs: use alternative source of nmd
  thunderbird: configure signature if set (nix-community#4852)
  Translate using Weblate (Czech)
  home-manager: update --version to 24.05
  github: fix broken links
  xplr: support multiple plugins in cfg.plugins
  xdg-portal: add new module
  yazi: fix nushell integration
  flake.lock: Update
  zsh: fix zprof typo
  i3blocks: added configuration module
  lorri: unbreak due to too tight sandboxing
  Translate using Weblate (Ukrainian)
  flake.lock: Update
  bemenu: add module
  Translate using Weblate (German)
  zoxide: fix use with recent Nushell
  zsh: add support for zproof
  oh-my-posh: fix test under Darwin
  alacritty: make compatible with alacritty 0.13
  gh: idempotently consider existing symlinks
  sftpman: add module
  zsh.prezto: fix path in example for 'pmoduleDirs'
  osmscout-server: add module
  gpg-agent: don't set a default for pinentry
  gh: test for existence of hosts file
  gh: include version in settings
@Toomoch
Copy link

Toomoch commented Feb 16, 2024

Somehow my Sway desktop is booting to workspace 10 always since I updated my inputs, and I can't seem to know why until I found this commit. Could it be the cause?
Here is my config: https://github.com/Toomoch/nixos-config/blob/testing/home/arnau/sway/default.nix

@SebTM
Copy link
Collaborator

SebTM commented Feb 16, 2024

Yes, but I don't see you using https://nix-community.github.io/home-manager/options.xhtml#opt-wayland.windowManager.sway.config.defaultWorkspace

correct? Then it's handled like explained here: #4636 (review)

@Toomoch
Copy link

Toomoch commented Feb 16, 2024

Yes, but I don't see you using https://nix-community.github.io/home-manager/options.xhtml#opt-wayland.windowManager.sway.config.defaultWorkspace

correct? Then it's handled like explained here: #4636 (review)

So sorry, I didn't even read the whole issue.
That makes sense as number 10 is defined before number 1, see the config file home-manager built:
image
Using defaultWorkspace fixed it, thank you very much.

@Toomoch
Copy link

Toomoch commented Feb 16, 2024

Maybe it would be a good idea to set it to 1 if using mkOptionDefault in the keybindings.

@philipwilk
Copy link
Contributor

Hi, I've run into this as well - my default workspace being changed to 10. This is manageable on single display environments, where you can change the default workspace to be 1 explicitly, but this does not work with multi monitor setups. When i connect my laptop to a monitor, the laptop screen is workspace 1 but the monitor is workspace 10 instead of workspace 2. How could I workaround this?
If there is an option to set the default workspace for a specific monitor or resolution or something, I cannot use it, because I'm not using just one specific monitor; I'm using different ones across the uni, and I can't practically manually create a config for each one.

@emilylange
Copy link
Contributor Author

Guess we could use lib.lists.naturalSort in keybindingsStr (from my opening comment).
Any objections?

@philipwilk
Copy link
Contributor

I'm not sure how to manually test it on my local machine without switching my hm input but i'd be up to try it

@emilylange
Copy link
Contributor Author

hmm turns out throwing lib.lists.naturalSort at keybindingsStr is not as straight forward as I thought :(

@alexarice
Copy link
Collaborator

is it possible to add mkAfter to the workspace 10 bindings?

@Scrumplex
Copy link
Member

Why don't we just set a default value for defaultWorkspace?

@emilylange
Copy link
Contributor Author

Why don't we just set a default value for defaultWorkspace?

Because it would not fix the ordering issue on multi-monitor setups. See #4636 (comment)

colonelpanic8 pushed a commit to colonelpanic8/home-manager that referenced this pull request Dec 30, 2024
workspace 10 is part of upstream's default config, but was missing in home-manager.

The initial "sway: add module" PR (02d6040) went through multiple iterations and had workspace 10 included for a brief moment.
Until the author removed it in a force-push commenting

> Have removed the last change which added bound ${modifer}+0 to workspace number 10 as this messed up workspace numbering in sway.

The reason might have been, that sway used to sort the workspaces in the order they appeared in the config.

Attribute sets in nix are sorted, but not "naturally sorted", meaning `bindsym Mod1+0 workspace number 10` comes before `bindsym Mod1+0 workspace number 1`.

It's unclear if that's what really happened. A workaround would have been to use `lib.lists.naturalSort` in `keybindingsStr`.

But I cannot reproduce this anymore in any way.
I assume this has been fixed many years ago by now.

upstream config: https://github.com/swaywm/sway/blob/020a572ed615b8fe272c7566a27ee0abe73a58d7/config.in#L113-L134
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants