Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

plugins/dap-rr: init #3089

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

GaetanLepage
Copy link
Member

Add support for nvim-dap-rr, nvim-dap extension for the record and replay debugger.

Fixes #894

Comment on lines +64 to +60
rust = lib.mkDefault [ { __raw = "require('nvim-dap-rr').get_rust_config()"; } ];
cpp = lib.mkDefault [ { __raw = "require('nvim-dap-rr').get_config()"; } ];
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we set up the configurationType so that a string would be automatically converted to lua?

Copy link
Member

@MattSturgeon MattSturgeon Mar 20, 2025

Choose a reason for hiding this comment

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

Initial reaction: I'd assume not, since we want to gradually move away from such implicit conversion?

Saying that, this is a non-freeform option (right?), so we can design it to be more helpful if we wanted.

If we aren't trying to exactly represent an upstream setting and it's unlikely that non-raw string values will ever be valid, then treating strings as lua is fine IMO.

Copy link
Member Author

Choose a reason for hiding this comment

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

Saying that, this is a non-freeform option (right?), so we can design it to be more helpful if we wanted.

If we aren't trying to exactly represent an upstream setting and it's unlikely that non-raw string values will ever be valid, then treating strings as lua is fine IMO.

This was exactly my reasoning.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I gave it a try and it adds more complexity than I thought.
As dap-go and dap-python both use this type, we would need to port the conversion logic there too.
Maybe it is better to stick to using the rawType straight.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, if we did this we'd want to construct two types; one for use in freeform settings and another one that can be used in nixvim's own options.

Both types could use the same modules in their submodule-type though.

@GaetanLepage
Copy link
Member Author

I opened jonboh/nvim-dap-rr#9 to deal with the non-initialization of opts.mappings in setup().

@MattSturgeon
Copy link
Member

I opened jonboh/nvim-dap-rr#9 to deal with the non-initialization of opts.mappings in setup().

Does that block this PR?

@GaetanLepage
Copy link
Member Author

I opened jonboh/nvim-dap-rr#9 to deal with the non-initialization of opts.mappings in setup().

Does that block this PR?

Nope, I declared settings.integrations with { __empty = null; } as default, so it doesn't crash.

Comment on lines 15 to 17
# Without providing a `mappings` table, the `setup` function crashes with:
# bad argument #1 to 'pairs' (table expected, got nil)
default.__empty = null;
Copy link
Member

@MattSturgeon MattSturgeon Mar 20, 2025

Choose a reason for hiding this comment

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

Have you double checked that __empty doesn't get merged with normal attr definitions?

Normally option-default prio definitions aren't merged with normal prio definitions, but I can't recall whether attrsOf handles that differently.

If the __empty is merged into the final value then toLuaObject will always print "{}" regardless of what other attrs are in the value.

@GaetanLepage GaetanLepage marked this pull request as draft March 20, 2025 15:44
@jonboh
Copy link
Contributor

jonboh commented Mar 21, 2025

I've set this branch as the input of my config flake, but I get an error building the nixvim config:

error: attribute 'attrsOf' missing
       at /nix/store/clsmijxr35nxw1j5xir5kb6mrpkcmrsz-source/plugins/by-name/dap-rr/default.nix:15:7

I get the same error running checks on your latest commit (in the nixvim repo)

@GaetanLepage
Copy link
Member Author

I've set this branch as the input of my config flake, but I get an error building the nixvim config:

error: attribute 'attrsOf' missing
       at /nix/store/clsmijxr35nxw1j5xir5kb6mrpkcmrsz-source/plugins/by-name/dap-rr/default.nix:15:7

I get the same error running checks on your latest commit (in the nixvim repo)

It should be good now.

@MattSturgeon
Copy link
Member

Empty test case is failing:

empty> ERROR: Error detected while processing /nix/store/1mw23g71qhja1y2vhhagbr9njaxql94p-init.lua:
empty> E5113: Error while calling lua chunk: ...k/myNeovimPackages/start/nvim-dap-rr/lua/nvim-dap-rr.lua:274: bad argument #1 to 'pairs' (table expected, got nil)
empty> stack traceback:
empty>  [C]: in function 'pairs'
empty>  ...k/myNeovimPackages/start/nvim-dap-rr/lua/nvim-dap-rr.lua:274: in function 'setup'
empty>  /nix/store/1mw23g71qhja1y2vhhagbr9njaxql94p-init.lua:12: in main chunk

@GaetanLepage
Copy link
Member Author

Empty test case is failing:

empty> ERROR: Error detected while processing /nix/store/1mw23g71qhja1y2vhhagbr9njaxql94p-init.lua:
empty> E5113: Error while calling lua chunk: ...k/myNeovimPackages/start/nvim-dap-rr/lua/nvim-dap-rr.lua:274: bad argument #1 to 'pairs' (table expected, got nil)
empty> stack traceback:
empty>  [C]: in function 'pairs'
empty>  ...k/myNeovimPackages/start/nvim-dap-rr/lua/nvim-dap-rr.lua:274: in function 'setup'
empty>  /nix/store/1mw23g71qhja1y2vhhagbr9njaxql94p-init.lua:12: in main chunk

This should go away once #3094 is merged.

Copy link
Contributor

@jonboh jonboh left a comment

Choose a reason for hiding this comment

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

I've tested it in one of my projects with this config and everything seems to work fine.

  plugins.dap-rr = {
    enable = true;
    settings.mappings = {
      continue = "<F10>"; # FRight
      step_over = "<F1>"; # FDown
      step_out = "<F2>"; # FUp
      step_into = "<F3>"; # FRight
      reverse_continue = "<F22>"; # <S-FLeft>
      reverse_step_over = "<F13>"; # <S-FDown>
      reverse_step_out = "<F14>"; # <S-FUp>
      reverse_step_into = "<F15>"; # <S-FRight>
      step_over_i = "<F25>"; # <C-FDown>
      step_out_i = "<F26>"; # <C-FUp>
      step_into_i = "<F27>"; # <C-FRight>
      reverse_step_over_i = "<F37>"; # <SC-FDown>
      reverse_step_out_i = "<F38>"; # <SC-FUp>
      reverse_step_into_i = "<F39>"; # <SC-FRight>
    };
  };

@GaetanLepage
Copy link
Member Author

The fix has made its way to our version of nixpkgs. This should now work just fine.

@GaetanLepage GaetanLepage marked this pull request as ready for review March 23, 2025 00:05
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PLUGIN REQUEST] nvim-dap-rr
3 participants