-
-
Notifications
You must be signed in to change notification settings - Fork 473
Add helper for formatting long warning messages #2563
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
base: main
Are you sure you want to change the base?
Changes from all commits
d345b65
072fcc0
748b6d0
ebee442
b8c9b3a
8e13003
907659c
f91e1e9
d5501f8
ca633ab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| ``Warning_banner`` static method added to ``Console`` class. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -250,6 +250,97 @@ def export_log(self): | |
| """Export the text of the entire log; the log is also cleared.""" | ||
| return self._log_impl.export_text() | ||
|
|
||
| def warning_banner( | ||
| self, | ||
| message: str, | ||
| title: str = "", | ||
| width: int = 80, | ||
| border_char: str = "*", | ||
| ) -> str: | ||
| """Format warning banner message inside an asterisk border box. It is possible | ||
| to input title/message as multiline or single string. To manually split the | ||
| message into paragraphs, use the "\\n" character. If you need empty lines, use | ||
| the "\\n\\n" sequence. | ||
| :param message: The message to format inside the box. | ||
| :param title: The title of the box. If provided, appears centered at the top. | ||
| :param width: The total width of the box in characters. Defaults to 80. | ||
| :param border_char: Character to use for the box border. Defaults to "*". | ||
| :return: The formatted message enclosed in a bordered box. | ||
| """ | ||
|
|
||
| # If message and title are both empty or width is not an integer, | ||
| # return empty string | ||
| if not any((message, title)) or not isinstance(width, int): | ||
| return "" | ||
|
|
||
| def dedent_and_split(text): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than defining this as an inline function, it should be a private method on the base Console class (i.e. , I'd also suggest that we can add an extra affordance around the handling of input strings. As written, I think this requires that the input string has a trailing |
||
| """Dedent and split text into paragraphs by manual line breaks.""" | ||
| # Remove common leading whitespace | ||
| text = textwrap.dedent(text).strip() | ||
| # replace line breaks with spaces | ||
| text = text.replace("\n", " ") | ||
| # split the message into paragraphs by manual line breaks | ||
| return text.split("\\n") | ||
|
|
||
| # Create border line | ||
| border_line = border_char * width | ||
| # create lines array with opening line of the box | ||
| lines = [border_line] | ||
|
|
||
| # if title exists, format title in the box | ||
| if title: | ||
| if not isinstance(title, str): | ||
| return "" | ||
|
Comment on lines
+293
to
+294
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we're doing error handling, we should be raising an error, not failing silently. But again - is any error handling needed? |
||
|
|
||
| # width of title line inside the box | ||
| inner_width = width - 4 | ||
|
|
||
| # spilt title into paragraphs | ||
| paragraphs = dedent_and_split(title) | ||
|
|
||
| for paragraph in paragraphs: | ||
| paragraph = paragraph.strip() | ||
| if paragraph: # Non-empty paragraph | ||
| # Wrap paragraph to lines to fit the width of the box | ||
| wrapped_title_lines = textwrap.wrap(paragraph, width=width - 6) | ||
|
|
||
| for line in wrapped_title_lines: | ||
| # Center each line within the available space | ||
| padded_line = line.center(inner_width) | ||
| # add line to the box | ||
| lines.append(f"{border_char * 2}{padded_line}{border_char * 2}") | ||
| else: # Empty paragraph (preserve blank lines) | ||
| lines.append( | ||
| f"{border_char * 2}{''.center(inner_width)}{border_char * 2}" | ||
| ) | ||
|
|
||
| # closing line of title in the box | ||
| lines.append(border_line) | ||
|
|
||
| # If message is not empty, add it to the box | ||
| if message: | ||
| if not isinstance(message, str): | ||
| return "" | ||
|
Comment on lines
+323
to
+324
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above - is this error handling needed; and if it is, shouldn't it be error handling? |
||
|
|
||
| # spilt message into paragraphs | ||
| paragraphs = dedent_and_split(message) | ||
|
|
||
| for paragraph in paragraphs: | ||
| paragraph = paragraph.strip() | ||
| if paragraph: # Non-empty paragraph | ||
| # Wrap paragraph to lines to fit the width of the box | ||
| wrapped_lines = textwrap.wrap(paragraph, width=width) | ||
| lines.extend(wrapped_lines) | ||
| else: # Empty paragraph (preserve blank lines) | ||
| lines.append("") | ||
|
|
||
| # closing line of message | ||
| lines.append(border_line) | ||
|
|
||
| # merge lines into a single string and return | ||
| return "\n".join(lines) | ||
|
|
||
| ################################################################# | ||
| # Logging controls | ||
| ################################################################# | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,212 @@ | ||||||||||||||
| import pytest | ||||||||||||||
|
|
||||||||||||||
| name = "Anton" | ||||||||||||||
| issue_idx = 2559 | ||||||||||||||
|
|
||||||||||||||
| source_msg = f"""\ | ||||||||||||||
| Hi there! My name is {name}, and I'm a Python developer.\\n\\n | ||||||||||||||
| This is my very first | ||||||||||||||
| experience in open source | ||||||||||||||
| development. | ||||||||||||||
| I like your project, and I'm excited to contribute to it.\ | ||||||||||||||
| """ | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the use of an f-string testing anything specific here? I'm fairly sure this can be simplified to a static string definition. |
||||||||||||||
|
|
||||||||||||||
| source_title = f"""\ | ||||||||||||||
| \\nINFO: I TRIED TO SOLVE THIS ISSUE #{issue_idx}.\\n\\n | ||||||||||||||
| IT SEEMED TO ME RATHER SIMPLE, | ||||||||||||||
| BUT I GOT VERY GOOD EXPERIENCE\ | ||||||||||||||
| """ | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| @pytest.mark.parametrize( | ||||||||||||||
| ("test_name", "message", "title", "width", "expected"), | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no need to include a descriptive test name - your test doesn't use it anyway:
Suggested change
If you do want to provide a test identifier, use |
||||||||||||||
| [ | ||||||||||||||
| ("Test 1: Empty message and title", "", "", 80, ""), | ||||||||||||||
| ( | ||||||||||||||
| "Test 2: Default width (80) with title and message", | ||||||||||||||
| source_msg, | ||||||||||||||
| source_title, | ||||||||||||||
| 80, | ||||||||||||||
| """\ | ||||||||||||||
| ******************************************************************************** | ||||||||||||||
| ** ** | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This extra blank link in the title is a little odd. It wouldn't match any existing uses of warning banners. |
||||||||||||||
| ** INFO: I TRIED TO SOLVE THIS ISSUE #2559. ** | ||||||||||||||
| ** ** | ||||||||||||||
| ** IT SEEMED TO ME RATHER SIMPLE, BUT I GOT VERY GOOD EXPERIENCE ** | ||||||||||||||
| ******************************************************************************** | ||||||||||||||
| Hi there! My name is Anton, and I'm a Python developer. | ||||||||||||||
| This is my very first experience in open source development. I like your | ||||||||||||||
| project, and I'm excited to contribute to it. | ||||||||||||||
| ********************************************************************************\ | ||||||||||||||
| """, | ||||||||||||||
| ), | ||||||||||||||
| ( | ||||||||||||||
| "Test 3: Narrow width (40) with title and message", | ||||||||||||||
| source_msg, | ||||||||||||||
| source_title, | ||||||||||||||
| 40, | ||||||||||||||
| """\ | ||||||||||||||
| **************************************** | ||||||||||||||
| ** ** | ||||||||||||||
| ** INFO: I TRIED TO SOLVE THIS ISSUE ** | ||||||||||||||
| ** #2559. ** | ||||||||||||||
| ** ** | ||||||||||||||
| ** IT SEEMED TO ME RATHER SIMPLE, BUT ** | ||||||||||||||
| ** I GOT VERY GOOD EXPERIENCE ** | ||||||||||||||
| **************************************** | ||||||||||||||
| Hi there! My name is Anton, and I'm a | ||||||||||||||
| Python developer. | ||||||||||||||
| This is my very first experience in open | ||||||||||||||
| source development. I like your project, | ||||||||||||||
| and I'm excited to contribute to it. | ||||||||||||||
| ****************************************\ | ||||||||||||||
| """, | ||||||||||||||
| ), | ||||||||||||||
| ( | ||||||||||||||
| "Test 4: Default width (80) without title", | ||||||||||||||
| source_msg, | ||||||||||||||
| None, | ||||||||||||||
| 80, | ||||||||||||||
| """\ | ||||||||||||||
| ******************************************************************************** | ||||||||||||||
| Hi there! My name is Anton, and I'm a Python developer. | ||||||||||||||
| This is my very first experience in open source development. I like your | ||||||||||||||
| project, and I'm excited to contribute to it. | ||||||||||||||
| ********************************************************************************\ | ||||||||||||||
| """, | ||||||||||||||
| ), | ||||||||||||||
| ( | ||||||||||||||
| "Test 5: Custom width (60) with title and message", | ||||||||||||||
| source_msg, | ||||||||||||||
| source_title, | ||||||||||||||
| 60, | ||||||||||||||
| """\ | ||||||||||||||
| ************************************************************ | ||||||||||||||
| ** ** | ||||||||||||||
| ** INFO: I TRIED TO SOLVE THIS ISSUE #2559. ** | ||||||||||||||
| ** ** | ||||||||||||||
| ** IT SEEMED TO ME RATHER SIMPLE, BUT I GOT VERY GOOD ** | ||||||||||||||
| ** EXPERIENCE ** | ||||||||||||||
| ************************************************************ | ||||||||||||||
| Hi there! My name is Anton, and I'm a Python developer. | ||||||||||||||
| This is my very first experience in open source development. | ||||||||||||||
| I like your project, and I'm excited to contribute to it. | ||||||||||||||
| ************************************************************\ | ||||||||||||||
| """, | ||||||||||||||
| ), | ||||||||||||||
| ( | ||||||||||||||
| "Test 6: Very narrow width (30) with title and message", | ||||||||||||||
| source_msg, | ||||||||||||||
| source_title, | ||||||||||||||
| 30, | ||||||||||||||
| """\ | ||||||||||||||
| ****************************** | ||||||||||||||
| ** ** | ||||||||||||||
| ** INFO: I TRIED TO SOLVE ** | ||||||||||||||
| ** THIS ISSUE #2559. ** | ||||||||||||||
| ** ** | ||||||||||||||
| ** IT SEEMED TO ME RATHER ** | ||||||||||||||
| ** SIMPLE, BUT I GOT VERY ** | ||||||||||||||
| ** GOOD EXPERIENCE ** | ||||||||||||||
| ****************************** | ||||||||||||||
| Hi there! My name is Anton, | ||||||||||||||
| and I'm a Python developer. | ||||||||||||||
| This is my very first | ||||||||||||||
| experience in open source | ||||||||||||||
| development. I like your | ||||||||||||||
| project, and I'm excited to | ||||||||||||||
| contribute to it. | ||||||||||||||
| ******************************\ | ||||||||||||||
| """, | ||||||||||||||
| ), | ||||||||||||||
| ( | ||||||||||||||
| "Test 7: Message with only title (empty message)", | ||||||||||||||
| "", | ||||||||||||||
| source_title, | ||||||||||||||
| 80, | ||||||||||||||
| """\ | ||||||||||||||
| ******************************************************************************** | ||||||||||||||
| ** ** | ||||||||||||||
| ** INFO: I TRIED TO SOLVE THIS ISSUE #2559. ** | ||||||||||||||
| ** ** | ||||||||||||||
| ** IT SEEMED TO ME RATHER SIMPLE, BUT I GOT VERY GOOD EXPERIENCE ** | ||||||||||||||
| ********************************************************************************\ | ||||||||||||||
| """, | ||||||||||||||
| ), | ||||||||||||||
| ( | ||||||||||||||
| "Test 8: Very short message with title", | ||||||||||||||
| "Short message", | ||||||||||||||
| "Short title", | ||||||||||||||
| 80, | ||||||||||||||
| """\ | ||||||||||||||
| ******************************************************************************** | ||||||||||||||
| ** Short title ** | ||||||||||||||
| ******************************************************************************** | ||||||||||||||
| Short message | ||||||||||||||
| ********************************************************************************\ | ||||||||||||||
| """, | ||||||||||||||
| ), | ||||||||||||||
| ( | ||||||||||||||
| "Test 9: Message and title lengths equal to box width", | ||||||||||||||
| "Length of message is equal to box _width", | ||||||||||||||
| "Length of title is e-l to box w-th", | ||||||||||||||
| 40, | ||||||||||||||
| """\ | ||||||||||||||
| **************************************** | ||||||||||||||
| ** Length of title is e-l to box w-th ** | ||||||||||||||
| **************************************** | ||||||||||||||
| Length of message is equal to box _width | ||||||||||||||
| ****************************************\ | ||||||||||||||
| """, | ||||||||||||||
| ), | ||||||||||||||
| ( | ||||||||||||||
| "Test 10: Message and title lengths longer +1 symbol to box width", | ||||||||||||||
| "Length of message is equal to box __width", | ||||||||||||||
| "Length of title is e-l to box _w-th", | ||||||||||||||
| 40, | ||||||||||||||
| """\ | ||||||||||||||
| **************************************** | ||||||||||||||
| ** Length of title is e-l to box _w- ** | ||||||||||||||
| ** th ** | ||||||||||||||
| **************************************** | ||||||||||||||
| Length of message is equal to box | ||||||||||||||
| __width | ||||||||||||||
| ****************************************\ | ||||||||||||||
| """, | ||||||||||||||
| ), | ||||||||||||||
| ( | ||||||||||||||
| "Test 11: Invalid input types", | ||||||||||||||
| 5, | ||||||||||||||
| 5, | ||||||||||||||
| 40, | ||||||||||||||
| "", | ||||||||||||||
| ), | ||||||||||||||
| ( | ||||||||||||||
| "Test 12: Invalid input types", | ||||||||||||||
| "5", | ||||||||||||||
| "5", | ||||||||||||||
| "40", | ||||||||||||||
| "", | ||||||||||||||
| ), | ||||||||||||||
| ], | ||||||||||||||
| ) | ||||||||||||||
| def test_warning_banner(console, test_name, message, title, width, expected): | ||||||||||||||
| """Test warning_banner with various inputs.""" | ||||||||||||||
| if title is None: | ||||||||||||||
| msg = console.warning_banner(message, width=width) | ||||||||||||||
| else: | ||||||||||||||
| msg = console.warning_banner(message, title=title, width=width) | ||||||||||||||
|
|
||||||||||||||
| # # Debug output if needed | ||||||||||||||
| # print('\n\n' + test_name) | ||||||||||||||
| # print("Generated:") | ||||||||||||||
| # print(msg) | ||||||||||||||
| # print("Expected:") | ||||||||||||||
| # print(expected) | ||||||||||||||
|
|
||||||||||||||
|
Comment on lines
+205
to
+211
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no need to retain this debug code.
Suggested change
|
||||||||||||||
| assert msg == expected | ||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it make more sense to raise an error if the arguments aren't the right type?
It's also worth asking if any error handling needed at all. This isn't a public API - it's an internal utility; we don't have to be rock solid in the API we expose, because we're in control of how the method is used.