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

Refactored Display implementation for Platform enum #83

Closed
wants to merge 8 commits into from

Conversation

sebihutanu
Copy link

WHOAMI

Key Changes:

  1. Display Restructuring:

    • Significantly restructured the Display implementation for the Platform type.
    • Introduced a dedicated file, disp.rs, to centralize and streamline platform-specific display functions.
  2. Aesthetic Enhancements:

    • Revamped the platform display, focusing on enhancing the overall aesthetic.
    • Resulting in a more visually appealing output for improved user experience.

    To enhance the utility's adaptability, a crucial implementation has been added. The program checks whether it is running in a terminal environment (using https://crates.io/crates/is-terminal) before deciding how to display the platform. If executed in a terminal, the graphical version is presented; otherwise, in scenarios like running in a pipe, the text version is displayed.

Copy link

@alexandruradovici alexandruradovici left a comment

Choose a reason for hiding this comment

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

Please run cargo fmt to format your source.

Copy link

@alexandruradovici alexandruradovici left a comment

Choose a reason for hiding this comment

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

Please fix clippys errors.

src/fake.rs Outdated
@@ -1,75 +1,681 @@
//! Currently used for WebAssembly unknown (non-web) only

Choose a reason for hiding this comment

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

Why delete this?

Copy link
Author

Choose a reason for hiding this comment

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

by mistake, will fix

src/fake.rs Outdated

Choose a reason for hiding this comment

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

Please explain why do we need this file?

Copy link
Author

Choose a reason for hiding this comment

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

I am sorry, I just noticed that I somehow copied the entire code from lib.rs to fake.rs. . I'm not sure how that happened, but I'll fix it immediately.

Copy link
Member

@AldaronLau AldaronLau left a comment

Choose a reason for hiding this comment

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

Is there a reason that this has to exist within the whoami crate? I feel like this functionality could be split out into its own crate depending on whoami.

@AldaronLau
Copy link
Member

@sebihutanu1 I'm going to close this PR, as I have decided it's functionality doesn't belong in the whoami crate. I recommend creating a separate crate for it, depending on whoami and is-terminal, if you require this functionality.

If you'd like to contribute to this project in another way, feel free to send me an email at aldaronlau@gmail.com or comment in #89, and we can figure something out. Thank you for your interest.

@AldaronLau AldaronLau closed this Feb 14, 2024
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.

4 participants