Black: --check should work on paths

Created on 20 Apr 2018  ·  20Comments  ·  Source: psf/black

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.

Most helpful comment

Would running CI on Windows catch this? I think we need more tests 😁

I'll look into using AppVeyor.

All 20 comments

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.

  1. I wish we could run CI on windows
  2. Why isn't Proactor the default on windows?

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.

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!

Was this page helpful?
0 / 5 - 0 ratings