plugins/wezterm: init #2597
plugins/wezterm: init #2597
Conversation
samos667
force-pushed
the
init-wezterm
branch
3 times, most recently
from
a49ca07 to
191626d
Compare
samos667
force-pushed
the
init-wezterm
branch
from
1c6db30 to
7f4f569
Compare
samos667
force-pushed
the
init-wezterm
branch
2 times, most recently
from
5cfbfba to
dbd8645
Compare
|
@samos667 We updated flake lock today so you should be able to rebase this, now. |
samos667
force-pushed
the
init-wezterm
branch
from
dbd8645 to
d5832f2
Compare
samos667
force-pushed
the
init-wezterm
branch
from
3693f17 to
12ac032
Compare
Sadge :'( |
If you're not in nixpkgs maintainers, you can add yourself to our maintainers file in another commit. |
|
We also have a test failure: Perhaps the nixpkgs package doesn't have wezterm marked as a dependency? |
MattSturgeon
force-pushed
the
init-wezterm
branch
2 times, most recently
from
11c4a25 to
e1c0570
Compare
|
The following patch causes the test to pass, however I believe this should be fixed upstream in nixpkgs. diff --git a/plugins/by-name/wezterm/default.nix b/plugins/by-name/wezterm/default.nix
index aecb332e..e95b36a4 100644
--- a/plugins/by-name/wezterm/default.nix
+++ b/plugins/by-name/wezterm/default.nix
@@ -1,4 +1,4 @@
-{ lib, ... }:
+{ lib, pkgs, ... }:
let
inherit (lib.nixvim) defaultNullOpts;
in
@@ -9,6 +9,10 @@ lib.nixvim.neovim-plugin.mkNeovimPlugin {
maintainers = [ lib.maintainers.samos667 ];
+ extraPackages = [
+ pkgs.wezterm
+ ];
+
settingsOptions = {
create_commands = defaultNullOpts.mkBool true ''
Whether to create plugin commands.Or if not*, then we should have a nullable weztermPackage = lib.mkPackageOption pkgs "wezterm" {
nullable = true;
# maybe also `default = null` and `example = [ "wezterm" ]`?
};* Perhaps nixpkgs decide that bundling the dep leads to too many potential issues, such as if users have an overridden/custom wezterm installed to their PATH? |
This is still something we have not clearly agreed on in nixpkgs... |
Co-authored-by: Austin Horstman <[email protected]> Co-authored-by: Gaétan Lepage <[email protected]>
MattSturgeon
force-pushed
the
init-wezterm
branch
from
d4053a1 to
a68354e
Compare
| weztermPackage = lib.mkPackageOption pkgs "wezterm" { | ||
| nullable = true; | ||
| default = null; | ||
| example = [ "wezterm" ]; | ||
| }; |
There was a problem hiding this comment.
What's the best default here? The package or null?
I'm assuming most users who enable this plugin will already have wezterm installed 🤔
There was a problem hiding this comment.
Usually we set the package.
There was a problem hiding this comment.
This would achieve that:
| weztermPackage = lib.mkPackageOption pkgs "wezterm" { | |
| nullable = true; | |
| default = null; | |
| example = [ "wezterm" ]; | |
| }; | |
| weztermPackage = lib.mkPackageOption pkgs "wezterm" { | |
| nullable = true; | |
| }; |
Is there any reason not to do that here?
|
@samos667 I've pushed up a couple tweaks. Let us know if you're happy and we should be good to approve & merge. |
| ] | ||
| '' | ||
| How images are displayed. | ||
|
|
||
| If set to `"wezterm"`, `plugins.wezterm.enable` must be true. |
There was a problem hiding this comment.
Why not adding a warning to check this ? This is usually what we do.
|
Seems good to me ! |
|
I believe @GaetanLepage is hoping you'll change the following before we approve/merge:
|
This pull request introduces a new Neovim plugin for
weztermand integrates it into the existing plugin system. The changes include adding theweztermplugin to themoltenplugin list, defining theweztermplugin in its own file, and adding test cases for the new plugin.New Plugin Addition:
plugins/by-name/molten/default.nix: Added theweztermplugin to the list of plugins and enabled it by default.plugins/by-name/wezterm/default.nix: Defined theweztermplugin with its settings and maintainers.Testing:
tests/test-sources/plugins/by-name/wezterm/default.nix: Added test cases for theweztermplugin to ensure it is enabled and its settings are correctly applied.fixes #2596