feat(build cli): `--no-rich-output` flag to prevent rich output #3708

ndonkoHenri · 2024-07-25T09:39:26Z

Context

(this PR was completed by #3735)
The error in the below code block occured in a github action when trying to build a simple Windows application. It is raised because github action cli uses cp1252 encoding, causing characters like ✅ not to be displayed.
In this PR, we make it possible through the new optional --no-rich-output flag to prevent rich output (✅) from being displayed.

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "C:\hostedtoolcache\windows\Python\3.12.2\x64\Scripts\flet.exe\__main__.py", line 7, in <module>
  File "C:\hostedtoolcache\windows\Python\3.12.2\x64\Lib\site-packages\flet\cli\cli.py", line 88, in main
    args.handler(args)
  File "C:\hostedtoolcache\windows\Python\3.12.2\x64\Lib\site-packages\flet\cli\commands\build.py", line 335, in handle
    with console.status(
  File "C:\hostedtoolcache\windows\Python\3.12.2\x64\Lib\site-packages\rich\status.py", line 106, in __exit__
    self.stop()
  File "C:\hostedtoolcache\windows\Python\3.12.2\x64\Lib\site-packages\rich\status.py", line 91, in stop
    self._live.stop()
  File "C:\hostedtoolcache\windows\Python\3.12.2\x64\Lib\site-packages\rich\live.py", line 147, in stop
    with self.console:
  File "C:\hostedtoolcache\windows\Python\3.12.2\x64\Lib\site-packages\rich\console.py", line 865, in __exit__
    self._exit_buffer()
  File "C:\hostedtoolcache\windows\Python\3.12.2\x64\Lib\site-packages\rich\console.py", line 823, in _exit_buffer
    self._check_buffer()
  File "C:\hostedtoolcache\windows\Python\3.12.2\x64\Lib\site-packages\rich\console.py", line [202](https://github.com/ndonkoHenri/test-flet-github-actions/actions/runs/10085185884/job/27885499164#step:6:203)7, in _check_buffer
    legacy_windows_render(buffer, LegacyWindowsTerm(self.file))
  File "C:\hostedtoolcache\windows\Python\3.12.2\x64\Lib\site-packages\rich\_windows_renderer.py", line 17, in legacy_windows_render
    term.write_styled(text, style)
  File "C:\hostedtoolcache\windows\Python\3.12.2\x64\Lib\site-packages\rich\_win32_console.py", line 442, in write_styled
    self.write_text(text)
  File "C:\hostedtoolcache\windows\Python\3.12.2\x64\Lib\site-packages\rich\_win32_console.py", line 403, in write_text
    self.write(text)
  File "C:\hostedtoolcache\windows\Python\3.12.2\x64\Lib\encodings\cp1252.py", line 19, in encode
    return codecs.charmap_encode(input,self.errors,encoding_table)[0]
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
UnicodeEncodeError: 'charmap' codec can't encode character '\u2705' in position 1: character maps to <undefined>
           gh:flet-dev/flet-build-template with ref 0.23.2

Also, I created a method which runs flutter doctor in cases where the build fails.

Summary by Sourcery

This pull request adds a new --no-rich-output flag to the build CLI, allowing users to disable rich output and use plain text instead. This is particularly useful for environments that do not support rich text encoding, such as certain GitHub Actions runners. Additionally, console log messages have been updated to respect this new flag.

New Features:
- Introduced a --no-rich-output flag to the build CLI to disable rich output and use plain text instead.
Enhancements:
- Updated various console log messages to conditionally use plain text or rich output based on the --no-rich-output flag.

sourcery-ai · 2024-07-25T09:39:34Z

Reviewer's Guide by Sourcery

This pull request introduces a new --no-rich-output flag to the build CLI, allowing users to disable rich output (e.g., emojis) and use plain text instead. This is particularly useful for environments that do not support certain characters, such as GitHub Actions using cp1252 encoding. The implementation involves adding a new command-line argument and updating various parts of the build process to respect this flag.

File-Level Changes

Files	Changes
`sdk/python/packages/flet/src/flet/cli/commands/build.py`	Added support for the `--no-rich-output` flag, updated console log messages to conditionally use plain text or rich output, and introduced a new method to run `flutter doctor`.

Tips

Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
Continue your discussion with Sourcery by replying directly to review comments.
You can change your review settings at any time by accessing your dashboard:
- Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
- Change the review language;
You can always contact us if you have any questions or feedback.

sourcery-ai

Hey @ndonkoHenri - I've reviewed your changes and found some issues that need to be addressed.

Blocking issues:

Incorrect dictionary key access (link)

Here's what I looked at during the review

🔴 General issues: 1 blocking issue, 5 other issues
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.}

sourcery-ai · 2024-07-25T09:40:46Z

sdk/python/packages/flet/src/flet/cli/commands/build.py

+        self.dart_exe = None
+        self.verbose = None
+        self.flutter_dir = None
+        self.flutter_exe = None


suggestion: Consider initializing instance variables in the constructor

It might be more appropriate to initialize self.dart_exe, self.verbose, self.flutter_dir, and self.flutter_exe in the constructor to ensure they are always set when an instance of the class is created.

Suggested change

self.dart_exe = None

self.verbose = None

self.flutter_dir = None

self.flutter_exe = None

def __init__(self, parser: argparse.ArgumentParser) -> None:

super().__init__(parser)

self.dart_exe: Optional[str] = None

self.verbose: Optional[bool] = None

self.flutter_dir: Optional[str] = None

self.flutter_exe: Optional[str] = None

self.platforms = {

"windows": {

sourcery-ai · 2024-07-25T09:40:46Z

sdk/python/packages/flet/src/flet/cli/commands/build.py

+        checkmark = (
+            "[green]✓[/]"
+            if options.no_rich_output or get_bool_env_var("FLET_BUILD_NO_RICH_OUTPUT")
+            else "✅"


issue (bug_risk): Inverted logic for checkmark assignment

The logic for checkmark assignment seems inverted. It should be if not options.no_rich_output and not get_bool_env_var("FLET_BUILD_NO_RICH_OUTPUT") to use the rich output checkmark.

sourcery-ai · 2024-07-25T09:40:46Z

sdk/python/packages/flet/src/flet/cli/commands/build.py

@@ -335,7 +355,7 @@ def handle(self, options: argparse.Namespace) -> None:
        with console.status(
            f"[bold blue]Initializing {target_platform} build... ",
            spinner="bouncingBall",
-        ) as status:
+        ) as self.status:


issue (bug_risk): Potential issue with self.status reassignment

Reassigning self.status within the with block might lead to unexpected behavior if self.status is used elsewhere in the class. Consider using a different variable name.

sdk/python/packages/flet/src/flet/cli/commands/build.py

baseplate-admin · 2024-07-29T06:54:40Z

Possibly linked to

UnicodeEncodeError is raised when packaging for Windows #3730

please merge this PR?

ndonkoHenri · 2024-07-29T11:59:31Z

@FeodorFitsner do you think we should equally handle other emojis accordingly?

no-rich-output | run flutter doctor on failed build

c5c582e

ndonkoHenri changed the title ~~feat(build cli): --no-rich-ouput flag to prevent rich output~~ feat(build cli): --no-rich-output flag to prevent rich output Jul 25, 2024

ndonkoHenri requested a review from FeodorFitsner July 25, 2024 09:40

sourcery-ai bot reviewed Jul 25, 2024

View reviewed changes

apply sourcery review-suggestions

ca09a58

ndonkoHenri linked an issue Jul 29, 2024 that may be closed by this pull request

UnicodeEncodeError is raised when packaging for Windows #3730

Closed

1 task

FeodorFitsner approved these changes Jul 29, 2024

View reviewed changes

FeodorFitsner merged commit c02c2b9 into main Jul 29, 2024
2 checks passed

ndonkoHenri mentioned this pull request Aug 10, 2024

fix(build cli): using --no-rich-output should replace all emojis to prevent UnicodeError #3735

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(build cli): `--no-rich-output` flag to prevent rich output #3708

feat(build cli): `--no-rich-output` flag to prevent rich output #3708

ndonkoHenri commented Jul 25, 2024 •

edited

Loading

sourcery-ai bot commented Jul 25, 2024 •

edited

Loading

sourcery-ai bot left a comment

sourcery-ai bot Jul 25, 2024

sourcery-ai bot Jul 25, 2024

sourcery-ai bot Jul 25, 2024

baseplate-admin commented Jul 29, 2024 •

edited

Loading

ndonkoHenri commented Jul 29, 2024

-        self.dart_exe = None
-        self.verbose = None
-        self.flutter_dir = None
-        self.flutter_exe = None
+def __init__(self, parser: argparse.ArgumentParser) -> None:
+    super().__init__(parser)
+    self.dart_exe: Optional[str] = None
+    self.verbose: Optional[bool] = None
+    self.flutter_dir: Optional[str] = None
+    self.flutter_exe: Optional[str] = None
+    self.platforms = {
+        "windows": {

feat(build cli): --no-rich-output flag to prevent rich output #3708

feat(build cli): --no-rich-output flag to prevent rich output #3708

Conversation

ndonkoHenri commented Jul 25, 2024 • edited Loading

Context

Summary by Sourcery

sourcery-ai bot commented Jul 25, 2024 • edited Loading

Reviewer's Guide by Sourcery

File-Level Changes

sourcery-ai bot left a comment

Choose a reason for hiding this comment

sourcery-ai bot Jul 25, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 25, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 25, 2024

Choose a reason for hiding this comment

baseplate-admin commented Jul 29, 2024 • edited Loading

ndonkoHenri commented Jul 29, 2024

feat(build cli): `--no-rich-output` flag to prevent rich output #3708

feat(build cli): `--no-rich-output` flag to prevent rich output #3708

ndonkoHenri commented Jul 25, 2024 •

edited

Loading

sourcery-ai bot commented Jul 25, 2024 •

edited

Loading

baseplate-admin commented Jul 29, 2024 •

edited

Loading