Fix go#package#FromPath() #1435

arp242 · 2017-09-11T12:10:09Z

This didn't work at all on my system. Turned out the problem was that on
my system (Linux) some paths didn't have a trailing slash, so
workspace would be /home/martin/gosrc.

This makes sure that it always works, no matter if a trailing slash is
present or not.

Also break from the loop on the first match, so it selects the
first package found in GOPATH rather than the last. This is also how
imports in Go behave.

fatih · 2017-09-11T12:22:18Z

lgtm

This didn't work at all on my system. Turned out the problem was that on my system (Linux) some paths didn't have a trailing slash, so `workspace` would be `/home/martin/gosrc`. This makes sure that it always works, no matter if a trailing slash is present or not. Also `break` from the loop on the first match, so it selects the *first* package found in GOPATH rather than the *last*. This is also how imports in Go behave.

This fixes two things: - My comments here: fatih@8d617ec#r138018608 It's true that `:lcd` is only for the current window, but me personally I still wouldn't expect any user-visible changes in the current directory when using `:GoDebugStart`. - It prevents `dlv` from creating a binary in the directory. At best this is an annoying side-effect, but at worst it could cause people to lose files: consider what would happen if someone has a package named `foo` but also had a file named `foo` in that directory? If you always use `go install` (like I do) then this is fine, and `:GoDebugStart` creating this file can be rather surprising. Note: this PR requires this patch to work correctly: fatih#1435

* add GoDebug * set rpcid * abort immediately * implement stacktrace, balloon text * highlight * output window * start without breakpoint * add callback to handle delayed event * cancel next * logger * better to be partical callback * variable tree * delay start * buffer should be readonly * goto file * syntax support * function arguments * fix bug in variable viewer * arguments * remove expanded variables * update doc * check binary * display errors * Add modeline * More graceful handling when `dlv` isn't found Previously using `:GoDebugStart` without `dlv` installed would give the error: E117: Unknown function: go#debug#Start Which is not very clear. With this it's: vim-go: could not find 'dlv'. Run :GoInstallBinaries to fix it Which is a lot better :-) * Put the `highlight` commands inside a `ColorScheme` autocmd Otherwise they will be lost when the using `:colorscheme` and some other commands. * Define foreground colours too Inheriting it from the colour scheme is problematic, since it can be anything. In my case (the default colour scheme), it's black. Black on dark blue is hard to read :-) * Show paths relative to current working directory when it makes sense I think this makes more sense? No need for a long path for e.g. `a.go`. Stuff like e.g. `/usr/lib/go/src/runtime/proc.go` is still shown as the full path. * Add g:go_debug_windows setting To allow folk to configure their own window layout, or even hide certain windows. * Add g:go_debug_address setting In case folk already have something running on port 8181 or something * fix eval string * fix eval_var * open/close tree * string is array of byte * change directory * clear state before restart * reset state * unplace sign * add GoDebugStartWith to add options * fix GoDebugStart argments * Display error for Vim 7.4 Previously it would error out with some undefined error. * Run dlv from a temporary directory This fixes two things: - My comments here: 8d617ec#r138018608 It's true that `:lcd` is only for the current window, but me personally I still wouldn't expect any user-visible changes in the current directory when using `:GoDebugStart`. - It prevents `dlv` from creating a binary in the directory. At best this is an annoying side-effect, but at worst it could cause people to lose files: consider what would happen if someone has a package named `foo` but also had a file named `foo` in that directory? If you always use `go install` (like I do) then this is fine, and `:GoDebugStart` creating this file can be rather surprising. Note: this PR requires this patch to work correctly: #1435 * Show error for Neovim As it's not yet supported; otherwise it will give a confusing error. #1390 (comment) * Make temp dir in cross-platform way Since tempnam() seems to behave different on Linux and Windows. * Add documentation Was previously mattn#4 * Rename :GoDebugToggleBreakpoint to :GoDebugBreakpoint Such a mouthful otherwise... * Make :GoDebugStart accept a package name We need this because otherwise there is no way to debug a non-main package. For example if `arp242.net/foo` is the main package, and I want to add a breakpoint to e.g. `arp242.net/foo/bar`. Also use `g:go_build_tags` and remove `:GoDebugStartWith`; I don't think we really need to pass arguments to `dlv`, and if we do we can add an option for it or something. * Add ability to debug tests Need to call `dlv test` for that. * Clean tmpdir on job stop and exit s:exit() isn't called when using `:wq`, so also hook in to VimLeave * Remove :GoDebugStart command in debug mode It doesn't do anything. * Check for a:status > 0 It's -1? * Use go#util#Echo* helpers * Use g:go_debug for debug/development flags This is a more generic system we can also use for other stuff; for example one thing I'd like to add is a "cmd" flag to show all external commands that are being called. We could also add other flags. * Show more information in the variable window * Bugfix: log output would make windows switch Go back to previous window. Also fix lint errors: https://travis-ci.org/Carpetsmoker/vim-go/jobs/306457592 * Run "continue" when using :GoDebug{Next,Step} etc. if the program isn't running Otherwise it will just error out. * Don't error out if localVars or functionArgs aren't defined * Fix some more instances where it would jump windows * Rename :GoDebugEval to :GoDebugPrint I think this command makes more sense, as it "prints the result from an expression". It's also the command that the dlv commandline uses (`p` or `print`). * Remove :GoDebugCommand I'm not sure when this is useful? Remove for now. * Fix stepOut, expand g:go_debug, and remove stepIn - Delve needs `stepOut`, not `stepout`. This took me an insane amount of time to track down :-/ - Add `debugger-commands` to `g:go_debug`; useful to track down the above. - Remove stepin, as Delve doesn't seem to support that. The current implementation doesn't do anything, and it's not listed as one of the constants in `service/api/types.go` (around like 265). * Make :GoDebugBreakpoint accept a line number * Make it possible to define breakpoints before calling :GoDebugStart Much better UX, I think. Especially since starting dlv can take a bit; this will reduce the time people will need spend fiddling their thumbs. * Use --output instead of tmp dir * Make :GoDebugRestart work Need to remove the BufWipe events, as that would cause problems with them stopping the newly created async job. Not sure if that was a good idea in the first place to be honest, why shouldn't I be allowed to remove one of the windows from the layout? * Show string vars better in window * silent! the BalloonExpr Otherwise hovering over a string and lots of other stuff will show errors. * Some small fixes to the documentation. * Small documentation tweaks * Fix GoDebugBreakpoint colours on cterm

arp242 mentioned this pull request Sep 11, 2017

Run dlv from a temporary directory mattn/vim-go#3

Merged

arp242 merged commit 5f0c594 into fatih:master Sep 11, 2017

arp242 deleted the frompath branch September 11, 2017 14:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix go#package#FromPath() #1435

Fix go#package#FromPath() #1435

arp242 commented Sep 11, 2017

fatih commented Sep 11, 2017

Fix go#package#FromPath() #1435

Fix go#package#FromPath() #1435

Conversation

arp242 commented Sep 11, 2017

fatih commented Sep 11, 2017