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

No Emojis on PowerShell #3156

Open
adam-grant-hendry opened this issue Jul 6, 2022 · 26 comments · May be fixed by #3374
Open

No Emojis on PowerShell #3156

adam-grant-hendry opened this issue Jul 6, 2022 · 26 comments · May be fixed by #3374
Labels
S: needs repro Needs a functional reproduction T: bug Something isn't working

Comments

@adam-grant-hendry
Copy link

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

  • Black's version: 22.6.0
  • Pre-Commit's version: 2.19.0
  • Pycln's version: 2.0.1
  • OS and Python version: Windows 10 x64-bit
  • PowerShell: 7.2.5
  • VSCode: 1.68.1
@adam-grant-hendry adam-grant-hendry added the T: bug Something isn't working label Jul 6, 2022
@Jackenmen
Copy link
Contributor

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

@adam-grant-hendry
Copy link
Author

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

Huh...weird.

image

@adam-grant-hendry
Copy link
Author

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

@adam-grant-hendry
Copy link
Author

@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
Copy link
Author

@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.

@ichard26 ichard26 added the S: needs repro Needs a functional reproduction label Jul 6, 2022
@JelleZijlstra
Copy link
Collaborator

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
Copy link
Author

@JelleZijlstra Who will submit the issue with click?

@nnrepos
Copy link
Contributor

nnrepos commented Oct 27, 2022

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
Copy link
Author

@JelleZijlstra Can you provide an update?

@JelleZijlstra
Copy link
Collaborator

I don't have any insights to share.

@adam-grant-hendry
Copy link
Author

Did you open an issue with click?

@adam-grant-hendry
Copy link
Author

@ichard26 Can you provide insights?

@ichard26
Copy link
Collaborator

Sadly no.

@adam-grant-hendry
Copy link
Author

Sadly no.

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

@JelleZijlstra
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Author

Closing as a "won't fix".

@ashb
Copy link

ashb commented Nov 3, 2022

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
Copy link
Author

adam-grant-hendry commented Nov 3, 2022

@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: #3156 (comment)

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
Copy link
Author

@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
Copy link
Collaborator

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

@ashb
Copy link

ashb commented Nov 3, 2022

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

@ashb ashb linked a pull request Nov 3, 2022 that will close this issue
1 task
@ashb
Copy link

ashb commented Nov 3, 2022

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
Copy link

ashb commented Nov 4, 2022

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

@ambv
Copy link
Collaborator

ambv commented Nov 4, 2022

@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
Copy link
Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S: needs repro Needs a functional reproduction T: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants