Skip to content

Show detected platform on startup and in About popup #1221

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Show detected platform on startup and in About popup #1221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f1db4d4
run: Add platform information on startup.
plugyawn May 8, 2022
e405fbd
views: Add platform information to the About Pop-Up.
plugyawn May 8, 2022
9857ec0
test: Add tests for detected platform.
plugyawn May 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions tests/cli/test_run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
parse_args,
)
from zulipterminal.model import ServerConnectionFailure
from zulipterminal.platform_code import PLATFORM
from zulipterminal.version import ZT_VERSION


Expand Down Expand Up @@ -132,6 +133,7 @@ def test_valid_zuliprc_but_no_connection(

lines = captured.out.strip().split("\n")
expected_lines = [
"Detected Platform: " + PLATFORM,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note that this will be the platform that the tests are running on. We may want to mock this instead to ensure we get some known output (and avoid the running vs testing confusion for readers).

It's good to keep tests in the same commit as the code that changes, as it's a unit of change, so here combined with one of the other commits.

We may want a specific test to handle how this output should vary on different platforms, including unsupported.

"Loading with:",
" theme 'zt_dark' specified with no config.",
" autohide setting 'no_autohide' specified with no config.",
Expand Down Expand Up @@ -182,6 +184,7 @@ def test_warning_regarding_incomplete_theme(

lines = captured.out.strip().split("\n")
expected_lines = [
"Detected Platform: " + PLATFORM,
"Loading with:",
f" theme '{bad_theme}' specified on command line.",
"\x1b[93m WARNING: Incomplete theme; results may vary!",
Expand Down Expand Up @@ -395,6 +398,7 @@ def test_successful_main_function_with_config(
captured = capsys.readouterr()
lines = captured.out.strip().split("\n")
expected_lines = [
f"Detected Platform: {PLATFORM}",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason you used different code here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

None, I'll change it to the same code as the others.

"Loading with:",
" theme 'zt_dark' specified in zuliprc file (by alias 'default').",
" autohide setting 'autohide' specified in zuliprc file.",
Expand Down
5 changes: 5 additions & 0 deletions zulipterminal/cli/run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
)
from zulipterminal.core import Controller
from zulipterminal.model import ServerConnectionFailure
from zulipterminal.platform_code import PLATFORM
from zulipterminal.version import ZT_VERSION


Expand Down Expand Up @@ -439,6 +440,10 @@ def main(options: Optional[List[str]] = None) -> None:
if args.notify:
zterm["notify"] = (args.notify, "on command line")

if PLATFORM:
print("Detected Platform: " + PLATFORM)
else:
print("Running on undetected platform.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this intended to be for unsupported platforms, or something else?

Please make sure you're aware what values this can take :)

You're not testing this result, in any case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is this intended to be for unsupported platforms, or something else?

Please make sure you're aware what values this can take :)

You're not testing this result, in any case.

I'll look into that; I'm still not exactly sure how to check for unsupported platforms, though.

print("Loading with:")
print(" theme '{}' specified {}.".format(*theme_to_use))
complete, incomplete = complete_and_incomplete_themes()
Expand Down
9 changes: 8 additions & 1 deletion zulipterminal/ui_tools/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
match_stream,
match_user,
)
from zulipterminal.platform_code import PLATFORM
from zulipterminal.server_url import near_message_url
from zulipterminal.ui_tools.boxes import MessageBox, PanelSearchBox
from zulipterminal.ui_tools.buttons import (
Expand Down Expand Up @@ -1113,7 +1114,13 @@ def __init__(
else []
)
contents = [
("Application", [("Zulip Terminal", zt_version)]),
(
"Application",
[
("Zulip Terminal", zt_version),
("Platform detected", PLATFORM if PLATFORM else "Invalid platform"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This condition shouldn't arise - again, check the values of PLATFORM.

],
),
("Server", [("Version", server_version)] + self.feature_level_content),
(
"Application Configuration",
Expand Down