Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Refactoring: remove dead code from Raw display #707

Merged
merged 2 commits into from Dec 14, 2023
Merged

Refactoring: remove dead code from Raw display #707

merged 2 commits into from Dec 14, 2023

Conversation

penguinolog
Copy link
Collaborator

  • CursesDisplay: stop using private namespace _curses for exceptions
  • Mass add typing for better static logic trace
Checklist
  • I've ensured that similar functionality has not already been implemented
  • I've ensured that similar functionality has not earlier been proposed and declined
  • I've branched off the master or python-dual-support branch
  • I've merged fresh upstream into my branch recently
  • I've ran tox successfully in local environment
  • I've included docstrings and/or documentation and/or examples for my code (if this is a new feature)

penguinolog
penguinolog commented Dec 14, 2023
View reviewed changes
urwid/_posix_raw_display.py
@@ -212,90 +213,6 @@ def _stop(self):

super()._stop()

@typing.overload
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

duplicate from base class

penguinolog
penguinolog commented Dec 14, 2023
View reviewed changes
urwid/_posix_raw_display.py
b |= mod
result.extend([27, ord("["), ord("M"), b + 32, x + 32, y + 32])

def determine_button_release(flag: int) -> None:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not used and unreachable from outside

penguinolog
penguinolog commented Dec 14, 2023
View reviewed changes
urwid/_raw_display_base.py
signals.connect_signal(self, UPDATE_PALETTE_ENTRY, self._on_update_palette_entry)
self.colors: Literal[1, 16, 88, 256, 16777216] = 16 # FIXME: detect this
self.has_underline = True # FIXME: detect this
self._keyqueue = []
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not used across whole project

penguinolog
penguinolog commented Dec 14, 2023
View reviewed changes
urwid/_raw_display_base.py
if not partial_display():
return escape.set_cursor_position(0, 0)
return escape.CURSOR_HOME_COL + escape.move_cursor_up(cy)

def set_cursor_row(y):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not used and unreachable from outside

penguinolog
penguinolog commented Dec 14, 2023
View reviewed changes
urwid/display_common.py
@@ -1153,7 +1153,7 @@ def register_palette_entry(
mono = ",".join(mono)
if mono is None:
mono = DEFAULT
mono = AttrSpec(mono, DEFAULT, 1)
mono_spec = AttrSpec(mono, DEFAULT, 1)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

do not replace variable and not confuse static analysis

@coveralls
Copy link

coveralls commented Dec 14, 2023 •
edited

Pull Request Test Coverage Report for Build 7209892230

  • 57 of 86 (66.28%) changed or added relevant lines in 7 files are covered.
  • 3 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.4%) to 70.954%
Changes Missing Coverage Covered Lines Changed/Added Lines %
urwid/_posix_raw_display.py 9 10 90.0%
urwid/curses_display.py 1 8 12.5%
urwid/display_common.py 6 13 46.15%
urwid/_raw_display_base.py 16 30 53.33%
Files with Coverage Reduction New Missed Lines %
urwid/_posix_raw_display.py 1 12.29%
urwid/_raw_display_base.py 1 7.35%
urwid/util.py 1 89.75%
Totals Coverage Status
Change from base Build 7205354780: 0.4%
Covered Lines: 7606
Relevant Lines: 10810
💛 - Coveralls

penguinolog
penguinolog commented Dec 14, 2023
View reviewed changes
urwid/curses_display.py
@@ -108,7 +108,6 @@ def __init__(self):
self.has_color = False
self.s = None
self.cursor_state = None
self._keyqueue = []
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not used across whole project

penguinolog
penguinolog commented Dec 14, 2023
View reviewed changes
urwid/old_str_util.py
@@ -29,7 +29,7 @@
SAFE_ASCII_RE = re.compile("^[ -~]*$")
SAFE_ASCII_BYTES_RE = re.compile(b"^[ -~]*$")

_byte_encoding = None
_byte_encoding: Literal["utf8", "narrow", "wide"] = "narrow"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

use correct default value with minimal functionality

penguinolog
penguinolog commented Dec 14, 2023
View reviewed changes
urwid/display_common.py
pass

@abc.abstractmethod
def draw_screen(self, size: tuple[int, int], r: Canvas) -> None:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

method is ultimately used by MainLoop - require to implement

@penguinolog
Refactoring: remove dead code from Raw display
99aca4b 
* CursesDisplay: stop using private namespace `_curses` for exceptions
* Mass add typing for better static logic trace
wardi
wardi reviewed Dec 14, 2023
View reviewed changes
urwid/display_common.py Outdated

Default (fallback) is 80x24.
"""
return 24, 80
Copy link
Collaborator

Choose a reason for hiding this comment

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

docstring says columns, rows but return value suggests rows, columns

Copy link
Collaborator

Choose a reason for hiding this comment

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

the original code removed above returned 80, 24

@penguinolog penguinolog merged commit 08946eb into urwid:master Dec 14, 2023
35 checks passed
@penguinolog penguinolog deleted the ref_dead_code branch December 14, 2023 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wardi wardi wardi approved these changes

Assignees
No one assigned
Labels
refactoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@penguinolog @coveralls @wardi