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

nixos/gnome-terminal: init #66990

Merged
merged 2 commits into from Aug 21, 2019

Conversation

@worldofpeace
Copy link
Member

commented Aug 19, 2019

This module obsoletes services.gnome3.gnome-terminal-server
as that's a confusing option for users, and sounds internal.
It's much simpler to have a gnome-terminal module.

This module also correctly includes the vte.sh script
required for vte terminals like gnome-terminal to show the
CWD in the window title and preserved across instances.

This is achieved with the options:

  • programs.bash.vteIntegration
  • programs.zsh.vteIntegration

as it's best to keep this configuration unguarded by gnome3.enable
to support other vte terminals (such as elementary-terminal).
Note that the distinction between Zsh and Bash doesn't include
a different script, as this script only supports those two shells.

Motivation for this change

Fixes #40878.

We should probably check if other desktop managers default include a vte terminal and enable
the appropriate option.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@worldofpeace worldofpeace requested a review from jtojnar Aug 19, 2019
@worldofpeace worldofpeace requested a review from Infinisil as a code owner Aug 19, 2019
@jtojnar

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

There is a lot of non-DE VTE-based terminals, is this needed for them?

Is gnome-terminal-server used by something other than GNOME Terminal?

@worldofpeace

This comment has been minimized.

Copy link
Member Author

commented Aug 19, 2019

Is gnome-terminal-server used by something other than GNOME Terminal?

I considered this, and the fact that gnome terminal's server isn't a separate package entirely makes me believe this is unlikely and that we shouldn't support it.

@worldofpeace

This comment has been minimized.

Copy link
Member Author

commented Aug 19, 2019

There is a lot of non-DE VTE-based terminals, is this needed for them?

Yes I believe that any terminal that wants #40878 feature (cwd in title and preserved across instances) needs this. Do we need documentation?

@jtojnar

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

Yes I believe that any terminal that wants #40878 feature (cwd in title and preserved across instances) needs this.

I would suggest to split it to a VTE module to make it easier to find.

Do we need documentation?

Yeah, for the VTE stuff we probably should write something. Maybe I could add it to the “Creating a custom Freedesktop-like DE” chapter I am meaning to write for a manual.

@worldofpeace

This comment has been minimized.

Copy link
Member Author

commented Aug 19, 2019

would suggest to split it to a VTE module to make it easier to find.

I didn't really like that idea too strongly, as vte is just a terminal widget for GTK.
So it would be a super "for this" module which I'm not fond of.

See #40878 (comment)
It'd be

  • programs.vte.enable*Integration (not a program)
  • vte.enable*Integration

I found environment.gnome3.enable*Integration to appear more pleasant, but if you have suggestions I'd be happy to hear them.

@worldofpeace

This comment has been minimized.

Copy link
Member Author

commented Aug 19, 2019

I guess environment.vte.enable*Integration is the best I can think of.

Edit: even better is programs.$shell.vteIntegration

})

(mkIf config.programs.bash.vteIntegration {
programs.bash.interactiveShellInit = vteInitSnippet;

This comment has been minimized.

Copy link
@Infinisil

Infinisil Aug 20, 2019

Member

How about just a single programs.shell.vteIntegration (or so) which then does

mkIf vteIntegration {
  programs.bash.interactiveShellInit = vteInitSnippet;
  programs.zsh.interactiveShellInit = vteInitSnippet;
}

The user already controls whether bash or zsh is enabled with programs.{bash,zsh}.enable

This comment has been minimized.

Copy link
@worldofpeace

worldofpeace Aug 20, 2019

Author Member

The user already controls whether bash or zsh is enabled with programs.{bash,zsh}.enable

Right, so even if zsh isn't enabled programs.zsh.interactiveShellInit doesn't do anything unneeded.

However I feel like this would be slightly dissonant with the programs.{bash,zsh} interfaces, we could eliminate one extra option but I don't think programs.shell.* would have any more use.

@worldofpeace worldofpeace added this to In Progress in GNOME Aug 20, 2019
@worldofpeace worldofpeace force-pushed the worldofpeace:gnome-vte-config branch 3 times, most recently from 23ed69e to 8fb0bbe Aug 21, 2019
@worldofpeace

This comment has been minimized.

Copy link
Member Author

commented Aug 21, 2019

@jtojnar Split them up, the vte stuff is under the config/ directory.

nixos/modules/config/vte.nix Outdated Show resolved Hide resolved
This module correctly includes the vte.sh script
required for vte terminals like gnome-terminal to show the
CWD in the window title and preserved across instances.

This is achieved with the options:
* programs.bash.vteIntegration
* programs.zsh.vteIntegration

as it's best to keep this configuration unguarded by gnome3.enable
to support other vte terminals (such as elementary-terminal).
Note the distinction between Zsh and Bash doesn't include
a different script, as this script only supports those two shells.
This module obsoletes services.gnome3.gnome-terminal-server
as that's a confusing option for users, and sounds internal.
It's much simpler to have a gnome-terminal module.
@worldofpeace worldofpeace force-pushed the worldofpeace:gnome-vte-config branch from bd04ba2 to 4a46140 Aug 21, 2019
@worldofpeace worldofpeace merged commit 4ba10fb into NixOS:master Aug 21, 2019
GNOME automation moved this from In Progress to Done Aug 21, 2019
@worldofpeace worldofpeace deleted the worldofpeace:gnome-vte-config branch Aug 21, 2019
@worldofpeace

This comment has been minimized.

Copy link
Member Author

commented Aug 21, 2019

Thanks @jtojnar, going to fix things up for other DE's (mostly pantheon) with the vte module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
GNOME
  
Done
3 participants
You can’t perform that action at this time.