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

refactor: make debug functionality better. #777

Merged
merged 18 commits into from
Jun 13, 2023
Merged

refactor: make debug functionality better. #777

merged 18 commits into from
Jun 13, 2023

Conversation

ayamir
Copy link
Owner

@ayamir ayamir commented May 31, 2023

Dap for c/cpp/rust:
We use lldb-vscode to debug, but I found that it doesn't work on my arch wsl2 although I have set the lldb-vscode's path to absolute path, ref: https://github.com/mfussenegger/nvim-dap/wiki/Debug-Adapter-installation#ccrust-via-lldb-vscode
It needs more test on other platforms.
Dap for python:
It works now but needs user install debugpy manually.
ref: https://github.com/mfussenegger/nvim-dap/wiki/Debug-Adapter-installation#Python

@ayamir

This comment was marked as outdated.

@CharlesChiuGit
Copy link
Collaborator

Debug for c/cpp/rust can work when using cpptools instead.

I'm considering whether add an installation option for dap adapters like cpptools and debugpy in install.sh and install.ps1. Now I put the executable path in ~/.local/share/nvim/dap/.

cc @Jint-lzxy @CharlesChiuGit

I think that's a gd idea, to save some newcomers from confusing.

@Groveer
Copy link
Contributor

Groveer commented Jun 1, 2023

Dap for c/cpp/rust: We use lldb-vscode to debug, but I found that it doesn't work on my arch wsl2 although I have set the lldb-vscode's path to absolute path, ref: https://github.com/mfussenegger/nvim-dap/wiki/Debug-Adapter-installation#ccrust-via-lldb-vscode It needs more test on other platforms. Dap for python: It works now but needs user install debugpy manually. ref: https://github.com/mfussenegger/nvim-dap/wiki/Debug-Adapter-installation#Python

lldb-vscode works for me, and I do't need absolute path, I use env path.

图片
图片

@ayamir ayamir changed the title fix: make dap for c/cpp/rust/python works. refactor: make debug functionality better. Jun 11, 2023
@ayamir
Copy link
Owner Author

ayamir commented Jun 11, 2023

screensht_23-06-12_01:18:07
screensht_23-06-12_01:28:23
screensht_23-06-12_01:28:54

@ayamir ayamir marked this pull request as ready for review June 11, 2023 17:30
@ayamir ayamir requested review from ClSlaid, aarnphm, CharlesChiuGit and Jint-lzxy and removed request for ClSlaid June 11, 2023 17:30
Copy link
Collaborator

@aarnphm aarnphm left a comment

Choose a reason for hiding this comment

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

The DAP implementation LGTM. I used to use DAP a lot, not anymore. Still cc others to take a look

@Groveer
Copy link
Contributor

Groveer commented Jun 12, 2023

LGTM. It works good for me.

Copy link
Collaborator

@CharlesChiuGit CharlesChiuGit left a comment

Choose a reason for hiding this comment

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

LGTM, but I only tested on python

@Jint-lzxy
Copy link
Collaborator

Sorry for the delay. Will test on C++, Objective-C, and Swift.

Copy link
Collaborator

@Jint-lzxy Jint-lzxy left a comment

Choose a reason for hiding this comment

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

LGTM, currently working on several cleanups like #458 (comment)

@Jint-lzxy
Copy link
Collaborator

Jint-lzxy commented Jun 12, 2023

A small change in wording: Executables with LSP-like capabilities are called "servers", while executables having DAP implementations are called "clients".

  • LSP: Language Server Protocol
  • DAP: Debug Adapter Protocol

@Jint-lzxy Jint-lzxy requested review from CharlesChiuGit and removed request for ClSlaid June 12, 2023 13:44
Copy link
Collaborator

@Jint-lzxy Jint-lzxy left a comment

Choose a reason for hiding this comment

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

LGTM for codelldb and lldb 👍

@Jint-lzxy
Copy link
Collaborator

1d9381c implemented a dirty workaround for utils (all parameters passed to the adapter should be functions or tables, and they shouldn't be called during setup):

return setmetatable({}, {
	__index = function(_, key)
		return function()
			return function()
				return M[key]()
			end
		end
	end,
})

Not sure if this is the best way to do so 😢

@ayamir ayamir requested a review from ClSlaid June 13, 2023 11:59
@ayamir
Copy link
Owner Author

ayamir commented Jun 13, 2023

We use project.nvim to change cwd automatically, so is it suitable to use vim.fn.getcwd() as default path completion? For example, I have a Cpp-Practices git repo to host my cpp code. Now I enter the directory of a leetcode problem and want to debug the executale. So what I needed is current directory path instead of the workspace path provided by vim.fn.getcwd().

@ayamir
Copy link
Owner Author

ayamir commented Jun 13, 2023

all parameters passed to the adapter should be functions or tables, and they shouldn't be called during setup

You are right. Previous implementation has the bug about multi-times prompts asking for exe path.

@Jint-lzxy
Copy link
Collaborator

We use project.nvim to change cwd automatically, so is it suitable to use vim.fn.getcwd() as default path completion? For example, I have a Cpp-Practices git repo to host my cpp code. Now I enter the directory of a leetcode problem and want to debug the executale. So what I needed is current directory path instead of the workspace path provided by vim.fn.getcwd().

fdb356c

@ayamir
Copy link
Owner Author

ayamir commented Jun 13, 2023

I think it can be merged now.

@Jint-lzxy Jint-lzxy merged commit 53727fc into main Jun 13, 2023
@Jint-lzxy Jint-lzxy deleted the fix/dap branch June 13, 2023 15:41
singlemancombat pushed a commit to singlemancombat/nvim-config that referenced this pull request Jun 13, 2023
* fix: make dap for c/cpp/rust/python works.

* refactor: use cpptools to debug c/cpp/rust.

* refactor: make dap servers configurable.

* refactor: move dap servers config to subdir.

* refactor: use mason-nvim-dap.nvim to manage dap servers.

* perf: add note for each dap server.

* fixup! refactor: make debug functionality better

* fixup! fixup! refactor: make debug functionality better

* fix typo

* re-format code

* feat: simplify `utils/dap.lua`

* fixup! add checks for Windows NT (`codelldb`)

* fixup! fixup! remove redundant comment

* fixup! fix path expansion

* fix: expand current file path to exe prompt.

---------

Co-authored-by: Jint-lzxy <[email protected]>
bleedingfight pushed a commit to bleedingfight/nvimdots that referenced this pull request Jun 22, 2023
* fix: make dap for c/cpp/rust/python works.

* refactor: use cpptools to debug c/cpp/rust.

* refactor: make dap servers configurable.

* refactor: move dap servers config to subdir.

* refactor: use mason-nvim-dap.nvim to manage dap servers.

* perf: add note for each dap server.

* fixup! refactor: make debug functionality better

* fixup! fixup! refactor: make debug functionality better

* fix typo

* re-format code

* feat: simplify `utils/dap.lua`

* fixup! add checks for Windows NT (`codelldb`)

* fixup! fixup! remove redundant comment

* fixup! fix path expansion

* fix: expand current file path to exe prompt.

---------

Co-authored-by: Jint-lzxy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants