search

samuelcolvin / python-devtools

Dev tools for python
https://python-devtools.helpmanual.io/
MIT License
985 stars 47 forks source link

Feature/color on win #57

Closed Cielquan closed 4 years ago

Cielquan commented 4 years ago

Problem:

The problem is the missing support for colors on windows machines and is described in #55.

Changes:

highlight module

On import the module will, when on a windows machine, try to activate windows ANSI support by setting the ENABLE_VIRTUAL_TERMINAL_PROCESSING flag via SetConsoleMode. See here for more info: https://docs.microsoft.com/en-us/windows/console/console-virtual-terminal-sequences. Depending on the outcome the color_active variable will be set.

The code which does the work was posted here: https://bugs.python.org/msg291732. Truth be told I only kinda get what the code does but I think the author knows what he does. I tested it on two Win10 machines and it works.

Next there is the use_highlight function. It took over the work to distinguish if DebugOutput.str function called by Debug.__call__ should use highlighting. The function will first check if the Debug class instance got created with highlight=True. If not set it will check the PY_DEVTOOLS_HIGHLIGHT environment variable. If also not set it decides by checking if isatty(file_) and color_active variable are True. The latter part is new and the variable gets set on import (see above). The part before is just the old behavior.

fixes #55

EDIT: Info: My added tests succeed on windows but 21 others do not.

codecov[bot] commented 4 years ago

Codecov Report

Merging #57 into master will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master       #57   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          364       374   +10     
  Branches        56        57    +1     
=========================================
+ Hits           364       374   +10
Impacted Files Coverage Δ
devtools/debug.py 100.00% <100.00%> (ø)
devtools/utils.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 082d8bb...9ec2d1b. Read the comment docs.

Cielquan commented 4 years ago

I don't quite get the coverage thing but will look into it.

EDIT: I messed with settings and fixed them with a force push.

samuelcolvin commented 4 years ago

hi @Cielquan I'm afraid you still have lots of conflicts - see the PR summary above - that's what I meant by move into utils, not just rename.

Cielquan commented 4 years ago

hi @Cielquan I'm afraid you still have lots of conflicts - see the PR summary above - that's what I meant by move into utils, not just rename.

Year I rebased the up-to-date master into the branch .. but I need to fix some things .. the util.py was not there on the older master.

samuelcolvin commented 4 years ago

Year I rebased the up-to-date master into the branch .. but I need to fix some things .. the util.py was not there on the older master.

Yes sorry about that, my fault.

Cielquan commented 4 years ago

Year I rebased the up-to-date master into the branch .. but I need to fix some things .. the util.py was not there on the older master.

Yes sorry about that, my fault.

No problem. Thats normal to update master is it not?

I think it should now work. Lets see.

samuelcolvin commented 4 years ago

yes, it's normal, but I try to always merge PRs in the order they were created so people don't have to fix conflicts caused by new PRs. However in this case, I was rushing to get some things fixed, thus merged newer PRs before this one.

Cielquan commented 4 years ago

Oh we got a circular import problem with the merge of highlight.py into utils.py.

I will refactor a bit to fix it.

Cielquan commented 4 years ago

Ok I refactored env_bool & env_true from prettier into utils.

I also moved tests into a new test_utils.py file.

Cielquan commented 4 years ago

@samuelcolvin The test_print and test_print_subprocess tests are failing everywhere. I will look into it.

The windows pipline fails because it cannot find twine. EDIT: The install cmd fails on win.

Cielquan commented 4 years ago

So I fixed linux/mac with removal of covdefaults package which I just introduced. Problem is that it seems it does not work correctly with coverage directly .. coverage html fails while pytest with pytest-cov works. Problem is the fail_under config. Because of this I removed the package and changed the platform specific pragma to no cover for the windows parts. This should be fixed later when the windows tests are fixed.

The windows+ twine problem is another topic.

Cielquan commented 4 years ago

Sooooo .. finally done :fireworks:

The windows problem was pip install -U setuptools pip in make install. On windows pip is unable to update itself. You need to call pip with python to update it: python -m pip install -U setuptools pip

Cielquan commented 4 years ago

@samuelcolvin Is it good to merge or do you need me to change something? :smiley_cat:

Cielquan commented 4 years ago

see above

samuelcolvin commented 4 years ago

Thanks so much for this, sorry it took so long to merge.

I'll make a new release soon.

Cielquan commented 4 years ago

Thanks so much for this, sorry it took so long to merge.

I'll make a new release soon.

When do you think you can release the new version? 😃

samuelcolvin commented 4 years ago

sorry 🙏, been busy 🏃 .

Just need to fix #62, then I can release. Will try to do so today.

samuelcolvin commented 4 years ago

done, sorry for the delay.

Cielquan commented 4 years ago

done, sorry for the delay.

No problem. Thanks a lot 😄