Operating system: Win 10 x86
Python version: 3.6.5 64 bit
Black version: 18.4a2
Does also happen on master: Throws this backtrace instead:
Traceback (most recent call last):
File "C:\Users\nikolaus.waxweiler\AppData\Local\Python\Python36\Scripts\black-script.py", line 11, in <module>
load_entry_point('black==18.4a2', 'console_scripts', 'black')()
File "C:\Users\nikolaus.waxweiler\AppData\Local\Python\Python36\site-packages\click\core.py", line 722, in __call__
return self.main(*args, **kwargs)
File "C:\Users\nikolaus.waxweiler\AppData\Local\Python\Python36\site-packages\click\core.py", line 697, in main
rv = self.invoke(ctx)
File "C:\Users\nikolaus.waxweiler\AppData\Local\Python\Python36\site-packages\click\core.py", line 895, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "C:\Users\nikolaus.waxweiler\AppData\Local\Python\Python36\site-packages\click\core.py", line 535, in invoke
return callback(*args, **kwargs)
File "C:\Users\nikolaus.waxweiler\AppData\Local\Python\Python36\site-packages\click\decorators.py", line 17, in new_func
return f(get_current_context(), *args, **kwargs)
File "C:\Users\nikolaus.waxweiler\AppData\Local\Python\Python36\site-packages\black.py", line 211, in main
sources, line_length, write_back, fast, quiet, loop, executor
File "c:\program files\python36\lib\asyncio\base_events.py", line 468, in run_until_complete
return future.result()
File "C:\Users\nikolaus.waxweiler\AppData\Local\Python\Python36\site-packages\black.py", line 296, in schedule_formatting
loop.add_signal_handler(signal.SIGINT, cancel, _task_values)
File "c:\program files\python36\lib\asyncio\events.py", line 499, in add_signal_handler
raise NotImplementedError
NotImplementedError
Running black . works fine, but black --check . returns the error code 1 instead. It should search for ./**/*.py and check everything. Reason: I'd like to write
[testenv:black]
deps = black
commands = black --check .
in my tox.ini and not worry about exact paths, wildcards, etc.
Oops. This used to work, it's a regression. Will fix!
Hm, wait. There are two problems here. First: the NotImplementedError is a Windows-specific error that needs fixing. (apparently on Windows there are no signal handlers or something?)
On 18.4a2, the documentation for black --check . says it returns 1 if there are any changes in your directory. Are there any changes in your files? Why would it return 1 otherwise? Can you show your output of black --check .?
@zsol, you're a Windows user, it would be great if you could find a solution for the NotImplementedError.
I ran black . before the check and on all files individually just to be
sure. There is no output.
Does the output of --check give you only "good job" messages?
Only if run on the individual files, no output if given a path.
I don't think there's a great way to do the same kind of clean shutdown on Windows as those signal handlers let us do on other OSes, so IMO our best bet here is just to gate adding those signal handlers on OSes that have proper POSIX signals
@madig are you by any chance running black --check on an empty directory? :) That's the only way I can get black to produce no output
Nope, files exist 😕 as said, black . works fine. Just black --check . always retuns a 1 and no output. Hm. black . also returns no output whatsoever, but the files are formatted.
This only is an issue on Windows, output and paths are fine on macOS/Linux.
@zsol, can you reproduce the lack of stderr output on Windows? AFAIK the "stderr" concept exists on Windows and should work just fine. I'm even using Click's echo to ensure colors are properly translated, and so on.
The lack of output here is likely what makes this hard for us to debug. Well, that, and me not having access to any Windows boxes.
Yep, I just managed to reproduce. Not sure what's going on, I'll have some time to dig tomorrow. Here are a couple of examples:
> black --version
black, version 18.4a2
> black --check .\black.py
black.py already well formatted, good job.
> black --check .\black.py .\tests\test_black.py
> black --check -l 80 .\black.py
would reformat black.py
> black --check -l 80 .\black.py .\tests\test_black.py
> black --check .
> black -l 80 .\black.py .\tests\test_black.py
Sounds like there's something wrong around how we schedule formatting of multiple files using asyncio.
Sounds like there's something wrong around how we schedule formatting of multiple files using asyncio.
This sounds Windows-specific.
OK, this is Windows-specific. We need to switch to use the ProactorEventLoop. See: https://docs.python.org/3/library/asyncio-eventloops.html#windows
Makes sense. I'll try using that tomorrow.
On Sat, Apr 21, 2018, 23:13 Łukasz Langa notifications@github.com wrote:
OK, this is Windows-specific. We need to switch to use the
ProactorEventLoop. See:
https://docs.python.org/3/library/asyncio-eventloops.html#windows—
You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub
https://github.com/ambv/black/issues/151#issuecomment-383335818, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAEEtIfF1Qn7YRg55l2zUgKGMA3bfxXwks5tq67ygaJpZM4Td1Kl
.
Would running CI on Windows catch this? I think we need more tests 😁
I'll look into using AppVeyor.
We use AppVeyor, too :) https://packaging.python.org/guides/supporting-windows-using-appveyor/
I can't reproduce this on master.
> pipenv run pip install black
> black --check .
> pipenv run pip install .
> black --check .
C:\Users\zsol\black\black.py wasn't modified on disk since last run.
C:\Users\zsol\black\blib2to3\pgen2\conv.py wasn't modified on disk since last run.
...
@madig can you try using master? pip install git+https://github.com/ambv/black
The issue in 18.4a2 was the same NotImplementedError, but it was never output before https://github.com/ambv/black/commit/15d5e36ea38988084584639b02aafcaaa2744dcf for reasons I don't fully understand. AFAICT the same exception is being thrown before and after that commit, the only difference is one extra level of function call.
TLDR: this is fixed entirely by #156
Confirmed to work, thanks!
Most helpful comment
Would running CI on Windows catch this? I think we need more tests 😁
I'll look into using AppVeyor.