home-manager icon indicating copy to clipboard operation
home-manager copied to clipboard

Kitty allow not setting shell_integration=no_rc

Open MithicSpirit opened this issue 1 year ago • 2 comments

Description

I would appreciate it if there was a way to disable setting shell_integration = no-rc in kitty. This is because I don't have home-manager managing my shell config, and so I want to use kitty's built-in shell integration method, but if no-rc is always enabled this is impossible. It's possible to work around this by setting settings.shell_integration since that makes it appear twice in the config, and it seems like the latter entry overrides the former, but this feels janky.

The solution I've come up with is that manually setting shellIntegration.mode = ""; should make that not show up at all in the config. I believe this (untested) patch should make it behave as such. I can do some testing and submit this as a PR if desired.

diff --git a/modules/programs/kitty.nix b/modules/programs/kitty.nix
index 48ec13da..ffc66c5b 100644
--- a/modules/programs/kitty.nix
+++ b/modules/programs/kitty.nix
@@ -144,20 +144,20 @@ in {
     shellIntegration = {
       mode = mkOption {
         type = types.str;
         default = "no-rc";
         example = "no-cursor";
         apply = (o:
-          let
+          optionalString o (let
             modes = splitString " " o;
             filtered = filter (m: m != "no-rc") modes;
-          in concatStringsSep " " (concatLists [ [ "no-rc" ] filtered ]));
+          in concatStringsSep " " (concatLists [ [ "no-rc" ] filtered ])));
         description = ''
           Set the mode of the shell integration. This accepts the same options
           as the `shell_integration` option of Kitty. Note that
-          `no-rc` is always implied. See
+          `no-rc` is always implied if nonempty. See
           <https://sw.kovidgoyal.net/kitty/shell-integration>
           for more details.
         '';
       };
 
       enableBashIntegration = mkEnableOption "Kitty Bash integration"
@@ -200,16 +200,16 @@ in {
                   "${pkgs.kitty-themes}/share/kitty-themes/themes.json"));
             in throwIf (length matching == 0)
             "kitty-themes does not contain a theme named ${cfg.theme}"
             (head matching).file
           }
         '')
-        ''
+        (optionalString cfg.shellIntegration.mode ''
           # Shell integration is sourced and configured manually
           shell_integration ${cfg.shellIntegration.mode}
-        ''
+        '')
         (toKittyConfig cfg.settings)
         (toKittyKeybindings cfg.keybindings)
         (toKittyEnv cfg.environment)
         cfg.extraConfig
       ]);
     } // optionalAttrs pkgs.stdenv.hostPlatform.isLinux {

MithicSpirit avatar Sep 06 '24 18:09 MithicSpirit

I would rather compare against null then "". PR welcome

teto avatar Oct 07 '24 21:10 teto

Yeah, that's a good idea. I've been pretty busy, but I'll submit the PR when I have time. Likely this weekend.

MithicSpirit avatar Oct 07 '24 22:10 MithicSpirit

Thank you for your contribution! I marked this issue as stale due to inactivity. Please be considerate of people watching this issue and receiving notifications before commenting 'I have this issue too'. We welcome additional information that will help resolve this issue. Please read the relevant sections below before commenting.

If you are the original author of the issue

  • If this is resolved, please consider closing it so that the maintainers know not to focus on this.
  • If this might still be an issue, but you are not interested in promoting its resolution, please consider closing it while encouraging others to take over and reopen an issue if they care enough.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.
If you are not the original author of the issue

  • If you are also experiencing this issue, please add details of your situation to help with the debugging process.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.
Memorandum on closing issues

Don't be afraid to manually close an issue, even if it holds valuable information. Closed issues stay in the system for people to search, read, cross-reference, or even reopen – nothing is lost! Closing obsolete issues is an important way to help maintainers focus their time and effort.

stale[bot] avatar Jan 06 '25 02:01 stale[bot]

PR #5960 should address this issue.

MithicSpirit avatar Jan 06 '25 02:01 MithicSpirit