Skip to content
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

Closed
wants to merge 3 commits into from
Closed

#682 gvim & pythonx #858

wants to merge 3 commits into from

Conversation

fdobad
Copy link

@fdobad fdobad commented Aug 14, 2023

Fixes #682

Thanks to emclimber

Copy link
Member

@alerque alerque left a 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.

@@ -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)
Copy link
Collaborator

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.

Copy link
Author

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

image

Copy link
Collaborator

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.

Copy link
Author

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

Copy link
Member

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.

@raven42
Copy link
Collaborator

raven42 commented Aug 16, 2023

@fdobad can you try this change and see if this addresses it too? This could be a better solution for everyone.

d73de9c

Edit: Oops, minor syntax error.

4be8de9

Comment on lines +3166 to +3170
if has('win32')
let py_version = get(g:, 'tagbar_python', 0)
else
let py_version = get(g:, 'tagbar_python', 1)
endif
Copy link
Collaborator

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.

@fdobad
Copy link
Author

fdobad commented Aug 16, 2023

tag_bar_works_3ways

I'm so sorry, now I'm trying to figure out which plugin was messing it up.

@fdobad fdobad closed this Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tagbar doesn't work on gvim for windows with python installed
3 participants