Skip to content

Add support for MINGW.#111

Closed
RedX2501 wants to merge 2 commits intoeditorconfig:masterfrom
RedX2501:mingw
Closed

Add support for MINGW.#111
RedX2501 wants to merge 2 commits intoeditorconfig:masterfrom
RedX2501:mingw

Conversation

@RedX2501
Copy link

@RedX2501 RedX2501 commented Oct 1, 2018

Add support for MINGW.

Prefer python 3 over python 2.


try:
sys.path.insert(0, vim.eval('a:editorconfig_core_py_dir'))
verbose = int(vim.eval('g:EditorConfig_verbose')) != 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a global variable with such a simple name can easily cause conflicts with other extensions -- Vim uses a single namespace for all global Python variables.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use ec_data['verbose'] instead.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not know they shared a common namespace. I'll improve it.

if int(vim.eval("has('win32unix') || has('win64unix')")) != 0:
if filepath[0] == '/':
# hacky way of making a unix path out of a linux path
filepath = filepath[1] + ':' + filepath[2:]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by "making a UNIX path out of a LINUX path"? Are you trying to convert a Cygwin/MinGW path to a Windows path? This might work on MinGW, but on Cygwin, I think a path like /cygdrive/c/blah would fail.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad comment. Should read:

# hacky way of making a windows path out of a unix path

Yes it will not work with "real" cygwin paths. My aim it to make it work under git bash with vim on windows while keeping it compatible to neovim directly on windows.

Do you have any suggestions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RedX2501 How about cygpath?


# Converts the path to windows when we are in mingw or cygwin
def maybe_to_win(filepath):
if int(vim.eval("has('win32unix') || has('win64unix')")) != 0:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vim doesn't have has('win64unix').
has('win32unix') returns 1 even on 64-bit Cygwin.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove win64unix. win32unix retunrs 1 on cygwin and msys but 0 when using native windows vim.

let s:py_cmd = 'py3'
elseif has('python')
let s:pyfile_cmd = 'pyfile'
let s:py_cmd = 'py'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recent Vim has pythonx, so the better checking order might be pythonx -> python3 -> python.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

@cxw42
Copy link
Member

cxw42 commented May 21, 2019

@RedX2501 the Python core is no longer required. Would you be willing to see if the latest runs ok on your mingw installation?

@RedX2501
Copy link
Author

@cxw42 I have updated to the latest version and it seems to be working. I haven't used vim in a long time, so I'm not sure if I hit the exact reproduction of the bug.

@cxw42
Copy link
Member

cxw42 commented May 22, 2019

@RedX2501 welcome back! ;) Glad to hear it looks ok so far.

@xuhdev
Copy link
Member

xuhdev commented May 22, 2019

Nice! Close this for now. Feel free to open a new issue if anything happens.

@xuhdev xuhdev closed this May 22, 2019
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.

4 participants