Reload warning (#276)

* change statusbar to be only for FileTabs

* simplify statusbar plugin a lot

* add reload warning

* get rid of old status property

* only show warning when file was modified

more_plugins/tetris.py

@@ -6,6 +6,7 @@
 import itertools
 import random
 import tkinter
+from tkinter import ttk
 from typing import Dict, Iterator, List, Optional, Tuple
 
 from porcupine import get_tab_manager, menubar, tabs, utils
@@ -227,6 +228,9 @@ def __init__(self, manager: tabs.TabManager) -> None:
             relief='ridge', bg='black', takefocus=True)
         self._canvas.pack()
 
+        self._score_label = ttk.Label(self)
+        self._score_label.pack()
+
         # this also requires binding on the tab when the tab is detached
         for key in ['<W>', '<w>', '<A>', '<a>', '<S>', '<s>', '<D>', '<d>',
                     '<Left>', '<Right>', '<Up>', '<Down>', '<Return>',
@@ -283,8 +287,7 @@ def _refresh(self) -> None:
                 color = COLORS[shape]
             self._canvas.itemconfig(item_id, fill=color)
 
-        self.status = "Score %d, level %d" % (
-            self._game.score, self._game.level)
+        self._score_label['text'] = f"Score {self._game.score}, level {self._game.level}"
 
     def new_game(self) -> None:
         if self._timeout_id is not None:

porcupine/plugins/statusbar.py

@@ -1,57 +1,51 @@
-"""Display a status bar in each tab."""
-import tkinter
+"""Display a status bar in each file tab."""
 from tkinter import ttk
 
-from porcupine import get_tab_manager, tabs
-
-# i have experimented with a logging handler that displays logging
-# messages in the label, but it's not as good idea as it sounds like,
-# not all INFO messages are something that users should see all the time
-
-
-# this widget is kind of weird
-class LabelWithEmptySpaceAtLeft(ttk.Label):
-
-    def __init__(self, master: tkinter.BaseWidget) -> None:
-        self._spacer = ttk.Frame(master)
-        self._spacer.pack(side='left', expand=True)
-        super().__init__(master)
-        self.pack(side='left')
-
-    def destroy(self) -> None:
-        self._spacer.destroy()
-        super().destroy()
+from porcupine import get_tab_manager, tabs, utils
 
 
 class StatusBar(ttk.Frame):
 
-    def __init__(self, master: tkinter.BaseWidget, tab: tabs.Tab):
-        super().__init__(master)
+    def __init__(self, tab: tabs.FileTab):
+        super().__init__(tab.bottom_frame)
         self.tab = tab
-        # one label for each tab-separated thing
-        self.labels = [ttk.Label(self)]
-        self.labels[0].pack(side='left')
+        self.left_label = ttk.Label(self)
+        self.right_label = ttk.Label(self)
+        self.left_label.pack(side='left')
+        self.right_label.pack(side='right')
 
-        tab.bind('<<StatusChanged>>', self.do_update, add=True)
-        self.do_update()
+    def show_path(self, junk: object = None) -> None:
+        self.left_label.config(text=("New file" if self.tab.path is None else str(self.tab.path)))
 
-    # this is do_update() because tkinter has a method called update()
-    def do_update(self, junk: object = None) -> None:
-        parts = self.tab.status.split('\t')
+    def show_cursor_pos(self, junk: object = None) -> None:
+        line, column = self.tab.textwidget.index('insert').split('.')
+        self.right_label.config(text=f"Line {line}, column {column}")
 
-        # there's always at least one part, the label added in
-        # __init__ is not destroyed here
-        while len(self.labels) > len(parts):
-            self.labels.pop().destroy()
-        while len(self.labels) < len(parts):
-            self.labels.append(LabelWithEmptySpaceAtLeft(self))
+    # TODO: it's likely not ctrl+z on mac
+    def show_reload_warning(self, event: utils.EventWithData) -> None:
+        if event.data_class(tabs.ReloadInfo).was_modified:
+            self.left_label.config(
+                foreground='red',
+                text="File was reloaded with unsaved changes. Press Ctrl+Z to get your changes back.",
+            )
 
-        for label, text in zip(self.labels, parts):
-            label.config(text=text)
+    def clear_reload_warning(self, junk: object) -> None:
+        if self.left_label['foreground']:
+            self.left_label.config(foreground='')
+            self.show_path()
 
 
 def on_new_tab(tab: tabs.Tab) -> None:
-    StatusBar(tab.bottom_frame, tab).pack(side='bottom', fill='x')
+    if isinstance(tab, tabs.FileTab):
+        statusbar = StatusBar(tab)
+        statusbar.pack(side='bottom', fill='x')
+        tab.bind('<<PathChanged>>', statusbar.show_path, add=True)
+        utils.bind_with_data(tab, '<<Reloaded>>', statusbar.show_reload_warning, add=True)
+        tab.textwidget.bind('<<CursorMoved>>', statusbar.show_cursor_pos, add=True)
+        tab.textwidget.bind('<<ContentChanged>>', statusbar.clear_reload_warning, add=True)
+
+        statusbar.show_path()
+        statusbar.show_cursor_pos()
 
 
 def setup() -> None:

porcupine/tabs.py

@@ -1,5 +1,6 @@
 r"""Tabs as in browser tabs, not \t characters."""
 
+import dataclasses
 import hashlib
 import importlib
 import itertools
@@ -203,7 +204,7 @@ def add_tab_callback(self, func: Callable[['Tab'], Any]) -> None:
 
 
 class Tab(ttk.Frame):
-    r"""Base class for widgets that can be added to TabManager.
+    """Base class for widgets that can be added to TabManager.
 
     You can easily create custom kinds of tabs by inheriting from this
     class. Here's a very minimal but complete example plugin::
@@ -235,11 +236,6 @@ def setup():
         is bound on the tab and not the tab manager, and hence is automatically
         unbound when the tab is destroyed.
 
-    .. virtualevent:: StatusChanged
-
-        This event is generated when :attr:`status` is set to a new
-        value. Use ``event.widget.status`` to access the current status.
-
     .. attribute:: title_choices
 
         A :class:`typing.Sequence` of strings that can be used as the title of
@@ -251,19 +247,6 @@ def setup():
         will be ``baz.py``, but if you also open ``foo/bar2/baz.py`` then the
         titles change to ``bar/baz.py`` and ``bar2/baz.py``.
 
-    .. attribute:: status
-
-        A human-readable string for showing in e.g. a status bar.
-
-        The status message can also contain multiple tab-separated
-        things, e.g. ``"File 'thing.py'\tLine 12, column 34"``.
-
-        This is ``''`` by default, but that can be changed like
-        ``tab.status = something_new``.
-
-        If you're writing something like a status bar, make sure to
-        handle ``\t`` characters and bind :virtevt:`~StatusChanged`.
-
     .. attribute:: master
 
         Tkinter sets this to the parent widget. Use this attribute to
@@ -288,7 +271,6 @@ def setup():
 
     def __init__(self, manager: TabManager) -> None:
         super().__init__(manager)
-        self._status = ''
         self._titles: Sequence[str] = ['']
 
         # top and bottom frames must be packed first because this way
@@ -302,15 +284,6 @@ def __init__(self, manager: TabManager) -> None:
         self.left_frame.pack(side='left', fill='y')
         self.right_frame.pack(side='right', fill='y')
 
-    @property
-    def status(self) -> str:
-        return self._status
-
-    @status.setter
-    def status(self, new_status: str) -> None:
-        self._status = new_status
-        self.event_generate('<<StatusChanged>>')
-
     @property
     def title_choices(self) -> Sequence[str]:
         return self._titles
@@ -400,6 +373,11 @@ def _import_lexer_class(name: str) -> LexerMeta:
     return klass
 
 
+@dataclasses.dataclass
+class ReloadInfo(utils.EventDataclass):
+    was_modified: bool
+
+
 class FileTab(Tab):
     """A subclass of :class:`.Tab` that represents an opened file.
 
@@ -517,8 +495,6 @@ def __init__(self, manager: TabManager, content: str = '',
         self._set_saved_state(None)
 
         self.bind('<<TabSelected>>', (lambda event: self.textwidget.focus()), add=True)
-        self.bind('<<PathChanged>>', self._update_status, add=True)
-        self.textwidget.bind('<<CursorMoved>>', self._update_status, add=True)
 
         self.scrollbar = ttk.Scrollbar(self.right_frame)
         self.scrollbar.pack(side='right', fill='y')
@@ -527,7 +503,6 @@ def __init__(self, manager: TabManager, content: str = '',
 
         self.textwidget.bind('<<ContentChanged>>', self._update_titles, add=True)
         self._update_titles()
-        self._update_status()
 
     @classmethod
     def open_file(cls: Type[_FileTabT], manager: TabManager, path: pathlib.Path) -> _FileTabT:
@@ -585,6 +560,8 @@ def reload(self) -> None:
             assert isinstance(f.newlines, str)
             self.settings.set('line_ending', settings.LineEnding(f.newlines))
 
+        modified_before = self.is_modified()
+
         # Reloading can be undoed with Ctrl+Z
         self.textwidget.config(autoseparators=False)
         try:
@@ -597,7 +574,7 @@ def reload(self) -> None:
         self._set_saved_state(stat_result)
 
         # TODO: document this
-        self.event_generate('<<Reloaded>>')
+        self.event_generate('<<Reloaded>>', data=ReloadInfo(was_modified=modified_before))
 
     def other_program_changed_file(self) -> bool:
         """Check whether some other program has changed the file.
@@ -673,14 +650,6 @@ def _update_titles(self, junk: object = None) -> None:
 
         self.title_choices = titles
 
-    def _update_status(self, junk: object = None) -> None:
-        if self.path is None:
-            path_string = "New file"
-        else:
-            path_string = "File '%s'" % self.path
-        line, column = self.textwidget.index('insert').split('.')
-        self.status = f"{path_string}\tLine {line}, column {column}"
-
     def can_be_closed(self) -> bool:    # override
         if not self.is_modified():
             return True

tests/test_statusbar_plugin.py

@@ -1,29 +1,25 @@
-from porcupine import tabs
+from porcupine.plugins.statusbar import StatusBar
 
 
-# it must work with all kinds of tabs, not just FileTabs
-class AsdTab(tabs.Tab):
-    pass
+def test_reload_warning(filetab, tmp_path):
+    [statusbar] = [w for w in filetab.bottom_frame.winfo_children() if isinstance(w, StatusBar)]
 
+    filetab.path = tmp_path / "lol.py"
+    filetab.save()
 
-def test_that_it_doesnt_crash_with_different_numbers_of_tab_separated_parts(tabmanager):
-    asd = AsdTab(tabmanager)
-    tabmanager.add_tab(asd)
+    filetab.path.write_text("hello")
+    filetab.reload()
+    filetab.update()
+    assert statusbar.left_label['text'].endswith('lol.py')
+    assert statusbar.left_label['foreground'] == ''
 
-    events = []
-    asd.bind('<<StatusChanged>>', events.append, add=True)
+    filetab.textwidget.insert('1.0', 'asdf')
+    filetab.path.write_text("foo")
+    filetab.reload()
+    filetab.update()
+    assert 'Press Ctrl+Z to get your changes back' in statusbar.left_label['text']
+    assert statusbar.left_label['foreground'] != ''
 
-    asd.status = "a"
-    asd.update()
-    events.pop()
-
-    asd.status = "a\tb\tc"
-    asd.update()
-    events.pop()
-
-    asd.status = "a"
-    asd.update()
-    events.pop()
-
-    tabmanager.close_tab(asd)
-    assert not events
+    filetab.textwidget.insert('1.0', 'a')   # assume user doesn't want changes back
+    assert statusbar.left_label['text'].endswith('lol.py')
+    assert statusbar.left_label['foreground'] == ''