psf / black

The uncompromising Python code formatter
https://black.readthedocs.io/en/stable/
MIT License
38.77k stars 2.44k forks source link

No Emojis on PowerShell #3156

Open adam-grant-hendry opened 2 years ago

adam-grant-hendry commented 2 years ago

Describe the bug

I've read Issue #1031, but my terminal supports emojis, yet black is not printing them for some reason:

image

To Reproduce

Simply run pre-commit run --all with the following configuration settings:

pyproject.toml Configuration

[tool.black]
line-length = 90
skip-string-normalization = true
target-version = ["py38"]
include = '.*\.pyi?$'
exclude = '\.eggs|\.git|\.mypy_cache|\.tox|\.venv|build|dist'

[tool.pycln]
all = true
include = '.*\.pyi?$'
extend_exclude = 'stubs/vtkmodules-stubs/all.pyi|stubs/vtkmodules-stubs/web/camera.pyi'

.pre-commit-config.yaml

minimum_pre_commit_version: "2.19.0"
fail_fast: true
repos:
  - repo: https://github.com/python-jsonschema/check-jsonschema
    rev: 0.16.2
    hooks:
      - id: check-gitlab-ci
        name: (check-jsonschema) Check `.gitlab-ci.yml` formatted correctly

  - repo: local
    hooks:
      - id: pyupgrade
        name: (pyupgrade) Format source to latest python syntax
        language: system
        files: .*\.pyi?$
        entry: pyupgrade
        args: [
            "--py310-plus",
        ]

  - repo: local
    hooks:
      - id: pycln
        name: (pycln) Remove unused imports
        language: system
        files: .*\.pyi?$
        entry: pycln
        args: [
          "--config=pyproject.toml"
        ]

  - repo: local
    hooks:
      - id: black
        name: (black) Format source code
        language: system
        files: .*\.pyi?$
        entry: black

Expected behavior

Emoji's print

Environment

Jackenmen commented 2 years ago

I might be wrong but I believe this is a pre-commit issue.

adam-grant-hendry commented 2 years ago

I might be wrong but I believe this is a pre-commit issue.

Huh...weird.

image

adam-grant-hendry commented 2 years ago

@jack1142 But then why do other packages work with pre-commit? (e.g. pycln in my example)

adam-grant-hendry commented 2 years ago

@jack1142 The pre-commit author is saying it's most likely black's issue, so you guys are probably gonna have to hash this one out.

adam-grant-hendry commented 2 years ago

@jack1142 The pre-commit author is saying it's most likely black's issue, so you guys are probably gonna have to hash this one out.

Just a warning/heads-up; nothing more.

JelleZijlstra commented 2 years ago

We could also consider blaming click, because click.echo is supposed to handle Unicode correctly for us: https://click.palletsprojects.com/en/7.x/api/#click.echo

adam-grant-hendry commented 2 years ago

@JelleZijlstra Who will submit the issue with click?

nnrepos commented 1 year ago

not sure if this is related, but i get broken emojis on windows command prompt:

.\black>py -m black ..\black_testo          
error: cannot format ..\black_testo\asd.py: Cannot parse: 3:12:     match x:dd

Oh no! 💥 💔 💥
1 file failed to reformat.

here it looks ok, but in pycharm it looks like this: image

adam-grant-hendry commented 1 year ago

@JelleZijlstra Can you provide an update?

JelleZijlstra commented 1 year ago

I don't have any insights to share.

adam-grant-hendry commented 1 year ago

Did you open an issue with click?

adam-grant-hendry commented 1 year ago

@ichard26 Can you provide insights?

ichard26 commented 1 year ago

Sadly no.

adam-grant-hendry commented 1 year ago

Sadly no.

Are there any black contributors who can help? Can someone be assigned to this?

JelleZijlstra commented 1 year ago

This is a volunteer project and you can't expect anyone to pick this up spontaneously. Personally I am unlikely to do anything more here because I can't reproduce the problem (I don't even have Windows) and I don't think I will enjoy digging into it.

adam-grant-hendry commented 1 year ago

This is a volunteer project and you can't expect anyone to pick this up spontaneously.

@JelleZijlstra This issue has been open since July. I just mean is someone in charge of assigning tasks to contributors for the black project.

image

Not expecting anything, just asking. If no one can tackle this, I'll close with "won't fix". Is that alright?

adam-grant-hendry commented 1 year ago

Closing as a "won't fix".

ashb commented 1 year ago

This is a fairly hard problem to solve and is shows up when black (or any python program) is captured/redirected in anyway; black | more on Windows has the same problem.

This snippet from sys.stdout doc says what's happening:

     On Windows, UTF-8 is used for the console device.  Non-character
     devices such as disk files and pipes use the system locale
     encoding (i.e. the ANSI codepage).  Non-console character
     devices such as NUL (i.e. where ``isatty()`` returns ``True``) use the
     value of the console input and output codepages at startup,
     respectively for stdin and stdout/stderr. This defaults to the
     system :term:`locale encoding` if the process is not initially attached
     to a console.

For anyone else hitting this problem, the only possible fix I have found is to set PYTHONUTF8=1 in the your/the users environment.

Is it worth adding this as a note in the docs somewhere perhaps?

adam-grant-hendry commented 1 year ago

@ashb Thank you for looking at this and taking an interest. I appreciate you contributing your time.

In regards to the above, I have some remarks:

  1. Nearly all other packages, e.g. pycln, do not experience problems with emojis out-of-the-box and do not require users to set PYTHONUTF8=1. It could be that pycln sets this environment variable programmatically under the hood in its source code, possibly like so:
import os

if os.name == 'nt':
   os.environ['PYTHONUTF8'] = 1

I mentioned pycln working out-of-the-box here: https://github.com/psf/black/issues/3156#issuecomment-1175688028

If you are able and would like to, would you consider looking at packages like pycln that are working and see what they are doing in their source code? It would be best to use the same approach other working libraries are using.

  1. I would not agree that requiring users set an environment variable manually is a long-term solution. Instead, if setting this environment variable is actually the required fix, I believe it should be implemented in a PR.

If are able to identify that other packages set PYTHONUTF8 programmatically, would you be willing to submit a PR to implement this fix?

adam-grant-hendry commented 1 year ago

@ashb I would also be willing to help you with a PR, should you need assistance. Please let me know if this is something you would like me to do.

JelleZijlstra commented 1 year ago

Great to see progress being made here! I'm happy to review a PR if that's the outcome.

ashb commented 1 year ago

Sure I'll take a look at pycln and see what it does differently

ashb commented 1 year ago

Here is how pycln fixes it:

https://github.com/hadialqattan/pycln/blob/a9b8b0738caf181df649005e5a51c2b61c911852/pycln/__init__.py#L15-L20

Setting PYTHONUITF8 on a running process won't have any effect, the encoding of sys.std* is set very early on in the initalization of the python interpreter.

In #3374 I've done a similar fix to what pycln did. PTAL @JelleZijlstra

ashb commented 1 year ago

@adam-grant-hendry That is an unhelpful comment. I understand your frustration, but leaving the comment serves no purpose but to annoy others.

ambv commented 1 year ago

@adam-grant-hendry, man, don't be like that. Don't treat maintainers of an open-source project like they owe you anything. The license is pretty explicit about the reality of the situation. I don't like waving it in front of anybody's nose but it feels like you misunderstand your role.

The issue has been filed (thank you), and it's open until somebody with Windows will handle it. Maybe you? You can reproduce it and you clearly care about it.

So go and contribute. There's an open PR already. Check it out and confirm if it solves your problem. But definitely don't go and demand action, nor -- worse yet! -- second-guess integrity of the maintainers. That's no way to build long-term collaboration.

adam-grant-hendry commented 1 year ago

@ashb @ambv You are right: my comment was rude and unhelpful.

@JelleZijlstra I'm sorry for what I said. I deleted my last comment and hope you will accept my apology. Thank you for maintaining black and offering to review. I am truly sorry for being rude; it was uncalled for.