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

Use scope-local functions #97

Merged
merged 1 commit into from
Feb 20, 2017
Merged

Conversation

hkupty
Copy link
Contributor

@hkupty hkupty commented Oct 10, 2016

This avoids overriding local cwd settings, as vim allows lcd
for window-local working directory and neovim supports additionally tcd
for tab-local working directory.

@justinmk
Copy link
Contributor

In every case that I see, the CWD directory is restored in a finally block. So why does it matter if lcd or cd or tcd is used?

@hkupty
Copy link
Contributor Author

hkupty commented Oct 11, 2016

The bug occurs when you have two tabs with different tcds set.

When the plugin runs on the first tab, apparently nothing happens, but the setting has changed the scope back to global.

When running on the second tab, the plugin changes the setting of that tab back to global, and both tabs now are located on the cwd of the latter.

Took me a while to figure this out.

@hkupty
Copy link
Contributor Author

hkupty commented Oct 11, 2016

Also, it's important to say that I tested this on jreybert/vimagit#95, mhinz/vim-signify#192 and this one and all these three plugins were causing the same issue. While neomake/neomake#688 has the same pattern, I couldn't prove it was offending.

Basically any plugin that does not support tcd will break its usage after some time.

This avoids overriding local cwd settings, as vim allows `lcd`
for window-local working directory and neovim supports additionally `tcd`
for tab-local working directory.
@hkupty
Copy link
Contributor Author

hkupty commented Oct 19, 2016

ping @ludovicchabant. Can we merge?

@ludovicchabant
Copy link
Owner

Ah sorry, totally didn't see this pull request. Thanks, I had not considered local CWDs.

However it looks like the version of getcwd that takes parameters is fairly new? It doesn't work on one of my machines that still has Vim 7.4. Is there a backwards compatible way to get a windows' local dir?

@hkupty
Copy link
Contributor Author

hkupty commented Dec 5, 2016

No problem.

I've been on a hurry lately, sorry about that.

About what you said I'm not sure since when it's available, but it looks like it has something to do with lcd, so we could add a check for lcd on that as well..

@ludovicchabant ludovicchabant merged commit 6395898 into ludovicchabant:master Feb 20, 2017
ludovicchabant added a commit that referenced this pull request Feb 20, 2017
@ludovicchabant
Copy link
Owner

Merged, thanks a lot!

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.

None yet

3 participants