Consolidate text wrapping support for console printing #1722

rmartin16 · 2024-04-07T18:34:49Z

Changes

This consolidates the text wrapping support in Console from its original implementation in NewCommand.

Notes

If there isn't objection, the next step is to apply this everywhere the formatting is implemented directly in the code.
That's a lot of places, though....so, I'm not sure yet if I'll try to do that in this PR or not.
Notably, this will likely require converting the liberal use of multil-ine f"""...""" string definitions in to strings without newlines so Console.textwrap() can format them.
Additionally, I think it would be ideal to textwrap the boxed **** WARNING **** messages...but that'll also take some thought for implementation.

PR Checklist:

All new features have been tested
All new features have been documented
I have read the CONTRIBUTING.md file
I will abide by the code of conduct

rmartin16 · 2024-04-07T21:20:20Z

FYI. I've seen this error twice today now.

flake8.exceptions.FailedToLoadPlugin: Flake8 failed to load plugin "pyflakes" due to source code string cannot contain null bytes.

https://github.com/beeware/briefcase/actions/runs/8591482948/job/23540572715?pr=1722#step:11:61

freakboy3742 · 2024-04-07T22:25:41Z

FYI. I've seen this error twice today now.

Any idea of the cause of this one? It's evidently transient, because it doesn't survive a re-run; but it's not a mode of failure I've seen before, and it doesn't read like an obvious networking issue (which is the usual GitHub Actions CI issue). Maybe a glitchy NFS mount?

rmartin16 · 2024-04-07T22:49:31Z

hmm....I'm inclined to say it wasn't a low-level protocol error/blip....since the error was identical both times:

  File "/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/flake8/plugins/pyflakes.py", line 10, in <module>
    import pyflakes.checker
  File "/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/pyflakes/checker.py", line 12, in <module>
    import doctest
SyntaxError: source code string cannot contain null bytes

Other instance: https://github.com/beeware/briefcase/actions/runs/8590845652/job/23538952152#step:11:61

I wonder if GitHub has a corrupted version of doctest in their caches somehow....

freakboy3742

Broadly looks good; a couple of minor suggestions inline.

freakboy3742 · 2024-04-08T05:14:35Z

src/briefcase/cmdline.py

@@ -36,19 +35,18 @@
 ]


-def parse_cmdline(args):
+def parse_cmdline(args, console: Console = Console()):


The use of the default here means a Console() instance will always be created (as the default argument is evaluated when the function is defined). Passing in None, and constructing if the argument is None would be preferable.

good point; defaulting to None now

freakboy3742 · 2024-04-08T05:18:13Z

src/briefcase/cmdline.py

    """Parses the command line to determine the Command and its arguments.

    :param args: the arguments provided at the command line
+    :param console:


Documentation required here.

freakboy3742 · 2024-04-08T05:21:17Z

tests/console/Console/test_textwrap.py

+            "roll the image, make it flutter. We can change the focus to a soft blur or\n"
+            "sharpen it to crystal clarity. For the next hour, sit quietly, and we will\n"
+            "control all that you see and hear. We repeat: There is nothing wrong with your\n"
+            "television set.",


freakboy3742 · 2024-04-08T05:23:22Z

tests/console/Console/test_textwrap.py

+)
+def test_textwrap(in_text, out_text):
+    """Text is formatted as expected."""
+    assert Console().textwrap(in_text) == out_text


Possibly overkill, but would it be worth testing a range of output sizes here to confirm that breaking works as expected?

I tried to cover most test cases; so, I'm curious which additional ones you think might be useful?

First case is just a line that will not be broken.

Second case contains a line break before the default break point to ensure line breaks in the text are respected.

Third case takes the second case to an extreme and at least confirms line breaks in the text handling will remain consistent over time.

Fourth case is another common case of a paragraph to format...although, I varied the line length throughout.

I was thinking more about the adaptation to the width argument in the line wrap - i.e., that wrapping with width of 30/50/80/120 yield different breaks.

ohh...ok; for the width argument tests; gotcha

- Only instantiate Console in cmdline.py if not passed - Broaden tests for textwrap width override

rmartin16 · 2024-04-08T06:23:12Z

Do you have any objections to applying this to the rest of the project?

This was prompted from my work on "build tracking"....so, I think I'll still probably delay actually updating all multi-line output from this PR but wanted to make sure we're on the same page for transitioning from the f"""...""" text blocks.

P.S. furthermore, I'm targeting to get this PR and my "linux abstractions" updates in before PyCon....since I'm expecting a release before then. just fyi. (third on that list is Toga typing but probably not targeting pre-pycon.)

freakboy3742 · 2024-04-08T06:42:26Z

Do you have any objections to applying this to the rest of the project?

None at all. Anywhere that we're currently doing manual line breaks is definitely a candidate for being cleaned up in this way. The only place that won't be a drop-in replacement are the "banner" warnings (e.g., the warning when JAVA_HOME doesn't point at a JDK - but I'd argue those should be factored out into some sort of common "banner" mechanism as well (although that doesn't need to be part of this PR unless you're feeling enthused).

P.S. furthermore, I'm targeting to get this PR and my "linux abstractions" updates in before PyCon....since I'm expecting a release before then. just fyi. (third on that list is Toga typing but probably not targeting pre-pycon.)

Agreed - I'm expecting a release of both Toga and Briefcase before PyCon. For planning purposes, I'd say we want to have a release candidate around May 1, so that any release issues get caught before we're live in the middle of a tutorial.

rmartin16 · 2024-04-08T07:16:22Z

All right; good deal. As a correction, I meant I'm targeting to get "build tracking" and "linux abstractions" in before PyCon....so, I'll shoot for May 1...that may turn out to be optimistic, though.

As for this PR, feel free to merge when you're comfortable with it and I'm return to the updating the rest of the text blocks later.

freakboy3742

👍

rmartin16 force-pushed the textwrap branch 4 times, most recently from ebbcaee to 646020a Compare April 7, 2024 20:29

Consolidate text wrapping support for console printing

d3a55f1

rmartin16 force-pushed the textwrap branch from 646020a to d3a55f1 Compare April 7, 2024 20:41

rmartin16 marked this pull request as ready for review April 7, 2024 21:22

freakboy3742 requested changes Apr 8, 2024

View reviewed changes

textwrap support revisions

526baf5

- Only instantiate Console in cmdline.py if not passed - Broaden tests for textwrap width override

freakboy3742 approved these changes Apr 8, 2024

View reviewed changes

freakboy3742 merged commit 31a8b91 into beeware:main Apr 8, 2024
51 checks passed

rmartin16 deleted the textwrap branch April 8, 2024 22:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consolidate text wrapping support for console printing #1722

Consolidate text wrapping support for console printing #1722

rmartin16 commented Apr 7, 2024 •

edited

Loading

rmartin16 commented Apr 7, 2024

freakboy3742 commented Apr 7, 2024

rmartin16 commented Apr 7, 2024

freakboy3742 left a comment

freakboy3742 Apr 8, 2024

rmartin16 Apr 8, 2024

freakboy3742 Apr 8, 2024

rmartin16 Apr 8, 2024

freakboy3742 Apr 8, 2024

freakboy3742 Apr 8, 2024

rmartin16 Apr 8, 2024

freakboy3742 Apr 8, 2024

rmartin16 Apr 8, 2024

rmartin16 commented Apr 8, 2024

freakboy3742 commented Apr 8, 2024

rmartin16 commented Apr 8, 2024

freakboy3742 left a comment

Consolidate text wrapping support for console printing #1722

Consolidate text wrapping support for console printing #1722

Conversation

rmartin16 commented Apr 7, 2024 • edited Loading

Changes

Notes

PR Checklist:

rmartin16 commented Apr 7, 2024

freakboy3742 commented Apr 7, 2024

rmartin16 commented Apr 7, 2024

freakboy3742 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmartin16 commented Apr 8, 2024

freakboy3742 commented Apr 8, 2024

rmartin16 commented Apr 8, 2024

freakboy3742 left a comment

Choose a reason for hiding this comment

rmartin16 commented Apr 7, 2024 •

edited

Loading