-
-
Notifications
You must be signed in to change notification settings - Fork 485
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
#682 gvim & pythonx #858
#682 gvim & pythonx #858
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to make sense to me, but not being on Windows I'm not qualified to test it. It would be helpful if the original reporter or anybody else with the issue could chime in to mention if this works for them. Baring any further feedback we can probably just merge eventually and see if anybody complains, but it would be nice to have at least one person verify it first.
autoload/tagbar.vim
Outdated
@@ -3162,7 +3163,7 @@ function! s:ExecuteCtags(ctags_cmd) abort | |||
call tagbar#debug#log('Exit code: ' . v:shell_error) | |||
redraw! | |||
else | |||
let py_version = get(g:, 'tagbar_python', 1) | |||
let py_version = get(g:, 'tagbar_python', 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer we don't change the default behavior for the g:tagbar_python
variable. Initializing the stdin to DEVNULL is fine, but can we verify if setting the default for g:tagbar_python = 0
is really necessary? Would it be possible to have this as a user setting so we aren't changing the default? We can add documentation around this variable to indicate when it should be set to 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unluckily g:tagbar_python = 1
reverts to the same problem.
Tried it with gvim9.0.1672_64 on windows 11 (> winget install --id vim.vim
), with python3.11.4. ctags5.8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are still seeing this after you set the let g:tagbar_python = 0
in your vimrc? That should set it to 0 for you and it won't use the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That works!
Guess a install for windows users hint should be in place? Like
Windows users should add let g:tagbar_python = 0
to their vimrc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not following closely enough to suggest an exact solution here, but somehow or another if we're at a point of recommend all Windows users do something special we should just make that the default for Windows. Or whatever the situation, the out of box configuration for a given platform should be the best defaults for that platform without most people needing to set something special.
if has('win32') | ||
let py_version = get(g:, 'tagbar_python', 0) | ||
else | ||
let py_version = get(g:, 'tagbar_python', 1) | ||
endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately this change doesn't make a whole lot of sense. Inside the s:run_system()
function call, there is a check already for if has('win32')
and is specifically going into the python handler for that. The original issue was raised specifically against gvim, not just vim. So we need to understand if this is required for all windows installs, or if this is specific to only gvim installations on windows. If we can get some confirmation if this is only for gvim, then we would want to handle this in the s:run_system()
function adding a !has('gui_running')
check:
function! s:run_system(cmd, version) abort
if has('win32') && !has('nvim') && !has('gui_running') && a:version > 0 && (has('python3') || has('python2'))
Otherwise if this does effect every windows installation, then still it would be better to handle it in the s:run_system()
function call adding a !
in front of the has('win32')
check:
function! s:run_system(cmd, version) abort
if !has('win32') && !has('nvim') && a:version > 0 && (has('python3') || has('python2'))
So if someone can help confirm the correct behavior on windows for both GUI and non-GUI installations, then we should update it like one of these options.
Fixes #682
Thanks to emclimber