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

Fix windows annotations #158

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

nh2
Copy link
Contributor

@nh2 nh2 commented Jul 12, 2012

Windows doesn't allow other programs to read files created with tempfile.NamedTemporaryFile(delete=True). That's why we get something like:

Exception in thread Thread-173:
Traceback (most recent call last):
  File ".\threading.py", line 532, in __bootstrap_inner
  File ".\git.py", line 96, in run
  File ".\subprocess.py", line 701, in communicate
  File ".\subprocess.py", line 911, in _communicate
IOError: [Errno 32] Broken pipe

Also, diff is usually not installed on windows. One of these commits tells the user about this.

Side note: It would be cool if the checks for the existence of external programs could be done in some initialisation part of the plugin, like in the detection phase of SublimeLinter.

@sheldon
Copy link
Collaborator

sheldon commented Jul 14, 2012

Thanks for the patch, I agree about the detection too it would be nice. If there's any other windows users that can confirm a test of this as working I'll get it merged up.

@sheldon
Copy link
Collaborator

sheldon commented Aug 12, 2012

@nh2 I did a little reading through this and it seems ok to me. the intention is to throw a nicer error on not being able to run annotations via diff right? if you say you've done a thorough test I'll get it merged up anyway, as it seems we don't have anyone testing separately. can always revert if there's any major problems.

@nh2
Copy link
Contributor Author

nh2 commented Aug 12, 2012

the intention is to throw a nicer error on not being able to run annotations via diff right?

Yes.

I tested it on a Windows 7 machine and would prefer if others could confirm that it works for them, but if we don't have anyone trying it, we should probably merge it and see if people complain.

@hex
Copy link

hex commented Aug 17, 2012

I tried this fix on Windows 7, but the annotations don't appear. I got rid of the erros, but still no live annotations.

@Kos
Copy link

Kos commented Nov 20, 2012

I confirm that this fix has worked for me on Windows 7 64bit.

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

4 participants