From e4128ad8191429d85ff4183d88d226b0da10a127 Mon Sep 17 00:00:00 2001 From: christopher-nguyen-re Date: Mon, 19 Sep 2022 22:36:10 -0400 Subject: [PATCH 1/8] Fix icon display issue and add test --- src/lightning_app/cli/cmd_pl_init.py | 14 ++++++++++++-- tests/tests_app/cli/test_cmd_pl_init.py | 10 ++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/lightning_app/cli/cmd_pl_init.py b/src/lightning_app/cli/cmd_pl_init.py index 06ec80d5cdb90..3075fe167c4b7 100644 --- a/src/lightning_app/cli/cmd_pl_init.py +++ b/src/lightning_app/cli/cmd_pl_init.py @@ -2,6 +2,7 @@ import re import shutil import subprocess +import sys import tarfile import urllib.request from pathlib import Path @@ -148,8 +149,17 @@ def print_pretty_report( text_pathname.stylize(f"link file://{path}") text_pathname.append(f" {padding} {help_text}", "blue") - icon = "📂 " if path.is_dir() else "📄 " - tree.add(Text(icon) + text_pathname) + icon = "" + try: + if path.is_dir(): + icon = "📂 ".encode() + else: + icon = "📄 ".encode() + except UnicodeEncodeError: + # Icon will be hidden. Two spaces to align pathnames. + icon = b" " + + tree.add(Text(icon.decode()) + text_pathname) print("\n") print("Done. The app is ready here:\n") diff --git a/tests/tests_app/cli/test_cmd_pl_init.py b/tests/tests_app/cli/test_cmd_pl_init.py index 1d5548b051323..71f355653abb1 100644 --- a/tests/tests_app/cli/test_cmd_pl_init.py +++ b/tests/tests_app/cli/test_cmd_pl_init.py @@ -84,6 +84,16 @@ def test_pl_app_download_frontend(tmp_path): assert "static" in contents +def test_pl_app_icon(tmp_path): + """Test that Icons in PL app CLI output are not broken""" + icon1 = None + icon2 = None + icon1 = "📂 ".encode() + icon2 = "📄 ".encode() + assert icon1 is not None + assert icon2 is not None + + @pytest.mark.parametrize( "cwd, source_dir, script_path", [ From c5b540506cdc37c65d723a2405114228f6f1e07b Mon Sep 17 00:00:00 2001 From: christopher-nguyen-re Date: Mon, 19 Sep 2022 22:40:57 -0400 Subject: [PATCH 2/8] Modify test case for icon encoding --- tests/tests_app/cli/test_cmd_pl_init.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/tests_app/cli/test_cmd_pl_init.py b/tests/tests_app/cli/test_cmd_pl_init.py index 71f355653abb1..b02385ddf476a 100644 --- a/tests/tests_app/cli/test_cmd_pl_init.py +++ b/tests/tests_app/cli/test_cmd_pl_init.py @@ -85,13 +85,12 @@ def test_pl_app_download_frontend(tmp_path): def test_pl_app_icon(tmp_path): - """Test that Icons in PL app CLI output are not broken""" - icon1 = None - icon2 = None - icon1 = "📂 ".encode() - icon2 = "📄 ".encode() - assert icon1 is not None - assert icon2 is not None + """Test that Icons in PL app CLI output are not broken in encoding""" + try: + icon1 = "📂 ".encode() + icon2 = "📄 ".encode() + except UnicodeEncodeError as exc: + pytest.fail(exc, pytrace=True) @pytest.mark.parametrize( From a843bc98b7fcf05a9b2adfa80198a82747e86ec8 Mon Sep 17 00:00:00 2001 From: christopher-nguyen-re Date: Mon, 19 Sep 2022 22:47:08 -0400 Subject: [PATCH 3/8] Minor change to test case --- tests/tests_app/cli/test_cmd_pl_init.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tests_app/cli/test_cmd_pl_init.py b/tests/tests_app/cli/test_cmd_pl_init.py index b02385ddf476a..dd500bfee1faa 100644 --- a/tests/tests_app/cli/test_cmd_pl_init.py +++ b/tests/tests_app/cli/test_cmd_pl_init.py @@ -84,7 +84,7 @@ def test_pl_app_download_frontend(tmp_path): assert "static" in contents -def test_pl_app_icon(tmp_path): +def test_pl_app_icon(): """Test that Icons in PL app CLI output are not broken in encoding""" try: icon1 = "📂 ".encode() From fb271c0883a097bc51aa689fb223929a6f783551 Mon Sep 17 00:00:00 2001 From: christopher-nguyen-re Date: Mon, 19 Sep 2022 23:04:29 -0400 Subject: [PATCH 4/8] Update Changelog --- src/lightning_app/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lightning_app/CHANGELOG.md b/src/lightning_app/CHANGELOG.md index e7ca605b4c2b5..aaaf916db6015 100644 --- a/src/lightning_app/CHANGELOG.md +++ b/src/lightning_app/CHANGELOG.md @@ -65,6 +65,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Fixed an issue where custom property setters were not being used `LightningWork` class. ([#14259](https://github.com/Lightning-AI/lightning/pull/14259)) +- Fixed an issue where some terminals would display broken icons in the PL app CLI. ([#14226](https://github.com/Lightning-AI/lightning/issues/14226)) + ## [0.6.0] - 2022-09-08 From c85050dfee251734bf2778e543671891d8e376ac Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 20 Sep 2022 03:06:55 +0000 Subject: [PATCH 5/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/lightning_app/cli/cmd_pl_init.py | 2 +- tests/tests_app/cli/test_cmd_pl_init.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lightning_app/cli/cmd_pl_init.py b/src/lightning_app/cli/cmd_pl_init.py index 3075fe167c4b7..cdb7b8a3f1b58 100644 --- a/src/lightning_app/cli/cmd_pl_init.py +++ b/src/lightning_app/cli/cmd_pl_init.py @@ -154,7 +154,7 @@ def print_pretty_report( if path.is_dir(): icon = "📂 ".encode() else: - icon = "📄 ".encode() + icon = "📄 ".encode() except UnicodeEncodeError: # Icon will be hidden. Two spaces to align pathnames. icon = b" " diff --git a/tests/tests_app/cli/test_cmd_pl_init.py b/tests/tests_app/cli/test_cmd_pl_init.py index dd500bfee1faa..8f8f4bfbabb43 100644 --- a/tests/tests_app/cli/test_cmd_pl_init.py +++ b/tests/tests_app/cli/test_cmd_pl_init.py @@ -85,7 +85,7 @@ def test_pl_app_download_frontend(tmp_path): def test_pl_app_icon(): - """Test that Icons in PL app CLI output are not broken in encoding""" + """Test that Icons in PL app CLI output are not broken in encoding.""" try: icon1 = "📂 ".encode() icon2 = "📄 ".encode() From 5d489f3d400d718242acbb7967e703d1c9ebb400 Mon Sep 17 00:00:00 2001 From: christopher-nguyen-re Date: Tue, 20 Sep 2022 08:09:33 -0400 Subject: [PATCH 6/8] Add helper function and modified encoding --- src/lightning_app/cli/cmd_pl_init.py | 25 ++++++++++++++++--------- tests/tests_app/cli/test_cmd_pl_init.py | 12 ++++-------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/lightning_app/cli/cmd_pl_init.py b/src/lightning_app/cli/cmd_pl_init.py index 3075fe167c4b7..72947dedafe01 100644 --- a/src/lightning_app/cli/cmd_pl_init.py +++ b/src/lightning_app/cli/cmd_pl_init.py @@ -150,19 +150,26 @@ def print_pretty_report( text_pathname.append(f" {padding} {help_text}", "blue") icon = "" - try: - if path.is_dir(): - icon = "📂 ".encode() - else: - icon = "📄 ".encode() - except UnicodeEncodeError: - # Icon will be hidden. Two spaces to align pathnames. - icon = b" " + if path.is_dir(): + if _can_encode_icon("📂 "): + icon = "📂 " + else: + if _can_encode_icon("📄 "): + icon = "📄 " - tree.add(Text(icon.decode()) + text_pathname) + tree.add(Text(icon) + text_pathname) print("\n") print("Done. The app is ready here:\n") print(tree) print("\nRun it:\n") print(Panel(f"[red]lightning run app {directory.relative_to(Path.cwd()) / 'app.py'}")) + + +def _can_encode_icon(icon:str) -> bool: + """Helper function to check whether an icon can be encoded""" + try: + icon.encode(sys.stdout.encoding) + return True + except UnicodeEncodeError: + return False \ No newline at end of file diff --git a/tests/tests_app/cli/test_cmd_pl_init.py b/tests/tests_app/cli/test_cmd_pl_init.py index dd500bfee1faa..e764df22349c9 100644 --- a/tests/tests_app/cli/test_cmd_pl_init.py +++ b/tests/tests_app/cli/test_cmd_pl_init.py @@ -5,7 +5,7 @@ from click.testing import CliRunner from lightning_app.cli import lightning_cli -from lightning_app.cli.cmd_pl_init import download_frontend, pl_app +from lightning_app.cli.cmd_pl_init import download_frontend, pl_app, _can_encode_icon def test_pl_app_input_paths_do_not_exist(tmp_path): @@ -84,13 +84,9 @@ def test_pl_app_download_frontend(tmp_path): assert "static" in contents -def test_pl_app_icon(): - """Test that Icons in PL app CLI output are not broken in encoding""" - try: - icon1 = "📂 ".encode() - icon2 = "📄 ".encode() - except UnicodeEncodeError as exc: - pytest.fail(exc, pytrace=True) +def test_pl_app_encode_icon(): + assert _can_encode_icon("📂") == True + assert _can_encode_icon("📄") == True @pytest.mark.parametrize( From d1dc124640e858a3f35f2f2eeb4791e5238e22ef Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 20 Sep 2022 12:13:43 +0000 Subject: [PATCH 7/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/lightning_app/cli/cmd_pl_init.py | 6 +++--- tests/tests_app/cli/test_cmd_pl_init.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lightning_app/cli/cmd_pl_init.py b/src/lightning_app/cli/cmd_pl_init.py index 72947dedafe01..4fc7d358ba7d0 100644 --- a/src/lightning_app/cli/cmd_pl_init.py +++ b/src/lightning_app/cli/cmd_pl_init.py @@ -166,10 +166,10 @@ def print_pretty_report( print(Panel(f"[red]lightning run app {directory.relative_to(Path.cwd()) / 'app.py'}")) -def _can_encode_icon(icon:str) -> bool: - """Helper function to check whether an icon can be encoded""" +def _can_encode_icon(icon: str) -> bool: + """Helper function to check whether an icon can be encoded.""" try: icon.encode(sys.stdout.encoding) return True except UnicodeEncodeError: - return False \ No newline at end of file + return False diff --git a/tests/tests_app/cli/test_cmd_pl_init.py b/tests/tests_app/cli/test_cmd_pl_init.py index e764df22349c9..84f8edc30d2d7 100644 --- a/tests/tests_app/cli/test_cmd_pl_init.py +++ b/tests/tests_app/cli/test_cmd_pl_init.py @@ -5,7 +5,7 @@ from click.testing import CliRunner from lightning_app.cli import lightning_cli -from lightning_app.cli.cmd_pl_init import download_frontend, pl_app, _can_encode_icon +from lightning_app.cli.cmd_pl_init import _can_encode_icon, download_frontend, pl_app def test_pl_app_input_paths_do_not_exist(tmp_path): From 2834215f6e07dd44ee5883a691d35174ae681523 Mon Sep 17 00:00:00 2001 From: awaelchli Date: Tue, 20 Sep 2022 15:19:46 +0200 Subject: [PATCH 8/8] improve test --- src/lightning_app/cli/cmd_pl_init.py | 9 ++------- tests/tests_app/cli/test_cmd_pl_init.py | 16 +++++++++++++--- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/lightning_app/cli/cmd_pl_init.py b/src/lightning_app/cli/cmd_pl_init.py index 4fc7d358ba7d0..dda492c9c9e08 100644 --- a/src/lightning_app/cli/cmd_pl_init.py +++ b/src/lightning_app/cli/cmd_pl_init.py @@ -149,13 +149,8 @@ def print_pretty_report( text_pathname.stylize(f"link file://{path}") text_pathname.append(f" {padding} {help_text}", "blue") - icon = "" - if path.is_dir(): - if _can_encode_icon("📂 "): - icon = "📂 " - else: - if _can_encode_icon("📄 "): - icon = "📄 " + icon = "📂 " if path.is_dir() else "📄 " + icon = icon if _can_encode_icon(icon) else "" tree.add(Text(icon) + text_pathname) diff --git a/tests/tests_app/cli/test_cmd_pl_init.py b/tests/tests_app/cli/test_cmd_pl_init.py index 84f8edc30d2d7..2e479ee6e9eb3 100644 --- a/tests/tests_app/cli/test_cmd_pl_init.py +++ b/tests/tests_app/cli/test_cmd_pl_init.py @@ -1,5 +1,7 @@ import os +import sys from unittest import mock +from unittest.mock import Mock import pytest from click.testing import CliRunner @@ -84,9 +86,17 @@ def test_pl_app_download_frontend(tmp_path): assert "static" in contents -def test_pl_app_encode_icon(): - assert _can_encode_icon("📂") == True - assert _can_encode_icon("📄") == True +def test_pl_app_encode_icon(monkeypatch): + stdout_mock = Mock(wraps=sys.stdout) + monkeypatch.setattr(sys, "stdout", stdout_mock) + + stdout_mock.encoding = "utf-8" + assert _can_encode_icon("📂") + assert _can_encode_icon("📄") + + stdout_mock.encoding = "ascii" + assert not _can_encode_icon("📂") + assert not _can_encode_icon("📄") @pytest.mark.parametrize(