From 2490d0efed710bcd080ded1d2e6ba31341236fe3 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Quang Date: Tue, 13 Jul 2021 19:41:57 +0700 Subject: [PATCH 01/12] display full path of current source file in header bar Currently, the full path of the current source file will be displayed. This usually results in long path -> Need more discussions. --- pudb/debugger.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pudb/debugger.py b/pudb/debugger.py index 4ce4ccd2..c634de36 100644 --- a/pudb/debugger.py +++ b/pudb/debugger.py @@ -2642,10 +2642,8 @@ def interaction(self, exc_tuple, show_exc_dialog=True): self.current_exc_tuple = exc_tuple from pudb import VERSION - caption = [(None, - "PuDB %s - ?:help n:next s:step into b:breakpoint " - "!:python command line" - % VERSION)] + caption = [(None, "PuDB %s - ?:help - %s" % + (VERSION, self.source_code_provider.get_source_identifier()))] if self.debugger.post_mortem: if show_exc_dialog and exc_tuple is not None: From e4af74a7490d52108935f67ab3fa31c2c7c5f1ee Mon Sep 17 00:00:00 2001 From: Huy Nguyen Quang Date: Thu, 15 Jul 2021 16:37:04 +0700 Subject: [PATCH 02/12] Display path to current source file dynamically based on terminal width The path to current source file will now be calculated dynamically. If there is enough space, the full absolute path will be shown. Otherwise, the longest possible path will be shown by trimming the full absolute path from the left. If there is an update to the terminal width, the path will be recalculated when the user runs the next line of code, however the method (next, step, continue). NOTICE: Pressing Ctrl-l will redraw the screen with the current path, it doesn't recalculated the path string. --- pudb/debugger.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/pudb/debugger.py b/pudb/debugger.py index c634de36..d8299ec6 100644 --- a/pudb/debugger.py +++ b/pudb/debugger.py @@ -2642,8 +2642,23 @@ def interaction(self, exc_tuple, show_exc_dialog=True): self.current_exc_tuple = exc_tuple from pudb import VERSION - caption = [(None, "PuDB %s - ?:help - %s" % - (VERSION, self.source_code_provider.get_source_identifier()))] + separator = ' - ' + info_string = separator.join(["PuDB %s" % VERSION, "?:help"]) + + def get_source_filename(): + available_width = self.screen.get_cols_rows( + )[0] - (len(info_string) + len(separator)) + full_filename = self.source_code_provider.get_source_identifier() + if available_width > len(full_filename): + return full_filename + else: + trim_index = len(full_filename) - available_width + filename = full_filename[trim_index:] + first_dirname_index = filename.find(os.sep) + filename = filename[first_dirname_index + 1:] + return filename + + caption = separator.join([info_string, get_source_filename()]) if self.debugger.post_mortem: if show_exc_dialog and exc_tuple is not None: From 28b9a55549c05fc0290c8d3fedbb8f96b1a3a773 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Quang Date: Thu, 15 Jul 2021 17:01:33 +0700 Subject: [PATCH 03/12] replace single quote with double quote --- pudb/debugger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pudb/debugger.py b/pudb/debugger.py index 78857055..2abeec9c 100644 --- a/pudb/debugger.py +++ b/pudb/debugger.py @@ -2617,7 +2617,7 @@ def interaction(self, exc_tuple, show_exc_dialog=True): self.current_exc_tuple = exc_tuple from pudb import VERSION - separator = ' - ' + separator = " - " info_string = separator.join(["PuDB %s" % VERSION, "?:help"]) def get_source_filename(): From 1446480ebea00b11726cd32bd25be52ba5690bd5 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Quang Date: Thu, 15 Jul 2021 17:08:39 +0700 Subject: [PATCH 04/12] `caption` should be a list, not string --- pudb/debugger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pudb/debugger.py b/pudb/debugger.py index 2abeec9c..a57681bf 100644 --- a/pudb/debugger.py +++ b/pudb/debugger.py @@ -2633,7 +2633,7 @@ def get_source_filename(): filename = filename[first_dirname_index + 1:] return filename - caption = separator.join([info_string, get_source_filename()]) + caption = [(None, separator.join([info_string, get_source_filename()]))] if self.debugger.post_mortem: if show_exc_dialog and exc_tuple is not None: From eb97760ea244290d21a0772f98e878eb1bfdedcb Mon Sep 17 00:00:00 2001 From: Huy Nguyen Quang Date: Thu, 15 Jul 2021 17:49:21 +0700 Subject: [PATCH 05/12] fix bug when source filename is not available --- pudb/debugger.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pudb/debugger.py b/pudb/debugger.py index a57681bf..d1e77fad 100644 --- a/pudb/debugger.py +++ b/pudb/debugger.py @@ -2624,7 +2624,9 @@ def get_source_filename(): available_width = self.screen.get_cols_rows( )[0] - (len(info_string) + len(separator)) full_filename = self.source_code_provider.get_source_identifier() - if available_width > len(full_filename): + if (full_filename is None): + return "Source filename not available" + elif available_width > len(full_filename): return full_filename else: trim_index = len(full_filename) - available_width From 7035e9639abed4d31d3a5ddcbb06efe1a630570f Mon Sep 17 00:00:00 2001 From: Huy Nguyen Quang Date: Sat, 17 Jul 2021 18:38:28 +0700 Subject: [PATCH 06/12] Use `class Caption(urwid.Text)` to represent the header. `Caption` will handle the display of header's content, including the current source filename --- pudb/debugger.py | 39 +++++++++++++++------------------ pudb/ui_tools.py | 57 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 22 deletions(-) diff --git a/pudb/debugger.py b/pudb/debugger.py index d1e77fad..befa05a9 100644 --- a/pudb/debugger.py +++ b/pudb/debugger.py @@ -858,7 +858,8 @@ def helpside(w, size, key): ], dividechars=1) - self.caption = urwid.Text("") + from pudb.ui_tools import Caption + self.caption = Caption("") header = urwid.AttrMap(self.caption, "header") self.top = SignalWrap(urwid.Frame( urwid.AttrMap(self.columns, "background"), @@ -2618,37 +2619,31 @@ def interaction(self, exc_tuple, show_exc_dialog=True): from pudb import VERSION separator = " - " - info_string = separator.join(["PuDB %s" % VERSION, "?:help"]) - - def get_source_filename(): - available_width = self.screen.get_cols_rows( - )[0] - (len(info_string) + len(separator)) - full_filename = self.source_code_provider.get_source_identifier() - if (full_filename is None): - return "Source filename not available" - elif available_width > len(full_filename): - return full_filename - else: - trim_index = len(full_filename) - available_width - filename = full_filename[trim_index:] - first_dirname_index = filename.find(os.sep) - filename = filename[first_dirname_index + 1:] - return filename - - caption = [(None, separator.join([info_string, get_source_filename()]))] + pudb_version = "PuDB %s" % VERSION + hotkey = "?:help" + if self.source_code_provider.get_source_identifier(): + source_filename = self.source_code_provider.get_source_identifier() + else: + source_filename = "source filename is unavailable" + caption = [(None, pudb_version), + (None, separator), + (None, hotkey), + (None, separator), + (None, source_filename) + ] if self.debugger.post_mortem: if show_exc_dialog and exc_tuple is not None: self.show_exception_dialog(exc_tuple) caption.extend([ - (None, " "), + (None, separator), ("warning", "[POST-MORTEM MODE]") ]) elif exc_tuple is not None: caption.extend([ - (None, " "), - ("warning", "[PROCESSING EXCEPTION - hit 'e' to examine]") + (None, separator), + ("warning", "[PROCESSING EXCEPTION, hit 'e' to examine]") ]) self.caption.set_text(caption) diff --git a/pudb/ui_tools.py b/pudb/ui_tools.py index 4931ff41..479fc50b 100644 --- a/pudb/ui_tools.py +++ b/pudb/ui_tools.py @@ -332,4 +332,61 @@ def keypress(self, size, key): return result + +class Caption(urwid.Text): + def __init__(self, markup, separator=" - "): + self.separator = separator + super().__init__(markup) + + def set_text(self, markup): + super().set_text(markup) + if len(markup) > 0: + # Assume the format of caption is: + # [optional_alert] + caption, _ = self.get_text() + caption_elements = caption.split(self.separator) + self.pudb_version = caption_elements[0] + self.hotkey = caption_elements[1] + self.full_source_filename = caption_elements[2] + self.optional_alert = caption_elements[3] if len( + caption_elements) > 3 else "" + else: + self.pudb_version = self.hotkey = "" + self.full_source_filename = self.optional_alert = "" + + def rows(self, size, focus=False): + # Always return 1 to avoid + # `assert head.rows() == hrows, "rows, render mismatch")` + # in urwid.Frame.render() in urwid/container.py + return 1 + + def render(self, size, focus=False): + maxcol = size[0] + if super().rows(size) > 1: + filename = self.get_shortened_source_filename(size) + else: + filename = self.full_source_filename + caption = self.separator.join( + [self.pudb_version, self.hotkey, filename, self.optional_alert] + ).strip(self.separator) + if self.optional_alert: + attr = [("warning", len(caption))] + else: + attr = [(None, 0)] + + return make_canvas([caption], [attr], maxcol) + + def get_shortened_source_filename(self, size): + import os + maxcol = size[0] + + occupied_width = (len(self.pudb_version) + len(self.hotkey) + + len(self.optional_alert) + len(self.separator)*3) + available_width = maxcol - occupied_width + trim_index = len(self.full_source_filename) - available_width + filename = self.full_source_filename[trim_index:] + first_dirname_index = filename.find(os.sep) + filename = filename[first_dirname_index + 1:] + + return filename # }}} From f1aef6c90271d8cb51a47d7e627e7e51839db316 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Quang Date: Wed, 21 Jul 2021 15:40:08 +0700 Subject: [PATCH 07/12] Redesign `class Caption(urwid.Text)` and fix bug - Caption is now composed of a CaptionParts namedtuple and a separator, both will be passed to Caption explicitly - separator and parts in CaptionParts will be tuples of the form (attribute, text), similar to text markup in urwid - Caption no longer overestimates the amount of text to be removed. --- pudb/debugger.py | 37 ++++------ pudb/ui_tools.py | 116 +++++++++++++++++++----------- test/test_caption.py | 164 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 252 insertions(+), 65 deletions(-) create mode 100644 test/test_caption.py diff --git a/pudb/debugger.py b/pudb/debugger.py index befa05a9..67fe31e0 100644 --- a/pudb/debugger.py +++ b/pudb/debugger.py @@ -529,7 +529,8 @@ def _runmodule(self, module_name): # UI stuff -------------------------------------------------------------------- from pudb.ui_tools import make_hotkey_markup, labelled_value, \ - SelectableText, SignalWrap, StackFrame, BreakpointFrame + SelectableText, SignalWrap, StackFrame, BreakpointFrame, \ + Caption, CaptionParts from pudb.var_view import FrameVarInfoKeeper @@ -858,8 +859,7 @@ def helpside(w, size, key): ], dividechars=1) - from pudb.ui_tools import Caption - self.caption = Caption("") + self.caption = Caption(CaptionParts(*[(None, "")]*4)) header = urwid.AttrMap(self.caption, "header") self.top = SignalWrap(urwid.Frame( urwid.AttrMap(self.columns, "background"), @@ -2618,35 +2618,26 @@ def interaction(self, exc_tuple, show_exc_dialog=True): self.current_exc_tuple = exc_tuple from pudb import VERSION - separator = " - " - pudb_version = "PuDB %s" % VERSION - hotkey = "?:help" + pudb_version = (None, "PuDB %s" % VERSION) + hotkey = (None, "?:help") if self.source_code_provider.get_source_identifier(): - source_filename = self.source_code_provider.get_source_identifier() + filename = (None, self.source_code_provider.get_source_identifier()) else: - source_filename = "source filename is unavailable" - caption = [(None, pudb_version), - (None, separator), - (None, hotkey), - (None, separator), - (None, source_filename) - ] + filename = (None, "source filename is unavailable") + optional_alert = (None, "") if self.debugger.post_mortem: if show_exc_dialog and exc_tuple is not None: self.show_exception_dialog(exc_tuple) - caption.extend([ - (None, separator), - ("warning", "[POST-MORTEM MODE]") - ]) + optional_alert = ("warning", "[POST-MORTEM MODE]") + elif exc_tuple is not None: - caption.extend([ - (None, separator), - ("warning", "[PROCESSING EXCEPTION, hit 'e' to examine]") - ]) + optional_alert = \ + ("warning", "[PROCESSING EXCEPTION, hit 'e' to examine]") - self.caption.set_text(caption) + self.caption.set_text(CaptionParts( + pudb_version, hotkey, filename, optional_alert)) self.event_loop() def set_source_code_provider(self, source_code_provider, force_update=False): diff --git a/pudb/ui_tools.py b/pudb/ui_tools.py index 479fc50b..6094c9bc 100644 --- a/pudb/ui_tools.py +++ b/pudb/ui_tools.py @@ -333,60 +333,92 @@ def keypress(self, size, key): return result +from collections import namedtuple +caption_parts = ["pudb_version", "hotkey", "full_source_filename", "optional_alert"] +CaptionParts = namedtuple( + "CaptionParts", + caption_parts, + ) + + class Caption(urwid.Text): - def __init__(self, markup, separator=" - "): + """ + A text widget that will automatically shorten its content + to fit in 1 row if needed + """ + + def __init__(self, caption_parts, separator=(None, " - ")): self.separator = separator - super().__init__(markup) - - def set_text(self, markup): - super().set_text(markup) - if len(markup) > 0: - # Assume the format of caption is: - # [optional_alert] - caption, _ = self.get_text() - caption_elements = caption.split(self.separator) - self.pudb_version = caption_elements[0] - self.hotkey = caption_elements[1] - self.full_source_filename = caption_elements[2] - self.optional_alert = caption_elements[3] if len( - caption_elements) > 3 else "" - else: - self.pudb_version = self.hotkey = "" - self.full_source_filename = self.optional_alert = "" + super().__init__(caption_parts) + + def __str__(self): + caption_text = self.separator[1].join( + [part[1] for part in self.caption_parts]).rstrip(self.separator[1]) + return caption_text + + @property + def markup(self): + """ + Returns markup of str(self) by inserting the markup of + self.separator between each item in self.caption_parts + """ + + # Reference: https://stackoverflow.com/questions/5920643/add-an-item-between-each-item-already-in-the-list # noqa + markup = [self.separator] * (len(self.caption_parts) * 2 - 1) + markup[0::2] = self.caption_parts + if not self.caption_parts.optional_alert[1]: + markup = markup[:-2] + return markup + + def render(self, size, focus=False): + markup = self._get_fit_width_markup(size) + return urwid.Text(markup).render(size) + + def set_text(self, caption_parts): + super().set_text([*caption_parts]) + self.caption_parts = caption_parts def rows(self, size, focus=False): # Always return 1 to avoid - # `assert head.rows() == hrows, "rows, render mismatch")` + # AssertionError: `assert head.rows() == hrows, "rows, render mismatch")` # in urwid.Frame.render() in urwid/container.py return 1 - def render(self, size, focus=False): + def _get_fit_width_markup(self, size): + if urwid.Text(str(self)).rows(size) == 1: + return self.markup + filename_markup_index = 4 maxcol = size[0] - if super().rows(size) > 1: - filename = self.get_shortened_source_filename(size) - else: - filename = self.full_source_filename - caption = self.separator.join( - [self.pudb_version, self.hotkey, filename, self.optional_alert] - ).strip(self.separator) - if self.optional_alert: - attr = [("warning", len(caption))] - else: - attr = [(None, 0)] - - return make_canvas([caption], [attr], maxcol) + markup = self.markup + markup[filename_markup_index] = ( + markup[filename_markup_index][0], + self._get_shortened_source_filename(size)) + caption = urwid.Text(markup) + while True: + if caption.rows(size) == 1: + return markup + else: + for i in range(len(markup)): + clip_amount = len(caption.get_text()[0]) - maxcol + markup[i] = (markup[i][0], markup[i][1][clip_amount:]) + caption = urwid.Text(markup) - def get_shortened_source_filename(self, size): + def _get_shortened_source_filename(self, size): import os maxcol = size[0] - occupied_width = (len(self.pudb_version) + len(self.hotkey) - + len(self.optional_alert) + len(self.separator)*3) - available_width = maxcol - occupied_width - trim_index = len(self.full_source_filename) - available_width - filename = self.full_source_filename[trim_index:] - first_dirname_index = filename.find(os.sep) - filename = filename[first_dirname_index + 1:] + occupied_width = len(str(self)) - \ + len(self.caption_parts.full_source_filename[1]) + available_width = max(0, maxcol - occupied_width) + trim_index = len( + self.caption_parts.full_source_filename[1]) - available_width + filename = self.caption_parts.full_source_filename[1][trim_index:] - return filename + if self.caption_parts.full_source_filename[1][trim_index-1] == os.sep: + #filename starts with the full name of a directory or file + return filename + else: + first_path_sep_index = filename.find(os.sep) + filename = filename[first_path_sep_index + 1:] + return filename # }}} diff --git a/test/test_caption.py b/test/test_caption.py new file mode 100644 index 00000000..ab5d14c6 --- /dev/null +++ b/test/test_caption.py @@ -0,0 +1,164 @@ +from pudb.ui_tools import Caption, CaptionParts +import pytest +import urwid + + +@pytest.fixture +def text_markups(): + from collections import namedtuple + Markups = namedtuple("Markups", + ["pudb_version", "hotkey", "full_source_filename", + "alert", "default_separator", "custom_separator"]) + + pudb_version = (None, "PuDB VERSION") + hotkey = (None, "?:help") + full_source_filename = (None, "/home/foo - bar/baz.py") + alert = ("warning", "[POST-MORTEM MODE]") + default_separator = (None, " - ") + custom_separator = (None, " | ") + return Markups(pudb_version, hotkey, full_source_filename, + alert, default_separator, custom_separator) + + +@pytest.fixture +def captions(text_markups): + empty = CaptionParts(*[(None, "")]*4) + always_display = [ + text_markups.pudb_version, text_markups.hotkey, + text_markups.full_source_filename] + return {"empty": Caption(empty), + "without_alert": Caption(CaptionParts(*always_display, (None, ""))), + "with_alert": Caption(CaptionParts(*always_display, text_markups.alert)), + "custom_separator": Caption(CaptionParts(*always_display, (None, "")), + separator=text_markups.custom_separator), + } + + +def test_init(captions): + for key in ["empty", "without_alert", "with_alert"]: + assert captions[key].separator == (None, " - ") + assert captions["custom_separator"].separator == (None, " | ") + + +def test_str(captions): + assert str(captions["empty"]) == "" + assert str(captions["without_alert"] + ) == "PuDB VERSION - ?:help - /home/foo - bar/baz.py" + assert str(captions["with_alert"] + ) == "PuDB VERSION - ?:help - /home/foo - bar/baz.py - [POST-MORTEM MODE]" # noqa + assert str(captions["custom_separator"] + ) == "PuDB VERSION | ?:help | /home/foo - bar/baz.py" + + +def test_markup(captions): + assert captions["empty"].markup \ + == [(None, ""), (None, " - "), + (None, ""), (None, " - "), + (None, "")] + + assert captions["without_alert"].markup \ + == [(None, "PuDB VERSION"), (None, " - "), + (None, "?:help"), (None, " - "), + (None, "/home/foo - bar/baz.py")] + + assert captions["with_alert"].markup \ + == [(None, "PuDB VERSION"), (None, " - "), + (None, "?:help"), (None, " - "), + (None, "/home/foo - bar/baz.py"), (None, " - "), + ("warning", "[POST-MORTEM MODE]")] + + assert captions["custom_separator"].markup \ + == [(None, "PuDB VERSION"), (None, " | "), + (None, "?:help"), (None, " | "), + (None, "/home/foo - bar/baz.py")] + + +def test_render(captions): + for k in captions.keys(): + sizes = {"wider_than_caption": (max(1, len(str(captions[k])) + 1), ), + "equals_caption": (max(1, len(str(captions[k]))), ), + "narrower_than_caption": (max(1, len(str(captions[k])) - 10), ), + } + for s in sizes: + got = captions[k].render(sizes[s]) + markup = captions[k]._get_fit_width_markup(sizes[s]) + expected = urwid.Text(markup).render(sizes[s]) + assert list(expected.content()) == list(got.content()) + + +def test_set_text(captions): + assert captions["empty"].caption_parts == CaptionParts(*[(None, "")]*4) + for key in ["without_alert", "custom_separator"]: + assert captions[key].caption_parts \ + == CaptionParts( + (None, "PuDB VERSION"), + (None, "?:help"), + (None, "/home/foo - bar/baz.py"), + (None, "")) + assert captions["with_alert"].caption_parts \ + == CaptionParts( + (None, "PuDB VERSION"), + (None, "?:help"), + (None, "/home/foo - bar/baz.py"), + ("warning", "[POST-MORTEM MODE]")) + + +def test_rows(captions): + for caption in captions.values(): + assert caption.rows(size=(99999, 99999)) == 1 + assert caption.rows(size=(80, 24)) == 1 + assert caption.rows(size=(1, 1)) == 1 + + +def test_get_fit_width_markup(captions): + # No need to check empty caption because + # len(str(caption)) == 0 always smaller than min terminal column == 1 + + # Set up + caption = captions["with_alert"] + caption_length = len(str(caption)) + full_source_filename = caption.caption_parts.full_source_filename[1] + cut_only_filename = ( + max(1, caption_length - len(full_source_filename) + 5), ) + cut_more_than_filename = (max(1, caption_length + - len(full_source_filename) - len("PuDB VE")), ) + sizes = {"cut_only_filename": cut_only_filename, + "cut_more_than_filename": cut_more_than_filename, + "one_col": (1, ), + } + # Test + assert caption._get_fit_width_markup(sizes["cut_only_filename"]) \ + == [(None, "PuDB VERSION"), (None, " - "), + (None, "?:help"), (None, " - "), + (None, "az.py"), (None, " - "), ("warning", "[POST-MORTEM MODE]")] + assert caption._get_fit_width_markup(sizes["cut_more_than_filename"]) \ + == [(None, "RSION"), (None, " - "), + (None, "?:help"), (None, " - "), + (None, ""), (None, " - "), ("warning", "[POST-MORTEM MODE]")] + assert caption._get_fit_width_markup(sizes["one_col"]) \ + == [(None, "")]*6 + [("warning", "]")] + + +def test_get_shortened_source_filename(captions): + # No need to check empty caption because + # len(str(caption)) == 0 always smaller than min terminal column == 1 + for k in ["with_alert", "without_alert", "custom_separator"]: + caption_length = len(str(captions[k])) + sizes = {"cut_at_path_sep": (max(1, caption_length - 1), ), + "lose_some_dir": (max(1, caption_length - 2), ), + "lose_all_dir": (max(1, + caption_length - len("/home/foo - bar/")), ), + "lose_some_filename_chars": (max(1, + caption_length - len("/home/foo - bar/ba")), ), + "lose_all": (max(1, + caption_length - len("/home/foo - bar/baz.py")), ), + } + assert captions[k]._get_shortened_source_filename(sizes["cut_at_path_sep"]) \ + == "home/foo - bar/baz.py" + assert captions[k]._get_shortened_source_filename(sizes["lose_some_dir"]) \ + == "foo - bar/baz.py" + assert captions[k]._get_shortened_source_filename(sizes["lose_all_dir"]) \ + == "baz.py" + assert captions[k]._get_shortened_source_filename( + sizes["lose_some_filename_chars"]) == "z.py" + assert captions[k]._get_shortened_source_filename(sizes["lose_all"]) == "" From 797bb300c3d9181038505e4725de10b16689c208 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Quang Date: Fri, 23 Jul 2021 20:41:19 +0700 Subject: [PATCH 08/12] Add more tests and fix formatting --- test/test_caption.py | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/test/test_caption.py b/test/test_caption.py index ab5d14c6..6254c3e5 100644 --- a/test/test_caption.py +++ b/test/test_caption.py @@ -62,7 +62,7 @@ def test_markup(captions): (None, "/home/foo - bar/baz.py")] assert captions["with_alert"].markup \ - == [(None, "PuDB VERSION"), (None, " - "), + == [(None, "PuDB VERSION"), (None, " - "), (None, "?:help"), (None, " - "), (None, "/home/foo - bar/baz.py"), (None, " - "), ("warning", "[POST-MORTEM MODE]")] @@ -122,21 +122,27 @@ def test_get_fit_width_markup(captions): max(1, caption_length - len(full_source_filename) + 5), ) cut_more_than_filename = (max(1, caption_length - len(full_source_filename) - len("PuDB VE")), ) - sizes = {"cut_only_filename": cut_only_filename, + sizes = {"equals_caption": (max(1, caption_length), ), + "cut_only_filename": cut_only_filename, "cut_more_than_filename": cut_more_than_filename, "one_col": (1, ), } # Test + assert caption._get_fit_width_markup(sizes["equals_caption"]) \ + == [(None, "PuDB VERSION"), (None, " - "), + (None, "?:help"), (None, " - "), + (None, "/home/foo - bar/baz.py"), (None, " - "), + ("warning", "[POST-MORTEM MODE]")] assert caption._get_fit_width_markup(sizes["cut_only_filename"]) \ - == [(None, "PuDB VERSION"), (None, " - "), - (None, "?:help"), (None, " - "), - (None, "az.py"), (None, " - "), ("warning", "[POST-MORTEM MODE]")] + == [(None, "PuDB VERSION"), (None, " - "), + (None, "?:help"), (None, " - "), + (None, "az.py"), (None, " - "), ("warning", "[POST-MORTEM MODE]")] assert caption._get_fit_width_markup(sizes["cut_more_than_filename"]) \ - == [(None, "RSION"), (None, " - "), - (None, "?:help"), (None, " - "), - (None, ""), (None, " - "), ("warning", "[POST-MORTEM MODE]")] + == [(None, "RSION"), (None, " - "), + (None, "?:help"), (None, " - "), + (None, ""), (None, " - "), ("warning", "[POST-MORTEM MODE]")] assert caption._get_fit_width_markup(sizes["one_col"]) \ - == [(None, "")]*6 + [("warning", "]")] + == [(None, "")]*6 + [("warning", "]")] def test_get_shortened_source_filename(captions): @@ -160,5 +166,7 @@ def test_get_shortened_source_filename(captions): assert captions[k]._get_shortened_source_filename(sizes["lose_all_dir"]) \ == "baz.py" assert captions[k]._get_shortened_source_filename( - sizes["lose_some_filename_chars"]) == "z.py" - assert captions[k]._get_shortened_source_filename(sizes["lose_all"]) == "" + sizes["lose_some_filename_chars"]) \ + == "z.py" + assert captions[k]._get_shortened_source_filename(sizes["lose_all"]) \ + == "" From 418a4d9d492b83b8f47355c249ad53657733a113 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Quang Date: Sat, 24 Jul 2021 15:59:26 +0700 Subject: [PATCH 09/12] Put terminal sizes generating code into 1 fixture --- test/test_caption.py | 75 ++++++++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 37 deletions(-) diff --git a/test/test_caption.py b/test/test_caption.py index 6254c3e5..0e4460f0 100644 --- a/test/test_caption.py +++ b/test/test_caption.py @@ -34,6 +34,33 @@ def captions(text_markups): } +@pytest.fixture +def term_sizes(): + def _term_sizes(caption): + caption_length = len(str(caption)) + full_source_filename = caption.caption_parts.full_source_filename[1] + cut_only_filename = ( + max(1, caption_length - len(full_source_filename) + 5), ) + cut_more_than_filename = (max(1, caption_length + - len(full_source_filename) - len("PuDB VE")), ) + return {"wider_than_caption": (caption_length + 1, ), + "equals_caption": (max(1, caption_length), ), + "narrower_than_caption": (max(1, caption_length - 10), ), + "cut_only_filename": cut_only_filename, + "cut_more_than_filename": cut_more_than_filename, + "one_col": (1, ), + "cut_at_path_sep": (max(1, caption_length - 1), ), + "lose_some_dir": (max(1, caption_length - 2), ), + "lose_all_dir": (max(1, + caption_length - len("/home/foo - bar/")), ), + "lose_some_filename_chars": (max(1, + caption_length - len("/home/foo - bar/ba")), ), + "lose_all_source": (max(1, + caption_length - len("/home/foo - bar/baz.py")), ), + } + return _term_sizes + + def test_init(captions): for key in ["empty", "without_alert", "with_alert"]: assert captions[key].separator == (None, " - ") @@ -73,16 +100,12 @@ def test_markup(captions): (None, "/home/foo - bar/baz.py")] -def test_render(captions): - for k in captions.keys(): - sizes = {"wider_than_caption": (max(1, len(str(captions[k])) + 1), ), - "equals_caption": (max(1, len(str(captions[k]))), ), - "narrower_than_caption": (max(1, len(str(captions[k])) - 10), ), - } - for s in sizes: - got = captions[k].render(sizes[s]) - markup = captions[k]._get_fit_width_markup(sizes[s]) - expected = urwid.Text(markup).render(sizes[s]) +def test_render(captions, term_sizes): + for caption in captions.values(): + for size in term_sizes(caption).values(): + got = caption.render(size) + markup = caption._get_fit_width_markup(size) + expected = urwid.Text(markup).render(size) assert list(expected.content()) == list(got.content()) @@ -110,24 +133,11 @@ def test_rows(captions): assert caption.rows(size=(1, 1)) == 1 -def test_get_fit_width_markup(captions): +def test_get_fit_width_markup(captions, term_sizes): # No need to check empty caption because # len(str(caption)) == 0 always smaller than min terminal column == 1 - - # Set up caption = captions["with_alert"] - caption_length = len(str(caption)) - full_source_filename = caption.caption_parts.full_source_filename[1] - cut_only_filename = ( - max(1, caption_length - len(full_source_filename) + 5), ) - cut_more_than_filename = (max(1, caption_length - - len(full_source_filename) - len("PuDB VE")), ) - sizes = {"equals_caption": (max(1, caption_length), ), - "cut_only_filename": cut_only_filename, - "cut_more_than_filename": cut_more_than_filename, - "one_col": (1, ), - } - # Test + sizes = term_sizes(caption) assert caption._get_fit_width_markup(sizes["equals_caption"]) \ == [(None, "PuDB VERSION"), (None, " - "), (None, "?:help"), (None, " - "), @@ -145,20 +155,11 @@ def test_get_fit_width_markup(captions): == [(None, "")]*6 + [("warning", "]")] -def test_get_shortened_source_filename(captions): +def test_get_shortened_source_filename(captions, term_sizes): # No need to check empty caption because # len(str(caption)) == 0 always smaller than min terminal column == 1 for k in ["with_alert", "without_alert", "custom_separator"]: - caption_length = len(str(captions[k])) - sizes = {"cut_at_path_sep": (max(1, caption_length - 1), ), - "lose_some_dir": (max(1, caption_length - 2), ), - "lose_all_dir": (max(1, - caption_length - len("/home/foo - bar/")), ), - "lose_some_filename_chars": (max(1, - caption_length - len("/home/foo - bar/ba")), ), - "lose_all": (max(1, - caption_length - len("/home/foo - bar/baz.py")), ), - } + sizes = term_sizes(captions[k]) assert captions[k]._get_shortened_source_filename(sizes["cut_at_path_sep"]) \ == "home/foo - bar/baz.py" assert captions[k]._get_shortened_source_filename(sizes["lose_some_dir"]) \ @@ -168,5 +169,5 @@ def test_get_shortened_source_filename(captions): assert captions[k]._get_shortened_source_filename( sizes["lose_some_filename_chars"]) \ == "z.py" - assert captions[k]._get_shortened_source_filename(sizes["lose_all"]) \ + assert captions[k]._get_shortened_source_filename(sizes["lose_all_source"]) \ == "" From 9584a9f62859ac51abf69952f46e07cb9986fb86 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Quang Date: Sun, 25 Jul 2021 15:12:25 +0700 Subject: [PATCH 10/12] Make sure the content passed to urwid is of type str urwid will raise error if the content in the markup is not of type str, even when they are convertable. So we have to do the converting. --- pudb/ui_tools.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pudb/ui_tools.py b/pudb/ui_tools.py index 6094c9bc..15cbdff6 100644 --- a/pudb/ui_tools.py +++ b/pudb/ui_tools.py @@ -375,8 +375,9 @@ def render(self, size, focus=False): return urwid.Text(markup).render(size) def set_text(self, caption_parts): - super().set_text([*caption_parts]) - self.caption_parts = caption_parts + markup = [(attr, str(content)) for (attr, content) in caption_parts] + self.caption_parts = CaptionParts(*markup) + super().set_text(markup) def rows(self, size, focus=False): # Always return 1 to avoid From 47d20260d844b8de727e2d3a070c98585d82f35d Mon Sep 17 00:00:00 2001 From: Huy Nguyen Quang Date: Sun, 25 Jul 2021 15:19:27 +0700 Subject: [PATCH 11/12] Replace list destructuring with CaptionParts._make() Use CaptionParts._make() when constructing CaptionParts objects to increase readability. --- pudb/debugger.py | 2 +- pudb/ui_tools.py | 2 +- test/test_caption.py | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pudb/debugger.py b/pudb/debugger.py index 67fe31e0..662bc6d1 100644 --- a/pudb/debugger.py +++ b/pudb/debugger.py @@ -859,7 +859,7 @@ def helpside(w, size, key): ], dividechars=1) - self.caption = Caption(CaptionParts(*[(None, "")]*4)) + self.caption = Caption(CaptionParts._make([(None, "")]*4)) header = urwid.AttrMap(self.caption, "header") self.top = SignalWrap(urwid.Frame( urwid.AttrMap(self.columns, "background"), diff --git a/pudb/ui_tools.py b/pudb/ui_tools.py index 15cbdff6..9bf212e7 100644 --- a/pudb/ui_tools.py +++ b/pudb/ui_tools.py @@ -376,7 +376,7 @@ def render(self, size, focus=False): def set_text(self, caption_parts): markup = [(attr, str(content)) for (attr, content) in caption_parts] - self.caption_parts = CaptionParts(*markup) + self.caption_parts = CaptionParts._make(markup) super().set_text(markup) def rows(self, size, focus=False): diff --git a/test/test_caption.py b/test/test_caption.py index 0e4460f0..471c03aa 100644 --- a/test/test_caption.py +++ b/test/test_caption.py @@ -22,14 +22,14 @@ def text_markups(): @pytest.fixture def captions(text_markups): - empty = CaptionParts(*[(None, "")]*4) + empty = CaptionParts._make([(None, "")]*4) always_display = [ text_markups.pudb_version, text_markups.hotkey, text_markups.full_source_filename] return {"empty": Caption(empty), - "without_alert": Caption(CaptionParts(*always_display, (None, ""))), - "with_alert": Caption(CaptionParts(*always_display, text_markups.alert)), - "custom_separator": Caption(CaptionParts(*always_display, (None, "")), + "without_alert": Caption(CaptionParts._make(always_display + [(None, "")])), + "with_alert": Caption(CaptionParts._make(always_display + [text_markups.alert])), + "custom_separator": Caption(CaptionParts._make(always_display + [(None, "")]), separator=text_markups.custom_separator), } @@ -110,7 +110,7 @@ def test_render(captions, term_sizes): def test_set_text(captions): - assert captions["empty"].caption_parts == CaptionParts(*[(None, "")]*4) + assert captions["empty"].caption_parts == CaptionParts._make([(None, "")]*4) for key in ["without_alert", "custom_separator"]: assert captions[key].caption_parts \ == CaptionParts( From ca4c7f10655e13c02ba828bd8ada8bcbfea8e19a Mon Sep 17 00:00:00 2001 From: Huy Nguyen Quang Date: Sun, 25 Jul 2021 15:35:13 +0700 Subject: [PATCH 12/12] Remove redundant test and fix formatting --- test/test_caption.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/test/test_caption.py b/test/test_caption.py index 471c03aa..232da96e 100644 --- a/test/test_caption.py +++ b/test/test_caption.py @@ -27,9 +27,12 @@ def captions(text_markups): text_markups.pudb_version, text_markups.hotkey, text_markups.full_source_filename] return {"empty": Caption(empty), - "without_alert": Caption(CaptionParts._make(always_display + [(None, "")])), - "with_alert": Caption(CaptionParts._make(always_display + [text_markups.alert])), - "custom_separator": Caption(CaptionParts._make(always_display + [(None, "")]), + "without_alert": Caption(CaptionParts._make( + always_display + [(None, "")])), + "with_alert": Caption(CaptionParts._make( + always_display + [text_markups.alert])), + "custom_separator": Caption(CaptionParts._make( + always_display + [(None, "")]), separator=text_markups.custom_separator), } @@ -69,12 +72,12 @@ def test_init(captions): def test_str(captions): assert str(captions["empty"]) == "" - assert str(captions["without_alert"] - ) == "PuDB VERSION - ?:help - /home/foo - bar/baz.py" - assert str(captions["with_alert"] - ) == "PuDB VERSION - ?:help - /home/foo - bar/baz.py - [POST-MORTEM MODE]" # noqa - assert str(captions["custom_separator"] - ) == "PuDB VERSION | ?:help | /home/foo - bar/baz.py" + assert str(captions["without_alert"]) \ + == "PuDB VERSION - ?:help - /home/foo - bar/baz.py" + assert str(captions["with_alert"]) \ + == "PuDB VERSION - ?:help - /home/foo - bar/baz.py - [POST-MORTEM MODE]" + assert str(captions["custom_separator"]) \ + == "PuDB VERSION | ?:help | /home/foo - bar/baz.py" def test_markup(captions): @@ -111,8 +114,7 @@ def test_render(captions, term_sizes): def test_set_text(captions): assert captions["empty"].caption_parts == CaptionParts._make([(None, "")]*4) - for key in ["without_alert", "custom_separator"]: - assert captions[key].caption_parts \ + assert captions["without_alert"].caption_parts \ == CaptionParts( (None, "PuDB VERSION"), (None, "?:help"),