debug: add explicit preserve layout setting #3125
Conversation
| silent! exe bufwinnr(bufnr('__GODEBUG_OUTPUT__')) 'wincmd c' | ||
| silent! exe bufwinnr(bufnr('__GODEBUG_GOROUTINES__')) 'wincmd c' | ||
|
|
||
| let stackbufnr = bufnr('__GODEBUG_STACKTRACE__') |
There was a problem hiding this comment.
without this change, plugin windows were getting closed when g:go_debug_windows was set to {}
autoload/go/config.vim
Outdated
| @@ -211,6 +211,10 @@ function! go#config#DebugWindows() abort | |||
|
|
|||
| endfunction | |||
|
|
|||
| function! go#config#DebugPreserveLayout() abort | |||
| return get(g:, 'go_debug_preserve_layout', -1) | |||
There was a problem hiding this comment.
Please make the default value 0, as the documentation you added indicates it should be.
autoload/go/debug.vim
Outdated
| @@ -441,7 +457,8 @@ endfunction | |||
| function! s:start_cb() abort | |||
| let l:winid = win_getid() | |||
| let l:debugwindows = go#config#DebugWindows() | |||
| if !empty(l:debugwindows) | |||
| let l:debugpreservelayout = go#config#DebugPreserveLayout() | |||
| if (empty(l:debugwindows) && l:debugpreservelayout == -1) || l:debugpreservelayout != 1 | |||
There was a problem hiding this comment.
if empty(l:debugwindows) || l:debugpreservelayout
There was a problem hiding this comment.
this is the condition for calling only ie not preserving the layout, so it needs to be flipped. i left off the ! on empty( and fixed that tho
|
|
||
| Preserve window layout in debugging mode. If explicitly set, it will override | ||
| the preserving behavior when |'g:go_debug_windows'| is empty. | ||
| > |
There was a problem hiding this comment.
As worded, this sounds like the inverse of what it does. I believe it should indicate that it preserves window layout in debugging mode regardless of the value of g:go_debug_windows.
Also, there should be no need for a user to explicitly set it. Making the default value 0 should be sufficient and results in a user experience consistent with how vim-go works.
There was a problem hiding this comment.
I agree that the wording can be improved. However, a default of 0 would conflict with setting g:go_debug_windows = {} and break backwards compatibility.
The problem is this new setting controls the same window option (:only) as g:go_debug_windows, so there needs to be a way to determine precedence when their values indicate conflicting behavior. The default value of -1 (unset) makes this new setting backwards compatible because -1 means defer to g:go_debug_windows for :only behavior ie if g:go_debug_windows is empty, preserve layout. If not -1, the user explicitly set the value and it should take precedence.
This also allows it to work in conjunction with g:go_debug_windows - a user can set g:go_debug_windows = {} and g:go_debug_preserve_layout = 0, which would not display any debug windows and also not preserve their layout (I'm not sure why someone would want this, but the settings behavior would be consistent nonetheless).
| g:go_debug_windows | g:go_debug_preserve_layout | only |
|---|---|---|
| {} | -1 | noonly |
| {} | 0 | only |
| {} | 1 | noonly |
| non-empty | -1 | only |
| non-empty | 0 | only |
| non-empty | 1 | noonly |
There was a problem hiding this comment.
I think there was a typo in my original request. The logic should be if !(empty(l:debugwindows) || l:debugpreservelayout). With that, there's no need to determine whether g:go_debug_preserve_layout was explicitly set.
When go#config#DebugWindows() returns an empty dict, then the current behavior on master is correct and should be preserved: only is not used. What you've asked for is a way to also skip only when l:debugwindows is not empty; the option you've introduced is useful for that purpose, but there's no need to determine whether the user explicitly set it; it only needs to be considered when l:debugwindows is not empty.
| g:go_debug_windows | g:go_debug_preserve_layout | only |
|---|---|---|
| {} | 0 | noonly |
| {} | 1 | noonly |
| non-empty | 0 | only |
| non-empty | 1 | noonly |
i.e. g:go_debug_preserve_layout should only be considered when l:debugwindows is not empty.
|
I don't understand the test failures. Looks like maybe a temporary test runner problem? |
|
That one failure looks like it was a problem with a flaky test. I've re-run them all. Thanks for contributing to vim-go! |
Adds a new setting,
g:go_debug_preserve_layout, which, when explicitly set, will preserve the current window layout, irrespective of the value ofg:go_debug_windows. Default value keeps current behavior (does not preserve). Fixes #3020.