Skip to content

fix: skip running flutter doctor on windows if no_rich_output=True #4108

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

Merged
merged 2 commits into from
Oct 18, 2024

Conversation

ndonkoHenri
Copy link
Contributor

@ndonkoHenri ndonkoHenri commented Oct 6, 2024

Description

Error message

  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 360, 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 864, in __exit__
    self._exit_buffer()
  File "C:\hostedtoolcache\windows\Python\3.12.2\x64\Lib\site-packages\rich\console.py", line 822, in _exit_buffer
    self._check_buffer()
  File "C:\hostedtoolcache\windows\Python\3.12.2\x64\Lib\site-packages\rich\console.py", line 2024, in _check_buffer
    self._write_buffer()
  File "C:\hostedtoolcache\windows\Python\3.12.2\x64\Lib\site-packages\rich\console.py", line 2060, in _write_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 '\u221a' in position 0: character maps to <undefined>

\u221a' represents the square root character, which comes from flutter doctor.

Summary by Sourcery

Fix a bug by skipping the execution of flutter doctor on Windows when no_rich_output=True to avoid Unicode encoding errors. Improve code readability by refactoring the handling of platform and output settings.

Bug Fixes:

  • Skip running flutter doctor on Windows if no_rich_output=True to prevent Unicode encoding errors.

Enhancements:

  • Refactor code to use self.no_rich_output and self.current_platform for better readability and maintainability.

Copy link
Contributor

sourcery-ai bot commented Oct 6, 2024

Reviewer's Guide by Sourcery

This pull request addresses an issue with running flutter doctor on Windows when no_rich_output=True. The main changes involve skipping the flutter doctor command in this specific scenario and refactoring some code for better organization and readability.

Sequence diagram for handling flutter doctor execution

sequenceDiagram
    actor User
    participant CLI as Command Line Interface
    participant BuildCmd as Build Command
    participant Flutter as Flutter

    User->>CLI: Run build command
    CLI->>BuildCmd: Initialize build
    BuildCmd->>BuildCmd: Check no_rich_output and platform
    alt no_rich_output is True and platform is Windows
        BuildCmd->>BuildCmd: Skip flutter doctor
    else
        BuildCmd->>Flutter: Run flutter doctor
    end
    BuildCmd->>CLI: Complete build process
    CLI->>User: Return build result
Loading

File-Level Changes

Change Details Files
Skip running flutter doctor on Windows when no_rich_output is True
  • Add a condition to check if no_rich_output is True and the platform is Windows
  • Wrap the run_flutter_doctor call in an if statement to skip it when the condition is met
sdk/python/packages/flet/src/flet/cli/commands/build.py
Refactor code for better organization and readability
  • Move import statements to improve organization
  • Introduce self.no_rich_output and self.current_platform as class attributes
  • Replace local variables with class attributes for consistency
sdk/python/packages/flet/src/flet/cli/commands/build.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @ndonkoHenri - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider adding a brief comment explaining why flutter doctor needs to be skipped on Windows when no_rich_output is True. This will help future maintainers understand the rationale behind this platform-specific behavior.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 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 and I'll use the feedback to improve your reviews.

@FeodorFitsner
Copy link
Contributor

Fix a conflict here please.

@FeodorFitsner FeodorFitsner merged commit 0d62c6f into main Oct 18, 2024
2 checks passed
@FeodorFitsner FeodorFitsner deleted the fix-encoding-issues-with-flutter-doctor branch October 18, 2024 18:41
pengwon added a commit to pengwon/flet that referenced this pull request Oct 30, 2024
* main: (31 commits)
  Migrate `colors` and `icons` variables to Enums (flet-dev#4180)
  feat: expose more properties in Controls (flet-dev#4105)
  feat!: Refactor `Badge` Control to a Dataclass; create new `Control.badge` property (flet-dev#4077)
  fix: clicking on `CupertinoContextMenuAction` doesn't close context menu (flet-dev#3948)
  fix dropdown `max_menu_height` (flet-dev#3974)
  Fix undefined name "Future" --> asyncio.Future (flet-dev#4230)
  wrap ListTile in material widget (flet-dev#4206)
  Update CONTRIBUTING.md (flet-dev#4208)
  TextField: suffix_icon, prefix_icon and icon can be Control or str (flet-dev#4173)
  feat!: enhance `Map` control (flet-dev#3994)
  skip running flutter doctor on windows if no_rich_output is True (flet-dev#4108)
  add --pyinstaller-build-args to pack cli command (flet-dev#4187)
  fix/feat: make `SearchBar`'s view height adjustable; add new properties (flet-dev#4039)
  fix: prevent button `style` from being modified in `before_update()` (flet-dev#4181)
  fix: disabling filled buttons is not visually respected (flet-dev#4090)
  when `label` is set, use `MainAxisSize.min` for the `Row` (flet-dev#3998)
  fix: `NavigationBarDestination.disabled` has no visual effect (flet-dev#4073)
  fix autofill in CupertinoTextField (flet-dev#4103)
  Linechart: jsonDecode tooltip before displaying (flet-dev#4069)
  fixed bgcolor, color and elevation (flet-dev#4126)
  ...
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.

2 participants