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.

Sign up for GitHub

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: Various improvements (package, module, and documentation) #121925

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

sway: Various improvements (package, module, and documentation) #121925

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a268b5a
nixos/sway: Stop installing rxvt-unicode by default
primeos May 6, 2021
dd97746
nixos/sway: Update the extraPackages examples
primeos May 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions nixos/modules/programs/sway.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,30 @@ in {
type = with types; listOf package;
default = with pkgs; [
swaylock swayidle alacritty dmenu
rxvt-unicode # For backward compatibility (old default terminal)
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if that requires a release notes entry. It's likely annoying if starting a terminal doesn't work anymore but Sway users will hopefully be skilled enough by figuring this out quickly (e.g. resorting to dmenu).

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could add a wrapper that launchers alacritty but displays a warning?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about that but not sure if it's a good idea / if a nice implementation is possible (would have to be a graphical warning and then it get's hacky quickly). Do you have something concrete in mind?

Copy link
Member

Choose a reason for hiding this comment

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

You could display a warning in the shell itself: alacritty -e sh -c 'echo WARN; exec $SHELL'

I don't know exactly how you'd make sure the real rxvt-unicode overrides this fake rxvt-unicode.

Copy link
Member Author

Choose a reason for hiding this comment

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

You could display a warning in the shell itself: alacritty -e sh -c 'echo WARN; exec $SHELL'

That looks and sound super hacky to me (haven't tried if it even works or if exec $SHELL resets the output).
Users could be very confused by this.
And, while I haven't thought about that yet, launching Alacritty for users that expect rxvt-unicode also sounds wrong.
Anyway, if someone ACKs that I'd be ok with that but to me it doesn't really sound like a good idea (the intentions are good but I feel like I'd be super confused).

I don't know exactly how you'd make sure the real rxvt-unicode overrides this fake rxvt-unicode.

There's the lowPrio function ( meta.priority) that should be able to handle that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please no, wofi on nixpkgs master segfaults when I run it.

Copy link
Member

Choose a reason for hiding this comment

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

Great, can you get a backtrace and open an issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

Either way we should add a release notes entry, so people will not be too surprised about this.

How does #127927 look?

Also, wouldn't it be sensible to go with wofi until we have wmenu?

Our programs.sway.extraPackages is "unopinionated", we're simply sticking to the upstream defaults (i.e. Drew's opinion / default configuration). dmenu certainly isn't ideal but is seems like there's still no good enough replacement for it: swaywm/sway#5112

Copy link
Contributor

Choose a reason for hiding this comment

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

Great, can you get a backtrace and open an issue?

The opposite of great. I was under the impression that we don't push broken packages to solicit bug reports.

Copy link
Member

Choose a reason for hiding this comment

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

Great(, that this problem is so reproducible for you), it would probably help you and others if we could report this issue upstream, hopefully with a backtrace (coredumpctl debug).

];
defaultText = literalExample ''
with pkgs; [ swaylock swayidle rxvt-unicode alacritty dmenu ];
with pkgs; [ swaylock swayidle alacritty dmenu ];
'';
example = literalExample ''
with pkgs; [
i3status i3status-rust
termite rofi light
# To run Qt5 applications natively on Wayland:
qt5.qtwayland
# Status bars:
i3status waybar
# Notification daemon:
mako
# Screenshots
grim slurp sway-contrib.grimshot
# dmenu replacements:
bemenu
# Redshift
gammastep wlsunset
# Alternative terminals:
foot
# Backlight management:
brightnessctl
# Clipboard:
wl-clipboard
]
'';
description = ''
Expand Down