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

Jump to bottom

plugins/utils/nvim-osc52: add desc option #1704

Closed
wants to merge 1 commit into from
Closed

plugins/utils/nvim-osc52: add desc option #1704

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 20 additions & 9 deletions plugins/utils/nvim-osc52.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,34 +12,40 @@ with lib;

package = helpers.mkPluginPackageOption "nvim-osc52" pkgs.vimPlugins.nvim-osc52;

maxLength = helpers.defaultNullOpts.mkInt 0 "Maximum length of selection (0 for no limit)";
silent = helpers.defaultNullOpts.mkBool false "Disable message on successful copy";
trim = helpers.defaultNullOpts.mkBool false "Trim text before copy";
maxLength = helpers.defaultNullOpts.mkInt 0 "Maximum length of selection (0 for no limit).";
silent = helpers.defaultNullOpts.mkBool false "Disable message on successful copy.";
trim = helpers.defaultNullOpts.mkBool false "Trim text before copy.";

keymaps = {
enable = mkEnableOption "keymaps for copying using OSC52";
enable = mkEnableOption "keymaps for copying using OSC52.";

silent = mkOption {
type = types.bool;
description = "Whether nvim-osc52 keymaps should be silent";
description = "Whether nvim-osc52 keymaps should be silent.";
default = false;
};

desc = mkOption {
type = types.str;
description = "Keymap description.";
default = "Copy using OSC52";
};
Copy link
Member

Choose a reason for hiding this comment

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

I think a better approach here would be to refactor the keymap's option in line with #603 (i.e. using mkMapOptionSubmodule)

If that helper doesn't currently suit this use-case we should consider either making the helper more flexible or changing how the keymaps option here works (breaking changes if absolutely necessary).

That said, this is another plugin that needs a switch to mkNeovimPlugin, so IMO it isn't worth refactoring anything until we make that larger switch.


copy = mkOption {
type = types.str;
description = "Copy into the system clipboard using OSC52";
description = "Copy into the system clipboard using OSC52.";
default = "<leader>y";
};

copyLine = mkOption {
type = types.str;
description = "Copy line into the system clipboard using OSC52";
description = "Copy line into the system clipboard using OSC52.";
default = "<leader>yy";
};

copyVisual = mkOption {
type = types.str;
description = "Copy visual selection into the system clipboard using OSC52";
description = "Copy visual selection into the system clipboard using OSC52.";
default = "<leader>y";
};
};
Expand All @@ -65,6 +71,7 @@ with lib;
action.__raw = "require('osc52').copy_operator";
options = {
expr = true;
desc = "Copy using OSC52";
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this also inherit desc from cfg.keymaps like the others?

inherit silent;
};
}
Expand All @@ -74,14 +81,18 @@ with lib;
action = "${copy}_";
options = {
remap = true;
inherit desc;
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, instead of adding a desc option here, we could use the options.plugins.nvim-osc52.keymaps.*.description value?

Maybe that's a little over-engineered...

Do you think it's ok that all keymaps have the same desc though? Maybe desc could use:

{
  # allow functions or strings
  type = either str (functionTo str);

  # normalise string to a function that ignores its argument
  apply = v: if isStr v then x: v else v;

  # Sane default uses the context provided 
  default = name:
    "Copy "
    + (optionalString (name != "") "${name} ")
    + "using OSC52.";
}

The option would then be a function (str -> str) that maps ["" "visual selection" "line"] to a keymap desc.

inherit silent;
};
}
{
mode = "v";
key = copyVisual;
action.__raw = "require('osc52').copy_visual";
options.silent = silent;
options = {
inherit silent;
inherit desc;
};
}
];

Expand Down