`dap-start-debugging-noexpand': expand :cwd #414
Conversation
In a previous commit, :cwd was expanded specifically in `dap-chrome'. Undo that, and centralize expanding :cwd in `dap-start-debugging-noexpand'. This allows :cwd to be a relative path. Fixes #413.
dap-mode.el
Outdated
| @@ -1590,7 +1590,8 @@ before starting the debug process." | |||
| :wait-for-port :type :request :port | |||
| :startup-function :environment-variables :hostName host) launch-args) | |||
| (session-name (dap--calculate-unique-name name (dap--get-sessions))) | |||
| (default-directory (or cwd default-directory)) | |||
| (default-directory (or (and cwd (expand-file-name cwd)) | |||
There was a problem hiding this comment.
default-directory should be expanded too.
There was a problem hiding this comment.
Why that? From its doc: "it should be an absolute directory name". Also, against what would default-directory be expanded, given that we omit it here? The doc of expand-file-name says that its argument is expanded against default-directory unless the second argument is specified.
All this would achieve is that "." and ".." paths get eliminated. This way, we don't change default-directory at all unless :cwd is specified.
There was a problem hiding this comment.
Your fix for dap-chrome would still behave exactly the same way, because it specifies a default-directory :cwd that is then (needlessly) expanded against default-directory.
There was a problem hiding this comment.
For the same reason we expand default-directory in dap-chrome.el - default directory could be ~/foo/bar
There was a problem hiding this comment.
Also, the PR is introducing regression - we will send ~/foo/bar to the adapter since :cwd won't be expanded.
There was a problem hiding this comment.
We don't need to expand-file-name default directory to launch the adapter:
(let ((default-directory "~/Downloads"))
(shell-command "pwd")) ;; => `message`s "/home/nikita/Downloads" on my systemI think there are five options here:
expand-file-nameindap-chrome- always send an expanded :cwd, even if unspecified
- send an expanded :cwd only if specified
- Either of the above, but also expand
default-directory(against itself)
:cwd would be expanded in all cases. I ended up going with the last one, always sending an expanded :cwd and always expanding default-directory.
There was a problem hiding this comment.
The cwd should be set in the template otherwise commands like restart session won't work in the expected manner when cwd is relative.
There was a problem hiding this comment.
Why does dap-start-debugging-noexpand mutate LAUNCH-ARGS with plist-put :name?
There was a problem hiding this comment.
What is the problem caused by that?
There was a problem hiding this comment.
Why does
dap-start-debugging-noexpandmutate LAUNCH-ARGS withplist-put :name?
Modifying the template should be fine and expected since we are copying it.
In `dap-start-debugging-noexpand', also `expand-file-name' `default-directory' if `:cwd' is unspecified. Additionally, always send a fully expanded :cwd based on the `default-directory' obtained above to the adapter.
In a previous commit, :cwd was expanded specifically in
dap-chrome. Undo that,and centralize expanding :cwd in
dap-start-debugging-noexpand. This allows:cwd to be a relative path.
Fixes #413.