-
-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
termonad: Add wrapper #50810
termonad: Add wrapper #50810
Conversation
c5f2cb5
to
12c32cf
Compare
Failure on x86_64-darwin (full log) Attempted: termonad Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: termonad Partial log (click to expand)
|
Timed out, unknown build status on aarch64-linux (full log) Attempted: termonad Partial log (click to expand)
|
pkgs/top-level/all-packages.nix
Outdated
@@ -6483,6 +6483,11 @@ with pkgs; | |||
|
|||
rush = callPackage ../shells/rush { }; | |||
|
|||
termonad = callPackage ../shells/termonad { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably not be placed in the section with other shells, but instead with other terminal emulators. It should go near where roxterm
, sakura
, etc are defined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I think this should maybe be called termonad-with-packages
(to match the xmonad-with-packages
derivation).
Or, maybe keep this as termonad
and at some point try to get xmonad-with-packages
to be called just xmonad
. I think this would be the better solution, but there will probably be some pushback from people who have been using xmonad-with-packages
for a long time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll move it.
I figured if you wanted "termonad-without-packages", you could just grab it from "haskellPackages", so no need to suffix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think termonad
is the better name as well.
However, in this case I think it might be better to keep a similar naming scheme with other similar packages (xmonad-with-packages
is the only one I know of), rather than using something different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll rename it and then I think we should be good to go
pkgs/shells/termonad/default.nix
Outdated
@@ -0,0 +1,19 @@ | |||
{ stdenv, ghcWithPackages, makeWrapper, packages }: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be great to have a default argument on packages
. Something like (pkgSet: [])
. That way people wouldn't necessarily have to give a packages
argument.
It would also be nice to have a little documentation at the top of this file explaining what the packages
argument is and some examples of passing stuff to it.
I'd say most(?) Termonad users will want the colour
and lens
packages to be available for use in their Termonad config file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to avoid people pulling in packages they don't need.
If you need colour and lens, you know it and can add them :)
I'll default to an empty list.
@NeQuissimus Thanks for putting together this PR! |
@jtojnar Okay, that sounds great! Thanks for your work on the gnome stuff. |
12c32cf
to
1934ba3
Compare
Success on x86_64-linux (full log) Attempted: termonad Partial log (click to expand)
|
Timed out, unknown build status on x86_64-darwin (full log) Attempted: termonad Partial log (click to expand)
|
Timed out, unknown build status on aarch64-linux (full log) Attempted: termonad Partial log (click to expand)
|
1934ba3
to
3dd96c6
Compare
Motivation for this change
Fixes cdepillabout/termonad#34
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)/cc @cdepillabout @puffnfresh