Merge pull request #197 from karlch/hierarchy-refactor

Refactor class hierarchy of the (status-) bar

docs/changelog.rst

@@ -31,6 +31,10 @@ Changed:
   the html-equivalent `` `` if there are multiple subsequent spaces. This keeps
   wanted additional spacing while allowing to use html code such as
   ``<span style='color: #8FBCBB; font-weight: bold;'>colored and bold</span>``.
+* Both the command line and the widget to display status messages are now overlay
+  widgets instead of being integrated with the bar. This decouples them from the main
+  grid layout and better reflects their role as they are being shown temporarily over
+  the current widget/image.
 
 Fixed:
 ^^^^^^
@@ -42,6 +46,7 @@ Fixed:
 * Crash when scrolling thumbnail mode with empty thumbnail list.
 * Crash when running ``:goto`` without valid paths/images/thumbnails.
 * Switching mode when toggling an inactive mode.
+* Displaying status messages larger than one line in manipulate mode.
 
 
 v0.6.1 (2020-03-07)

tests/end2end/features/commandline/test_commandline_bdd.py

@@ -11,7 +11,6 @@
 import vimiv
 from vimiv import api
 from vimiv.commands import runners
-from vimiv.gui import statusbar
 
 
 bdd.scenarios("commandline.feature")
@@ -29,20 +28,20 @@ def check_commandline_closed():
 
 @bdd.then(bdd.parsers.parse("the help for '{topic}' should be displayed"))
 @bdd.then("the help for '<topic>' should be displayed")
-def check_help(topic):
+def check_help(message_widget, topic):
     if topic == "vimiv":
-        assert vimiv.__description__ in statusbar.statusbar.message.text()
+        assert vimiv.__description__ in message_widget.text()
         return
     if topic == "wildcards":
-        assert "wildcards" in statusbar.statusbar.message.text().lower()
+        assert "wildcards" in message_widget.text().lower()
         return
     topic = topic.lower().lstrip(":")
     with suppress(api.commands.CommandNotFound):
         command = api.commands.get(topic, mode=api.modes.current())
-        assert command.description in statusbar.statusbar.message.text()
+        assert command.description in message_widget.text()
         return
     with suppress(KeyError):
         setting = api.settings.get(topic)
-        assert setting.desc in statusbar.statusbar.message.text()
+        assert setting.desc in message_widget.text()
         return
     raise AssertionError(f"Topic '{topic}' not found")

tests/end2end/features/conftest.py

@@ -18,13 +18,14 @@
 import vimiv.gui.library
 import vimiv.gui.thumbnail
 import vimiv.gui.mainwindow
+import vimiv.gui.message
 import vimiv.gui.commandline
-import vimiv.gui.bar
+import vimiv.gui.command_widget
 import vimiv.gui.image
 import vimiv.gui.prompt
+import vimiv.gui.statusbar
 from vimiv import api
 from vimiv.commands import runners
-from vimiv.gui import statusbar
 from vimiv.imutils import filelist
 
 
@@ -52,15 +53,32 @@ def commandline():
 
 
 @pytest.fixture()
-def bar():
-    yield vimiv.gui.bar.Bar.instance
+def statusbar(mainwindow):
+    yield mainwindow._statusbar
 
 
 @pytest.fixture()
 def image():
     yield vimiv.gui.image.ScrollableImage.instance
 
 
+@pytest.fixture()
+def message_widget(mainwindow):
+    return get_overlay(mainwindow, vimiv.gui.message.Message)
+
+
+@pytest.fixture()
+def overlay(mainwindow):
+    return lambda typ: get_overlay(mainwindow, typ)
+
+
+def get_overlay(mainwindow, typ):
+    for overlay in mainwindow._overlays:
+        if isinstance(overlay, typ):
+            return overlay
+    raise ValueError(f"{typ} not found")
+
+
 class Counter:
     """Counter class with the count command for simple testing of commands.
 
@@ -260,49 +278,42 @@ def no_crash(qtbot):
     qtbot.wait(1)
 
 
-@bdd.then(bdd.parsers.parse("the message\n'{message}'\nshould be displayed"))
-def check_statusbar_message(qtbot, message):
-    bar = statusbar.statusbar
+@bdd.then(bdd.parsers.parse("the {position} status should include {text}"))
+def check_status_text(qtbot, statusbar, position, text):
+    message = f"statusbar {position} should include {text}"
 
     def check_status():
-        assert message == bar.message.text(), "Message expected: '{message}'"
+        assert text in getattr(statusbar, position).text(), message
 
     qtbot.waitUntil(check_status, timeout=100)
-    assert bar.stack.currentWidget() == bar.message
+    assert statusbar.isVisible()
 
 
-@bdd.then(bdd.parsers.parse("the {position} status should include {text}"))
-def check_left_status(qtbot, position, text):
-    bar = statusbar.statusbar
-    message = f"statusbar {position} should include {text}"
-
-    def check_status():
-        assert text in getattr(bar.status, position).text(), message
+@bdd.then(bdd.parsers.parse("the message\n'{message}'\nshould be displayed"))
+def check_message(qtbot, message_widget, message):
+    def check():
+        assert message == message_widget.text(), "Message expected: '{message}'"
 
-    qtbot.waitUntil(check_status, timeout=100)
-    assert bar.stack.currentWidget() == bar.status
+    qtbot.waitUntil(check, timeout=100)
+    assert message_widget.isVisible()
 
 
 @bdd.then("a message should be displayed")
-def check_a_statusbar_message(qtbot):
-    bar = statusbar.statusbar
+def check_any_message(qtbot, message_widget):
+    def check():
+        assert message_widget.text(), "Any message expected"
 
-    def check_status():
-        assert bar.message.text(), "Any message expected"
-
-    qtbot.waitUntil(check_status, timeout=100)
-    assert bar.stack.currentWidget() == bar.message
+    qtbot.waitUntil(check, timeout=100)
+    assert message_widget.isVisible()
 
 
 @bdd.then("no message should be displayed")
-def check_no_statusbar_message(qtbot):
-    bar = statusbar.statusbar
+def check_no_message(qtbot, message_widget):
+    def check():
+        assert not message_widget.text(), "No message expected"
 
-    def check_status():
-        assert not bar.message.text(), "No message expected"
-
-    qtbot.waitUntil(check_status, timeout=100)
-    assert bar.stack.currentWidget() == bar.status
+    qtbot.waitUntil(check, timeout=100)
+    assert not message_widget.isVisible()
 
 
 @bdd.then(bdd.parsers.parse("the working directory should be {basename}"))

tests/end2end/features/misc/test_keyhint_bdd.py

@@ -13,8 +13,8 @@
 
 
 @pytest.fixture()
-def keyhint():
-    return keyhint_widget.KeyhintWidget.instance
+def keyhint(overlay):
+    return overlay(keyhint_widget.KeyhintWidget)
 
 
 bdd.scenarios("keyhint.feature")
@@ -73,9 +73,9 @@ def keyhint_widget_contains(keyhint, command):
 
 
 @bdd.then("the keyhint widget should be above the statusbar")
-def keyhint_widget_above_bar(keyhint, bar):
+def keyhint_widget_above_bar(keyhint, statusbar):
     keyhint_bottom = keyhint.y() + keyhint.height()
-    bar_top = bar.y()
+    bar_top = statusbar.y()
     assert keyhint_bottom == bar_top
 
 

tests/end2end/features/statusbar/message.feature

@@ -9,19 +9,10 @@ Feature: Push messages to the statusbar.
             'this is a warning'
             should be displayed
 
-    Scenario: Display message when bar is hidden
-        When I run set statusbar.show false
-        And I log the warning 'this is a warning'
-        Then the message
-            'this is a warning'
-            should be displayed
-        And the bar should be visible
-
-    Scenario: Hide statusbar after message
-        When I run set statusbar.show false
-        And I log the warning 'this is a warning'
+    Scenario: Hide message widget when clearing status
+        When I log the warning 'this is a warning'
         And I clear the status
-        Then the bar should not be visible
+        Then no message should be displayed
 
     Scenario: Clear message after key press
         When I log the warning 'this is a warning'

tests/end2end/features/statusbar/test_message_bdd.py

@@ -21,13 +21,3 @@ def log_warning(message, qtbot):
 @bdd.when("I clear the status")
 def clear_status():
     api.status.clear("clear in bdd step")
-
-
-@bdd.then("the bar should be visible")
-def check_bar_visible(bar):
-    assert bar.isVisible()
-
-
-@bdd.then("the bar should not be visible")
-def check_bar_not_visible(bar):
-    assert not bar.isVisible()

vimiv/completion/completer.py

@@ -23,7 +23,6 @@ class Completer(QObject):
         _completion: CompletionWidget object.
     """
 
-    @api.objreg.register
     def __init__(self, commandline, completion):
         super().__init__()
 
@@ -33,7 +32,6 @@ def __init__(self, commandline, completion):
         self._completion.activated.connect(self._complete)
         api.modes.COMMAND.first_entered.connect(self._init_models)
         self._cmd.textEdited.connect(self._on_text_changed)
-        self._cmd.editingFinished.connect(self._on_editing_finished)
 
     @property
     def proxy_model(self) -> api.completion.FilterProxyModel:
@@ -47,6 +45,11 @@ def model(self) -> api.completion.BaseModel:
     def has_completions(self) -> bool:
         return self.proxy_model.rowCount() > 0
 
+    def reset(self):
+        """Reset completion and models."""
+        self._completion.clearSelection()
+        self.proxy_model.reset()
+
     @utils.slot
     def _init_models(self):
         """Initialize all completionmodels from the corresponding module."""
@@ -70,13 +73,6 @@ def _on_text_changed(self, text: str):
         self.model.on_text_changed(text)
         self._show_unless_empty()
 
-    @utils.slot
-    def _on_editing_finished(self):
-        """Reset filter and hide completion widget."""
-        self._completion.clearSelection()
-        self.proxy_model.reset()
-        self._completion.hide()
-
     def _update_proxy_model(self, text: str):
         """Update completion proxy model depending on text.