preservim / tagbar

Vim plugin that displays tags in a window, ordered by scope
https://preservim.github.io/tagbar
Other
6.11k stars 486 forks source link

Use universal_newlines for Popen call #733

Closed raven42 closed 3 years ago

raven42 commented 3 years ago

Closes #629

Not actually tested on windows as I don't have a windows installation to test with. Will be a good idea if somebody can verify on windows.

alerque commented 3 years ago

This looks sane to me, but I don't actually have Windows to test with either. Confirmation from anyone would be appreciated, although if this sits too long without feedback I'd be okay with merging anyway and see if anybody complains.

CC @char101

char101 commented 3 years ago

I have been using that modification since the issue is created on Windows, regularly rebased and haven't found any problem. If any there could be problem with python2, if vim is only linked with python2.

raven42 commented 3 years ago

According to the python 2.7 documentation, the Popen constructor does still accept universal_newlines=True so it looks like we'll be ok there as well.

Thanks for the verification @char101

skywind3000 commented 3 years ago

You forgot to add a errors = "ignore" and popen may crash when filenames contain strange CJK characters, see:

https://github.com/preservim/tagbar/pull/745