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

Add windows support with blessed #78

Merged
merged 5 commits into from
Feb 2, 2020
Merged

Add windows support with blessed #78

merged 5 commits into from
Feb 2, 2020

Conversation

skjerns
Copy link
Contributor

@skjerns skjerns commented Jan 11, 2020

I've re-added support for Windows and fixed the broken imports/code

Main changes:

  • switched from blessing to blessed. Essentially the same library, yet also works on Windows
  • skip loading of signal.SIGPIPE on Windows (not available). Don't know what that line's adding anyway.
  • current time is just printed as a default formatter on Windows, as there's no easy way to get a locale formatting string
  • If memory usage is None, report None

Some remarks: On Windows (and partially on Linux), newer NVIDIA drivers can only report the memory usage of individual processes if the card is in TCC (dedicated) mode, that means no monitor is attached to them.

It might be handy to report this in the README

@skjerns
Copy link
Contributor Author

skjerns commented Jan 11, 2020

I don't understand why the check fails, could you help me?

@wookayin
Copy link
Owner

This line fails:

>           t_color.normal = u'\x1b[0;10m'
E           AttributeError: can't set attribute

which is a workaround for #32.

@skjerns
Copy link
Contributor Author

skjerns commented Jan 13, 2020

"fixed"

gpustat/core.py Outdated
@@ -487,11 +493,12 @@ def print_formatted(self, fp=sys.stdout, force_color=False, no_color=False,
raise ValueError("--color and --no_color can't"
" be used at the same time")

if force_color:
if force_color and not self.is_windows():
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why checking windows here? On windows, the --color flag should also work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're right! my bad

wookayin added a commit that referenced this pull request Jan 16, 2020
@wookayin
Copy link
Owner

wookayin commented Jan 17, 2020

I am trying to add a travis CI test. Do you have any idea about why the test fails?

@skjerns
Copy link
Contributor Author

skjerns commented Jan 17, 2020

I am trying to add a travis CI test. Do you have any idea about why the test fails?

No idea! For me the test works fine.

The tests worked before, didn't they?

@wookayin
Copy link
Owner

Not on windows.

@skjerns
Copy link
Contributor Author

skjerns commented Jan 17, 2020

Mhh. seriously no clue. The tests run fine on my Windows machine, also the command term=Terminal() has no problem

@wookayin
Copy link
Owner

No worries, I will take care of them. FYI I reported an issue jquast/blessed#126.

@wookayin wookayin changed the base branch from master to blessed February 2, 2020 05:48
@wookayin wookayin changed the title Re-add windows support Add windows support with blessed Feb 2, 2020
@wookayin wookayin merged commit ebd7f46 into wookayin:blessed Feb 2, 2020
wookayin added a commit that referenced this pull request Feb 2, 2020
- To prevent blinking cursors, do not re-store cursor locations
- Workaround sgr0 character issues (#32) again
- Misc: various style cleanups
@wookayin wookayin added this to the 1.0 milestone Feb 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants