Feature/color on win #57

Cielquan · 2020-05-28T15:58:44Z

Problem:

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

Changes:

I refactored the devtools.debug.Debug._env_bool classmethod as a function env_bool in devtools.prettier
Reason: The method was not necessary to be a classmethod so I refactored it near env_true function which is used by the method.
I added tests for new function env_bool.
I introduced the highlight module which takes the roll to distinguish if highlighting is used. More info below
I added tests for the new highlight`module.
I changed Debug.__init__ and Debug.__call__ for the use of the new highlight.use_highlight function.

`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 · 2020-05-28T16:30:05Z

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 · 2020-05-28T16:42:15Z

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

Thanks for working on this.

Overall looks good (I'll have to assume most of this is okay since I don't have access to windows).

Two main things are:

I prefer single quotes over double quotes
I think we should enable tests for windows, even if we have to skip of xfail some tests

devtools/highlight.py

devtools/prettier.py

tests/test_main.py

samuelcolvin

This is looking great but unfortunately there are a number of conflicts.

If this is too annoying to fix, let me know and I'll fix as much of it as possible (I don't have windows so it'll involve a bit of guess work).

devtools/highlight.py

tests/test_expr_render.py

samuelcolvin · 2020-05-31T11:39:53Z

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 · 2020-05-31T12:07:50Z

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 · 2020-05-31T12:20:32Z

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.

…to func; simplified utils._enable_vt_mode()

Cielquan · 2020-05-31T12:27:33Z

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 · 2020-05-31T12:32:48Z

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 · 2020-05-31T12:42:14Z

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 · 2020-05-31T12:47:56Z

Ok I refactored env_bool & env_true from prettier into utils.

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

… to test_utils.py

Cielquan · 2020-05-31T13:02:07Z

@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 · 2020-05-31T13:47:57Z

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 · 2020-05-31T14:08:25Z

Sooooo .. finally done 🎆

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 · 2020-06-06T19:58:55Z

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

samuelcolvin

sorry for the slow response, this looks great.

Just a few tiny comments and queries.

devtools/utils.py

Co-authored-by: Samuel Colvin <samcolvin@gmail.com>

Cielquan · 2020-06-11T09:55:13Z

see above

samuelcolvin · 2020-06-16T18:43:31Z

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

I'll make a new release soon.

Cielquan · 2020-07-29T06:52:23Z

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 · 2020-07-29T11:23:44Z

sorry 🙏, been busy 🏃 .

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

samuelcolvin · 2020-07-29T17:47:45Z

done, sorry for the delay.

Cielquan · 2020-07-30T05:33:09Z

done, sorry for the delay.

No problem. Thanks a lot 😄

samuelcolvin reviewed May 28, 2020

View reviewed changes

devtools/highlight.py Outdated Show resolved Hide resolved

devtools/prettier.py Outdated Show resolved Hide resolved

tests/test_main.py Outdated Show resolved Hide resolved

Cielquan requested a review from samuelcolvin May 30, 2020 09:19

samuelcolvin reviewed May 31, 2020

View reviewed changes

devtools/highlight.py Outdated Show resolved Hide resolved

devtools/highlight.py Outdated Show resolved Hide resolved

devtools/highlight.py Outdated Show resolved Hide resolved

tests/test_expr_render.py Outdated Show resolved Hide resolved

Cielquan added 12 commits May 31, 2020 13:54

refactored debug.Debug._env_bool to prettier.env_bool

6d14e83

added tests for prettier.env_bool

292d30b

added highlight module

34e6b69

implement use_highlight function in Debug cls

2e6764c

added tests for highlight module to test_main

62ba8a6

fixed imports

1d1f056

added type hints to env_true and env_bool

356f2a9

updated test_env_bool because of type hints

598bc12

change double quotes to single quotes in PR code

f2eb27e

added 'windows' to ci

a6b9150

added xfail to test failing on win

6997084

changed " to ' in test_expr_render

9fa9b3f

Cielquan added 3 commits May 31, 2020 14:22

refactored highlight.py into utils.py; refactored sys test for win in…

4fb13e4

…to func; simplified utils._enable_vt_mode()

changed " to ' in test_main

bcfd996

added covdefaults pkg

3025340

Cielquan added 2 commits May 31, 2020 14:28

fixed lint

0199463

fixed lint (2)

0a793fc

refactored env_bool & env_true from prettier into utils; movedd tests…

37555cd

… to test_utils.py

Cielquan added 2 commits May 31, 2020 14:51

fixed lint

7a7b73a

fixed type hints in utils

7073538

Cielquan added 3 commits May 31, 2020 15:07

fixed Debug.__call__ rebase error

c312ee7

added cov req = 0 to pytest cmd

940dd3f

removed covdefaults; added nocover to win parts

83df9c2

fixed make install

a5ea9e0

Cielquan requested a review from samuelcolvin May 31, 2020 14:08

samuelcolvin reviewed Jun 11, 2020

View reviewed changes

devtools/utils.py Show resolved Hide resolved

devtools/utils.py Outdated Show resolved Hide resolved

devtools/utils.py Outdated Show resolved Hide resolved

devtools/utils.py Outdated Show resolved Hide resolved

Cielquan and others added 3 commits June 11, 2020 11:47

fixed double quotes to single

3c2b126

Co-authored-by: Samuel Colvin <samcolvin@gmail.com>

changed docstring format

2718027

Co-authored-by: Samuel Colvin <samcolvin@gmail.com>

refactored _enable_vt_mode subfunction into activate_win_color function

9ec2d1b

samuelcolvin merged commit ff3bb9f into samuelcolvin:master Jun 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/color on win #57

Feature/color on win #57

Cielquan commented May 28, 2020 •

edited

Loading

codecov bot commented May 28, 2020 •

edited

Loading

Cielquan commented May 28, 2020 •

edited

Loading

samuelcolvin left a comment

samuelcolvin left a comment

samuelcolvin commented May 31, 2020

Cielquan commented May 31, 2020

samuelcolvin commented May 31, 2020

Cielquan commented May 31, 2020

samuelcolvin commented May 31, 2020

Cielquan commented May 31, 2020

Cielquan commented May 31, 2020

Cielquan commented May 31, 2020 •

edited

Loading

Cielquan commented May 31, 2020

Cielquan commented May 31, 2020

Cielquan commented Jun 6, 2020

samuelcolvin left a comment

Cielquan commented Jun 11, 2020

samuelcolvin commented Jun 16, 2020

Cielquan commented Jul 29, 2020

samuelcolvin commented Jul 29, 2020

samuelcolvin commented Jul 29, 2020

Cielquan commented Jul 30, 2020

Feature/color on win #57

Feature/color on win #57

Conversation

Cielquan commented May 28, 2020 • edited Loading

Problem:

Changes:

highlight module

codecov bot commented May 28, 2020 • edited Loading

Codecov Report

Cielquan commented May 28, 2020 • edited Loading

samuelcolvin left a comment

Choose a reason for hiding this comment

samuelcolvin left a comment

Choose a reason for hiding this comment

samuelcolvin commented May 31, 2020

Cielquan commented May 31, 2020

samuelcolvin commented May 31, 2020

Cielquan commented May 31, 2020

samuelcolvin commented May 31, 2020

Cielquan commented May 31, 2020

Cielquan commented May 31, 2020

Cielquan commented May 31, 2020 • edited Loading

Cielquan commented May 31, 2020

Cielquan commented May 31, 2020

Cielquan commented Jun 6, 2020

samuelcolvin left a comment

Choose a reason for hiding this comment

Cielquan commented Jun 11, 2020

samuelcolvin commented Jun 16, 2020

Cielquan commented Jul 29, 2020

samuelcolvin commented Jul 29, 2020

samuelcolvin commented Jul 29, 2020

Cielquan commented Jul 30, 2020

Cielquan commented May 28, 2020 •

edited

Loading

`highlight` module

codecov bot commented May 28, 2020 •

edited

Loading

Cielquan commented May 28, 2020 •

edited

Loading

Cielquan commented May 31, 2020 •

edited

Loading