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

Silently ignore rows that are not available #5

Merged
merged 1 commit into from
Apr 4, 2023

Conversation

triarius
Copy link
Contributor

@triarius triarius commented Apr 1, 2023

Previously, an N/A was printed, but I think looks nicer to just hide it.

The downside is that users will no longer know that something is missing. I can change it to print an error message in that case, but that could be annoying. If we want such error messages, I think we should make the visibility of certain info configurable so that users may hide it if it is erroring.

Fixes #4

@triarius triarius changed the title Add Silently ignore rows that are not available Silently ignore rows that are not available Apr 1, 2023
Previously, a "N/A" was printed, but I think looks nicer to just hide it.
@triarius triarius closed this Apr 2, 2023
@triarius triarius deleted the triarius/ignore-na branch April 2, 2023 08:50
@triarius triarius restored the triarius/ignore-na branch April 2, 2023 08:50
@triarius triarius reopened this Apr 2, 2023
@nidnogg
Copy link
Owner

nidnogg commented Apr 4, 2023

Heya! These look great from what I tested. There's only one minor detail - the get_gpu_name() function mostly panics when it fails as is, so it might only work for Windows machines. Keeping it there will show a warning, so I've refactored it alongside your changes to return None instead.

I'll merge your changes and cherry-pick the commit with that refactor - although I still need to figure out a command to fetch macOS display adapters or ignore it.

Overall I like these improvements a lot. Thanks for the updates @triarius!

@nidnogg nidnogg merged commit f999b04 into nidnogg:main Apr 4, 2023
@nidnogg
Copy link
Owner

nidnogg commented Apr 4, 2023

See associated cherry-picked commit at f749100

@triarius
Copy link
Contributor Author

triarius commented Apr 4, 2023

Thanks! I had something in the works for GPU too, and I left it alone as it was disabled anyway. Your change seems to work fine on Linux though.

@triarius triarius deleted the triarius/ignore-na branch December 29, 2023 14:10
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.

Hide info that is cannot be found
2 participants