replace . by current file when running without debugging #3147
Conversation
src/debugAdapter/goDebug.ts
Outdated
| @@ -431,12 +431,11 @@ class Delve { | |||
| if (launchArgs.buildFlags) { | |||
| runArgs.push(launchArgs.buildFlags); | |||
| } | |||
| runArgs.push(program); | |||
| if (isProgramDirectory) { | |||
| runOptions.cwd = program; | |||
There was a problem hiding this comment.
in both cases (isProgramDirectory is true or false) runOptions.cwd can be dirname.
There was a problem hiding this comment.
you're right, i have moved the cwd to initialization of runOptions 763acb8
src/debugAdapter/goDebug.ts
Outdated
| @@ -431,12 +431,11 @@ class Delve { | |||
| if (launchArgs.buildFlags) { | |||
| runArgs.push(launchArgs.buildFlags); | |||
| } | |||
| runArgs.push(program); | |||
There was a problem hiding this comment.
from src/goDebugConfiguration.ts, the program can be set to the active editor's document file name. If the program is set to the file name, I'm afraid it's basically reverting the intended fix for #3096 (supporting the main package with multiple files). What am I missing?
There was a problem hiding this comment.
my mistake, it turely reverts the intended fix for #3096 , but i found a win-win solution, if the program is set to *.go instead of ., the executable filename will be named as the filename of main function, and it can support multiple go files in a folder.
There was a problem hiding this comment.
Thank you for addressing the comment. In that case, the executable filename will be the first file name.
Naming the binary based on the directory name is the de-facto standard of the go command and users who'd get the command using go get command will depend on the behavior. For example, golang.org/x/tools/gopls has main.go as the only file but we expect the final binary name to be the directory name 'gopls', not 'main'. Why is it a problem?
There was a problem hiding this comment.
i agree with your opinion, actually, i thought the executable name should be same in both debug and run mode. but it's not a standard.
| @@ -427,15 +427,14 @@ class Delve { | |||
| if (mode === 'debug') { | |||
| this.noDebug = true; | |||
| const runArgs = ['run']; | |||
| const runOptions: { [key: string]: any } = { env }; | |||
| const runOptions: { [key: string]: any } = { cwd: dirname, env }; | |||
There was a problem hiding this comment.
@simpleapples, @hyangah I think I am missing something... I am not seeing a net change here.
Given that dirname is the same as program when isProgramDirectory is true, setting the cwd upfront here and the code before would essentially result in the same behavior
Of course, for simplicity sake, the change in this PR is good. I just want to confirm that this change doesnt change any behavior to the end user
There was a problem hiding this comment.
i reviewd the change again and fix some typo. i think it won't change any behavior to the end user.
|
For anyone landing here, the net change in this PR is only code refactoring and no change to functionality |
i have read the issue #3096 about adding 'go run .' support and i think the commit 78518d7 has some stuff to work on @ramya-rao-a :
the owner of Debug: add "go run ." support #3096 just want to solve the problem which can not run a go file without debugging, i think there is no need to pass '.' to
go runto solve this problem, pass current dir tocwdis fine.if pass the '.' to
go run, users will not check the process, because the executable file will by named as the directory name by go compiler, it's not consistent.