From 4d15badcccf029b9c400df56d4aaf865f7409456 Mon Sep 17 00:00:00 2001 From: toonn Date: Thu, 26 May 2022 17:15:51 +0200 Subject: browsercolumn: Fix off-by-one in line number width The status bar wasn't taken into account when calculating the width of the visible line numbers. This meant the width would change right before a number with an extra digit appeared or one later than when it disappeared. --- ranger/gui/widgets/browsercolumn.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ranger/gui/widgets/browsercolumn.py b/ranger/gui/widgets/browsercolumn.py index 3d68f017..57165424 100644 --- a/ranger/gui/widgets/browsercolumn.py +++ b/ranger/gui/widgets/browsercolumn.py @@ -275,7 +275,7 @@ class BrowserColumn(Pager): # pylint: disable=too-many-instance-attributes # Set the size of the linum text field to the number of digits in the # visible files in directory. - linum_text_len = len(str(self.scroll_begin + self.hei)) + linum_text_len = len(str(self.scroll_begin + self.hei - 1)) linum_format = "{0:>" + str(linum_text_len) + "}" selected_i = self._get_index_of_selected_file() -- cgit 1.4.1-2-gfad0 From 5db21cc02e10027bfc1148f85664c2c44b6b631d Mon Sep 17 00:00:00 2001 From: toonn Date: Thu, 26 May 2022 20:07:47 +0200 Subject: browsercolumn: Accept line_numbers case insensitively --- ranger/gui/widgets/browsercolumn.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/ranger/gui/widgets/browsercolumn.py b/ranger/gui/widgets/browsercolumn.py index 57165424..14f68589 100644 --- a/ranger/gui/widgets/browsercolumn.py +++ b/ranger/gui/widgets/browsercolumn.py @@ -212,7 +212,7 @@ class BrowserColumn(Pager): # pylint: disable=too-many-instance-attributes def _format_line_number(self, linum_format, i, selected_i): line_number = i - if self.settings.line_numbers == 'relative': + if self.settings.line_numbers.lower() == 'relative': line_number = abs(selected_i - i) if not self.settings.relative_current_zero and line_number == 0: if self.settings.one_indexed: @@ -312,7 +312,10 @@ class BrowserColumn(Pager): # pylint: disable=too-many-instance-attributes # Check if current line has not already computed and cached if key in drawn.display_data: # Recompute line numbers because they can't be reliably cached. - if self.main_column and self.settings.line_numbers != 'false': + if ( + self.main_column + and self.settings.line_numbers.lower() != 'false' + ): line_number_text = self._format_line_number(linum_format, i, selected_i) @@ -337,7 +340,7 @@ class BrowserColumn(Pager): # pylint: disable=too-many-instance-attributes space = self.wid # line number field - if self.settings.line_numbers != 'false': + if self.settings.line_numbers.lower() != 'false': if self.main_column and space - linum_text_len > 2: line_number_text = self._format_line_number(linum_format, i, -- cgit 1.4.1-2-gfad0 From c9cb40d86bcfb7cff6453d99ba0ed70356203b5c Mon Sep 17 00:00:00 2001 From: toonn Date: Thu, 26 May 2022 20:10:19 +0200 Subject: browsercolumn: Remove off-by-one trailing space The file size was always followed by a space in case it needed to be separated from other trailing information but this should only happen if anything actually follows it. --- ranger/gui/widgets/browsercolumn.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ranger/gui/widgets/browsercolumn.py b/ranger/gui/widgets/browsercolumn.py index 14f68589..28e91922 100644 --- a/ranger/gui/widgets/browsercolumn.py +++ b/ranger/gui/widgets/browsercolumn.py @@ -374,15 +374,16 @@ class BrowserColumn(Pager): # pylint: disable=too-many-instance-attributes try: infostringdata = current_linemode.infostring(drawn, metadata) if infostringdata: - infostring.append([" " + infostringdata + " ", + infostring.append([" " + infostringdata, ["infostring"]]) except NotImplementedError: infostring = self._draw_infostring_display(drawn, space) if infostring: infostringlen = self._total_len(infostring) if space - infostringlen > 2: - predisplay_right = infostring + predisplay_right - space -= infostringlen + sep = [" ", []] if predisplay_right else [] + predisplay_right = infostring + sep + predisplay_right + space -= infostringlen + len(sep) textstring = self._draw_text_display(text, space) textstringlen = self._total_len(textstring) @@ -448,7 +449,7 @@ class BrowserColumn(Pager): # pylint: disable=too-many-instance-attributes infostring_display = [] if self.display_infostring and drawn.infostring \ and self.settings.display_size_in_main_column: - infostring = str(drawn.infostring) + " " + infostring = str(drawn.infostring) if len(infostring) <= space: infostring_display.append([infostring, ['infostring']]) return infostring_display -- cgit 1.4.1-2-gfad0 From 9b47c41a3717ebcb7deeccd0ba947eb50069634e Mon Sep 17 00:00:00 2001 From: toonn Date: Thu, 26 May 2022 20:11:46 +0200 Subject: browsercolumn: Include linum_text_len in key The width of an item needs to be recalculated if the width of the line numbers changes. This happens every time we get to another order of magnitude (an additional digit). This is based on [review comments](https://github.com/ranger/ranger/pull/2346#issuecomment-1062257526) from markus-bauer. The solution opted for here is basically their second bullet point. I tried the third bullet point first, replacing `line_numbers` by `linum_text_len` in the key but this requires recalculating `linum_text_len` to be zero when line numbers are disabled, which feels like a somewhat clunky overloading while we have a perfectly usable boolean flag available. Closes #2346 Co-authored-by: markus-bauer --- ranger/gui/widgets/browsercolumn.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ranger/gui/widgets/browsercolumn.py b/ranger/gui/widgets/browsercolumn.py index 28e91922..59f2622d 100644 --- a/ranger/gui/widgets/browsercolumn.py +++ b/ranger/gui/widgets/browsercolumn.py @@ -307,7 +307,7 @@ class BrowserColumn(Pager): # pylint: disable=too-many-instance-attributes drawn.path in copied, tagged_marker, drawn.infostring, drawn.vcsstatus, drawn.vcsremotestatus, self.target.has_vcschild, self.fm.do_cut, current_linemode.name, metakey, active_pane, - self.settings.line_numbers) + self.settings.line_numbers, linum_text_len) # Check if current line has not already computed and cached if key in drawn.display_data: -- cgit 1.4.1-2-gfad0 From 3461f24de6d816aa4eab5de4ed6a019f80a61b41 Mon Sep 17 00:00:00 2001 From: toonn Date: Thu, 26 May 2022 20:20:22 +0200 Subject: browsercolumn: Fix relative line number width For relative line numbers the width that actually matters is the width for all the displayed line numbers. The width of the largest index of the displayed files doesn't matter. Only the width of the relative indices and the width of the currently highlighted item's index, unless `relative_current_zero` is enabled. --- ranger/gui/widgets/browsercolumn.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ranger/gui/widgets/browsercolumn.py b/ranger/gui/widgets/browsercolumn.py index 59f2622d..c65bf018 100644 --- a/ranger/gui/widgets/browsercolumn.py +++ b/ranger/gui/widgets/browsercolumn.py @@ -273,12 +273,20 @@ class BrowserColumn(Pager): # pylint: disable=too-many-instance-attributes copied = [f.path for f in self.fm.copy_buffer] + selected_i = self._get_index_of_selected_file() + # Set the size of the linum text field to the number of digits in the # visible files in directory. - linum_text_len = len(str(self.scroll_begin + self.hei - 1)) + scroll_end = self.scroll_begin + self.hei - 1 + if self.settings.line_numbers.lower() == "relative": + linum_text_len = len(str(max(selected_i - self.scroll_begin, + scroll_end - selected_i))) + if not self.settings.relative_current_zero: + linum_text_len = max(len(str(selected_i)), linum_text_len) + else: + linum_text_len = len(str(scroll_end)) linum_format = "{0:>" + str(linum_text_len) + "}" - selected_i = self._get_index_of_selected_file() for line in range(self.hei): i = line + self.scroll_begin -- cgit 1.4.1-2-gfad0 From 14ec178db8291cf14b5cff933f82e9de75092e90 Mon Sep 17 00:00:00 2001 From: toonn Date: Fri, 27 May 2022 00:02:48 +0200 Subject: browsercolumn: Refactor linum_text_len The calculation isn't difficult but the lines were long and there's several branches involved. Giving `len(str())` a name and introducing names for intermediate results should make it easier to comprehend --- ranger/gui/widgets/browsercolumn.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/ranger/gui/widgets/browsercolumn.py b/ranger/gui/widgets/browsercolumn.py index c65bf018..425edf7f 100644 --- a/ranger/gui/widgets/browsercolumn.py +++ b/ranger/gui/widgets/browsercolumn.py @@ -277,14 +277,20 @@ class BrowserColumn(Pager): # pylint: disable=too-many-instance-attributes # Set the size of the linum text field to the number of digits in the # visible files in directory. + def nr_of_digits(number): + return len(str(number)) + scroll_end = self.scroll_begin + self.hei - 1 + distance_to_top = selected_i - self.scroll_begin + distance_to_bottom = scroll_end - selected_i + if self.settings.line_numbers.lower() == "relative": - linum_text_len = len(str(max(selected_i - self.scroll_begin, - scroll_end - selected_i))) + linum_text_len = nr_of_digits(max(distance_to_top, + distance_to_bottom)) if not self.settings.relative_current_zero: - linum_text_len = max(len(str(selected_i)), linum_text_len) + linum_text_len = max(nr_of_digits(selected_i), linum_text_len) else: - linum_text_len = len(str(scroll_end)) + linum_text_len = nr_of_digits(scroll_end) linum_format = "{0:>" + str(linum_text_len) + "}" for line in range(self.hei): -- cgit 1.4.1-2-gfad0 From 2fbf622751e7abd97ba4843623eb549dfa789c31 Mon Sep 17 00:00:00 2001 From: toonn Date: Fri, 27 May 2022 00:05:37 +0200 Subject: browsercolumn: Lowercase line_numbers in key The `line_numbers` setting is a string but case shouldn't matter, especially for caching, this is handled in branches already but the caching key was overlooked. --- ranger/gui/widgets/browsercolumn.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ranger/gui/widgets/browsercolumn.py b/ranger/gui/widgets/browsercolumn.py index 425edf7f..4867d184 100644 --- a/ranger/gui/widgets/browsercolumn.py +++ b/ranger/gui/widgets/browsercolumn.py @@ -321,7 +321,7 @@ class BrowserColumn(Pager): # pylint: disable=too-many-instance-attributes drawn.path in copied, tagged_marker, drawn.infostring, drawn.vcsstatus, drawn.vcsremotestatus, self.target.has_vcschild, self.fm.do_cut, current_linemode.name, metakey, active_pane, - self.settings.line_numbers, linum_text_len) + self.settings.line_numbers.lower(), linum_text_len) # Check if current line has not already computed and cached if key in drawn.display_data: -- cgit 1.4.1-2-gfad0 From 72dab6555185ce92e3fb50e7f857f981cc1a7f83 Mon Sep 17 00:00:00 2001 From: toonn Date: Fri, 27 May 2022 13:57:58 +0200 Subject: browsercolumn: Take into account one_indexed setting Line number width calculation was ignoring the one_indexed setting. This meant recalculating the width one line too late. --- ranger/gui/widgets/browsercolumn.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ranger/gui/widgets/browsercolumn.py b/ranger/gui/widgets/browsercolumn.py index 4867d184..305b8284 100644 --- a/ranger/gui/widgets/browsercolumn.py +++ b/ranger/gui/widgets/browsercolumn.py @@ -283,14 +283,17 @@ class BrowserColumn(Pager): # pylint: disable=too-many-instance-attributes scroll_end = self.scroll_begin + self.hei - 1 distance_to_top = selected_i - self.scroll_begin distance_to_bottom = scroll_end - selected_i + one_indexed_offset = 1 if self.settings.one_indexed else 0 if self.settings.line_numbers.lower() == "relative": linum_text_len = nr_of_digits(max(distance_to_top, distance_to_bottom)) if not self.settings.relative_current_zero: - linum_text_len = max(nr_of_digits(selected_i), linum_text_len) + linum_text_len = max(nr_of_digits(selected_i + + one_indexed_offset), + linum_text_len) else: - linum_text_len = nr_of_digits(scroll_end) + linum_text_len = nr_of_digits(scroll_end + one_indexed_offset) linum_format = "{0:>" + str(linum_text_len) + "}" for line in range(self.hei): -- cgit 1.4.1-2-gfad0 From 6cef19e07fe3dfe905d81fa134dcf5b4089f6780 Mon Sep 17 00:00:00 2001 From: toonn Date: Fri, 27 May 2022 14:07:21 +0200 Subject: browsercolumn: Use number of items for width The width of the widest line number, which is often the last one visible in the list is not always the same thing as the width of the item that would be at the very bottom of the screen. When a directory contains fewer items than lines are available for display, the former can have fewer digits. --- ranger/gui/widgets/browsercolumn.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ranger/gui/widgets/browsercolumn.py b/ranger/gui/widgets/browsercolumn.py index 305b8284..0a3dc928 100644 --- a/ranger/gui/widgets/browsercolumn.py +++ b/ranger/gui/widgets/browsercolumn.py @@ -280,7 +280,7 @@ class BrowserColumn(Pager): # pylint: disable=too-many-instance-attributes def nr_of_digits(number): return len(str(number)) - scroll_end = self.scroll_begin + self.hei - 1 + scroll_end = self.scroll_begin + min(self.hei, len(self.target)) - 1 distance_to_top = selected_i - self.scroll_begin distance_to_bottom = scroll_end - selected_i one_indexed_offset = 1 if self.settings.one_indexed else 0 -- cgit 1.4.1-2-gfad0