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

[BUG] plugins.leap: safeLabels = [] does not disable auto-jump #1698

Open
1 task done
fin444 opened this issue Jun 14, 2024 · 9 comments
Open
1 task done

[BUG] plugins.leap: safeLabels = [] does not disable auto-jump #1698

fin444 opened this issue Jun 14, 2024 · 9 comments
Labels
bug Something isn't working

Comments

@fin444
Copy link

fin444 commented Jun 14, 2024

Field Description
Plugin leap
Nixpkgs unstable e8057b6 (June 5)
Home Manager N/A
  • I have read the FAQ and my bug is not listed there.

Description

According to the documentation, setting plugins.leap.safeLabels = [] should disable auto-jump, however it does not.

Minimal, Reproducible Example (MRE)

programs.nixvim = {
  enable = true;
  plugins.leap = {
    enable = true;
    safeLabels = [];
  };
};

Working Example of Auto-Jump Disabled

programs.nixvim = {
  enable = true;
  extraPlugins = [ pkgs.vimPlugins.leap-nvim ];
  extraConfigLua = ''
    require("leap").add_default_mappings()
    require("leap").opts.safe_labels = {}
  '';
};
@fin444 fin444 added the bug Something isn't working label Jun 14, 2024
@GaetanLepage
Copy link
Collaborator

GaetanLepage commented Jun 25, 2024

Indeed, empty lists and attrs are ignored during the translation to lua.
If you want to explicitly have an empty lua table, you can do the following:

setLabels.__empty = null;

@MattSturgeon
Copy link
Collaborator

MattSturgeon commented Jun 25, 2024

If we ever get to a point where all settings options (inc attrs and lists) are default-null, we might be able to change the behaviour to only omit null attrs?

Maybe we could have a configurable toLuaObject in the meantime where we can toggle behaviours on a per-plugin/per-usage basis? 🤔

helpers.toLuaObject'
  {
    omitNullAttrs = true;
    omitEmptyAttrs = false;
    multiline = false;
  }
  {
    someNullAttr = null;
    someEmptyAttrsAttr = { };
    someEmptyListAttr = [ ];
  }
=>
''
  { ["someEmptyAttrsAttr"] = { }, ["someEmptyListAttr"] = { } }
''

@traxys
Copy link
Member

traxys commented Jun 25, 2024

I think there used to be a reason for empty tables, I remember @GaetanLepage had done some stuff around that some time ago. A good test would be removing the empty attrs from a full config, and seeing how it increases.

@GaetanLepage
Copy link
Collaborator

I think there used to be a reason for empty tables, I remember @GaetanLepage had done some stuff around that some time ago. A good test would be removing the empty attrs from a full config, and seeing how it increases.

At some point, we did made the switch to stop ignoring empty attrs. It turned out to be bad. We had to rely on submodules (through helpers.mkCompositeOption) everywhere we needed nested options.
We rolled-back this change.

@fin444
Copy link
Author

fin444 commented Jun 29, 2024

Indeed, empty lists and attrs are ignored during the translation to lua. If you want to explicitly have an empty lua table, you can do the following:

setLabels.__empty = null;

Unfortunately safeLabels takes a list, not a table, so this is not an option

@MattSturgeon
Copy link
Collaborator

In lua, lists and dicts are both tables.

@MattSturgeon
Copy link
Collaborator

You can also do safeLabels.__raw = "{ }" to do it explicitly in raw lua if __empty isn't working for some reason

@fin444
Copy link
Author

fin444 commented Jun 29, 2024

On the Nix side, safeLabels is a list. So you can't set __raw or __empty

@MattSturgeon
Copy link
Collaborator

Can confirm, the type is nullOr (listOf (maybeRaw str)), so neither __raw or __empty can be used.

This is caused by the underlying issue @traxys brought up in an unrelated discussion: defaultNullOpts.mkListOf does not apply maybeRaw to the list itself, it is only applied to list elements (same for mkAttrsOf`).

The fact that __empty can't be used with list-type options is also unfortunate. I think the only way we'd get around that is with a custom listOf type, with an extended check function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants