Skip to content

Commit

Permalink
Replace inheritance with composition
Browse files Browse the repository at this point in the history
# DISCLAIMER
Super duper WIP changes ahead.

---

I just want to see what problems arise when using composition over inheritance, as
suggested by @The-Compiler in
qutebrowser#7407 (comment).

There is a lot of unaddressed ugliness (e.g. `widget.widget` everywhere) in this commit.
I just started with moving `QLabel` into `TextBase.widget`. Then fixing failing tests
one by one with the *easiest* solution (e.g. `widget.widget`); I even marked one test
with `xfail` because I could not quickly figure out why it was failing.

My hope is to get a better feel for what belongs to `QWidget` and what belongs to
`TextBase` (or its subclasses).
  • Loading branch information
pylbrecht committed Jun 22, 2024
1 parent 91be21a commit c761737
Show file tree
Hide file tree
Showing 17 changed files with 263 additions and 201 deletions.
18 changes: 9 additions & 9 deletions qutebrowser/mainwindow/mainwindow.py
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ def _connect_signals(self):

# commands
mode_manager.keystring_updated.connect(
self.status.keystring.on_keystring_updated)
self.status.keystring.widget.on_keystring_updated)
self.status.cmd.got_cmd[str].connect(self._commandrunner.run_safely)
self.status.cmd.got_cmd[str, int].connect(self._commandrunner.run_safely)
self.status.cmd.returnPressed.connect(self.tabbed_browser.on_cmd_return_pressed)
Expand All @@ -518,27 +518,27 @@ def _connect_signals(self):
self.status.on_tab_changed)

self.tabbed_browser.cur_progress.connect(
self.status.prog.on_load_progress)
self.status.prog.widget.on_load_progress)
self.tabbed_browser.cur_load_started.connect(
self.status.prog.on_load_started)
self.status.prog.widget.on_load_started)

self.tabbed_browser.cur_scroll_perc_changed.connect(
self.status.percentage.set_perc)
self.status.percentage.widget.set_perc)
self.tabbed_browser.widget.tab_index_changed.connect(
self.status.tabindex.on_tab_index_changed)
self.status.tabindex.widget.on_tab_index_changed)

self.tabbed_browser.cur_url_changed.connect(
self.status.url.set_url)
self.status.url.widget.set_url)
self.tabbed_browser.cur_url_changed.connect(functools.partial(
self.status.backforward.on_tab_cur_url_changed,
tabs=self.tabbed_browser))
self.tabbed_browser.cur_link_hovered.connect(
self.status.url.set_hover_url)
self.status.url.widget.set_hover_url)
self.tabbed_browser.cur_load_status_changed.connect(
self.status.url.on_load_status_changed)
self.status.url.widget.on_load_status_changed)

self.tabbed_browser.cur_search_match_changed.connect(
self.status.search_match.set_match)
self.status.search_match.widget.set_match)

self.tabbed_browser.cur_caret_selection_toggled.connect(
self.status.on_caret_selection_toggled)
Expand Down
4 changes: 2 additions & 2 deletions qutebrowser/mainwindow/statusbar/backforward.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def on_tab_cur_url_changed(self, tabs):
tab = tabs.widget.currentWidget()
if tab is None: # pragma: no cover
self.setText('')
self.hide()
self.widget.hide()
return
self.on_tab_changed(tab)

Expand All @@ -34,4 +34,4 @@ def on_tab_changed(self, tab):
if text:
text = '[' + text + ']'
self.setText(text)
self.setVisible(bool(text) and self.enabled)
self.widget.setVisible(bool(text) and self.enabled)
17 changes: 9 additions & 8 deletions qutebrowser/mainwindow/statusbar/bar.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def __init__(self, *, win_id, private, parent=None):
window=win_id)

self.txt = textbase.TextBase()
self._stack.addWidget(self.txt)
self._stack.addWidget(self.txt.widget)

self.cmd.show_cmd.connect(self._show_cmd_widget)
self.cmd.hide_cmd.connect(self._hide_cmd_widget)
Expand Down Expand Up @@ -242,7 +242,7 @@ def _draw_widgets(self):
# Read the list and set widgets accordingly
for segment in config.val.statusbar.widgets:
widget = self._get_widget_from_config(segment)
self._hbox.addWidget(widget)
self._hbox.addWidget(widget.widget)

if segment == 'scroll_raw':
widget.set_raw()
Expand All @@ -263,16 +263,17 @@ def _draw_widgets(self):
else:
widget.format = '%X'

widget.show()
widget.widget.show()

def _clear_widgets(self):
"""Clear widgets before redrawing them."""
# Start with widgets hidden and show them when needed
for widget in [self.url, self.percentage,
self.backforward, self.tabindex,
self.keystring, self.prog, self.clock, *self._text_widgets]:
for widget in [self.url.widget, self.percentage.widget,
self.backforward.widget, self.tabindex.widget,
self.keystring.widget, self.prog.widget, self.clock.widget,
*[w.widget for w in self._text_widgets]]:
assert isinstance(widget, QWidget)
if widget in [self.prog, self.backforward]:
if widget in [self.prog.widget, self.backforward.widget]:
widget.enabled = False # type: ignore[attr-defined]
widget.hide()
self._hbox.removeWidget(widget)
Expand Down Expand Up @@ -365,7 +366,7 @@ def _show_cmd_widget(self):
def _hide_cmd_widget(self):
"""Show temporary text instead of command widget."""
log.statusbar.debug("Hiding cmd widget")
self._stack.setCurrentWidget(self.txt)
self._stack.setCurrentWidget(self.txt.widget)
self.maybe_hide()

@pyqtSlot(str)
Expand Down
6 changes: 3 additions & 3 deletions qutebrowser/mainwindow/statusbar/clock.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def __init__(self, parent=None):
super().__init__(parent, elidemode=Qt.TextElideMode.ElideNone)
self.format = ""

self.timer = QTimer(self)
self.timer = QTimer(self.widget)
self.timer.timeout.connect(self._show_time)

def _show_time(self):
Expand All @@ -30,10 +30,10 @@ def _show_time(self):
def hideEvent(self, event):
"""Stop timer when widget is hidden."""
self.timer.stop()
super().hideEvent(event)
self.widget.hideEvent(event)

def showEvent(self, event):
"""Override showEvent to show time and start self.timer for updating."""
self.timer.start(Clock.UPDATE_DELAY)
self._show_time()
super().showEvent(event)
self.widget.showEvent(event)
16 changes: 12 additions & 4 deletions qutebrowser/mainwindow/statusbar/keystring.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,24 @@

"""Keychain string displayed in the statusbar."""

from qutebrowser.qt.core import pyqtSlot
from qutebrowser.qt.core import pyqtSlot, Qt

from qutebrowser.mainwindow.statusbar import textbase
from qutebrowser.qt.widgets import QLabel
from qutebrowser.utils import usertypes


class KeyString(textbase.TextBase):

"""Keychain string displayed in the statusbar."""
class KeyStringWidget(QLabel):

Check warning on line 14 in qutebrowser/mainwindow/statusbar/keystring.py

View workflow job for this annotation

GitHub Actions / linters (flake8)

Missing docstring in public class

Check warning on line 14 in qutebrowser/mainwindow/statusbar/keystring.py

View workflow job for this annotation

GitHub Actions / linters (pylint)

Missing class docstring

@pyqtSlot(usertypes.KeyMode, str)
def on_keystring_updated(self, _mode, keystr):
self.setText(keystr)


class KeyString(textbase.TextBase):

"""Keychain string displayed in the statusbar."""

def __init__(self, parent=None, elidemode=Qt.TextElideMode.ElideRight):
super().__init__(parent, elidemode=elidemode)
self.widget = KeyStringWidget(parent)
42 changes: 26 additions & 16 deletions qutebrowser/mainwindow/statusbar/percentage.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,46 @@
"""Scroll percentage displayed in the statusbar."""

from qutebrowser.qt.core import pyqtSlot, Qt
from qutebrowser.qt.widgets import QLabel

from qutebrowser.mainwindow.statusbar import textbase
from qutebrowser.misc import throttle
from qutebrowser.utils import utils


class PercentageWidget(QLabel):

Check warning on line 15 in qutebrowser/mainwindow/statusbar/percentage.py

View workflow job for this annotation

GitHub Actions / linters (flake8)

Missing docstring in public class

Check warning on line 15 in qutebrowser/mainwindow/statusbar/percentage.py

View workflow job for this annotation

GitHub Actions / linters (pylint)

Missing class docstring

def __init__(self, parent=None):
super().__init__(parent)
self._strings = {}
self._set_text = None

@pyqtSlot(int, int)
def set_perc(self, x, y):
"""Setter to be used as a Qt slot.
Args:
x: The x percentage (int), currently ignored.
y: The y percentage (int)
"""
utils.unused(x)
self._set_text(self._strings.get(y, '[???]'))


class Percentage(textbase.TextBase):

"""Reading percentage displayed in the statusbar."""

def __init__(self, parent=None):
"""Constructor. Set percentage to 0%."""
super().__init__(parent, elidemode=Qt.TextElideMode.ElideNone)
self._strings = self._calc_strings()
self._set_text = throttle.Throttle(self.setText, 100, parent=self)
self.set_perc(0, 0)
self.widget = PercentageWidget(parent)
self.widget._strings = self._calc_strings()
self.widget._set_text = throttle.Throttle(self.setText, 100, parent=self.widget)
self.widget.set_perc(0, 0)

def set_raw(self):
self._strings = self._calc_strings(raw=True)
self.widget._strings = self._calc_strings(raw=True)

def _calc_strings(self, raw=False):
"""Pre-calculate strings for the statusbar."""
Expand All @@ -32,17 +53,6 @@ def _calc_strings(self, raw=False):
strings.update({0: '[top]', 100: '[bot]'})
return strings

@pyqtSlot(int, int)
def set_perc(self, x, y):
"""Setter to be used as a Qt slot.
Args:
x: The x percentage (int), currently ignored.
y: The y percentage (int)
"""
utils.unused(x)
self._set_text(self._strings.get(y, '[???]'))

def on_tab_changed(self, tab):
"""Update scroll position when tab changed."""
self.set_perc(*tab.scroller.pos_perc())
self.widget.set_perc(*tab.scroller.pos_perc())
68 changes: 39 additions & 29 deletions qutebrowser/mainwindow/statusbar/progress.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,33 +11,11 @@
from qutebrowser.utils import utils, usertypes


class Progress(QProgressBar):

"""The progress bar part of the status bar."""

STYLESHEET = """
QProgressBar {
border-radius: 0px;
border: 2px solid transparent;
background-color: transparent;
font: {{ conf.fonts.statusbar }};
}
QProgressBar::chunk {
background-color: {{ conf.colors.statusbar.progress.bg }};
}
"""
class ProgressWidget(QProgressBar):

Check warning on line 14 in qutebrowser/mainwindow/statusbar/progress.py

View workflow job for this annotation

GitHub Actions / linters (flake8)

Missing docstring in public class

Check warning on line 14 in qutebrowser/mainwindow/statusbar/progress.py

View workflow job for this annotation

GitHub Actions / linters (pylint)

Missing class docstring

def __init__(self, parent=None):
super().__init__(parent)
stylesheet.set_register(self)
self.enabled = False
self.setSizePolicy(QSizePolicy.Policy.Fixed, QSizePolicy.Policy.Fixed)
self.setTextVisible(False)
self.hide()

def __repr__(self):
return utils.get_repr(self, value=self.value())

@pyqtSlot()
def on_load_started(self):
Expand All @@ -58,18 +36,50 @@ def on_load_progress(self, value):
if value == 100:
self.hide()


class Progress:

"""The progress bar part of the status bar."""

STYLESHEET = """
QProgressBar {
border-radius: 0px;
border: 2px solid transparent;
background-color: transparent;
font: {{ conf.fonts.statusbar }};
}
QProgressBar::chunk {
background-color: {{ conf.colors.statusbar.progress.bg }};
}
"""

def __init__(self, parent=None):
self.widget = ProgressWidget(parent)
# FIXME: this is horrible
self.widget.STYLESHEET = self.STYLESHEET
stylesheet.set_register(self.widget)
self.widget.enabled = False
self.widget.setSizePolicy(QSizePolicy.Policy.Fixed, QSizePolicy.Policy.Fixed)
self.widget.setTextVisible(False)
self.widget.hide()

def __repr__(self):
return utils.get_repr(self, value=self.widget.value())


def on_tab_changed(self, tab):

Check warning on line 71 in qutebrowser/mainwindow/statusbar/progress.py

View workflow job for this annotation

GitHub Actions / linters (flake8)

too many blank lines (2)
"""Set the correct value when the current tab changed."""
self.setValue(tab.progress())
if self.enabled and tab.load_status() == usertypes.LoadStatus.loading:
self.show()
self.widget.setValue(tab.progress())
if self.widget.enabled and tab.load_status() == usertypes.LoadStatus.loading:
self.widget.show()
else:
self.hide()
self.widget.hide()

def sizeHint(self):
"""Set the height to the text height."""
width = super().sizeHint().width()
height = self.fontMetrics().height()
width = self.widget.sizeHint().width()
height = self.widget.fontMetrics().height()
return QSize(width, height)

def minimumSizeHint(self):
Expand Down
14 changes: 11 additions & 3 deletions qutebrowser/mainwindow/statusbar/searchmatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@


from qutebrowser.qt.core import pyqtSlot
from qutebrowser.qt.widgets import QLabel

from qutebrowser.browser import browsertab
from qutebrowser.mainwindow.statusbar import textbase
from qutebrowser.utils import log


class SearchMatch(textbase.TextBase):

"""The part of the statusbar that displays the search match counter."""
class SerachMatchWidget(QLabel):

Check warning on line 16 in qutebrowser/mainwindow/statusbar/searchmatch.py

View workflow job for this annotation

GitHub Actions / linters (flake8)

Missing docstring in public class

Check warning on line 16 in qutebrowser/mainwindow/statusbar/searchmatch.py

View workflow job for this annotation

GitHub Actions / linters (pylint)

Missing class docstring

@pyqtSlot(browsertab.SearchMatch)
def set_match(self, search_match: browsertab.SearchMatch) -> None:
Expand All @@ -31,3 +30,12 @@ def set_match(self, search_match: browsertab.SearchMatch) -> None:
else:
self.setText(f'Match [{search_match}]')
log.statusbar.debug(f'Setting search match text to {search_match}')


class SearchMatch(textbase.TextBase):

"""The part of the statusbar that displays the search match counter."""

def __init__(self, parent=None):
super().__init__(parent)
self.widget = SerachMatchWidget(parent)
16 changes: 12 additions & 4 deletions qutebrowser/mainwindow/statusbar/tabindex.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,24 @@

"""TabIndex displayed in the statusbar."""

from qutebrowser.qt.core import pyqtSlot
from qutebrowser.qt.core import pyqtSlot, Qt
from qutebrowser.qt.widgets import QLabel

from qutebrowser.mainwindow.statusbar import textbase


class TabIndex(textbase.TextBase):

"""Shows current tab index and number of tabs in the statusbar."""
class TabIndexWidget(QLabel):

Check warning on line 13 in qutebrowser/mainwindow/statusbar/tabindex.py

View workflow job for this annotation

GitHub Actions / linters (flake8)

Missing docstring in public class

Check warning on line 13 in qutebrowser/mainwindow/statusbar/tabindex.py

View workflow job for this annotation

GitHub Actions / linters (pylint)

Missing class docstring

@pyqtSlot(int, int)
def on_tab_index_changed(self, current, count):
"""Update tab index when tab changed."""
self.setText('[{}/{}]'.format(current + 1, count))


class TabIndex(textbase.TextBase):

"""Shows current tab index and number of tabs in the statusbar."""

def __init__(self, parent=None, elidemode=Qt.TextElideMode.ElideRight):
super().__init__(parent, elidemode)
self.widget = TabIndexWidget(parent)
Loading

0 comments on commit c761737

Please sign in to comment.