Prompt once to reload if more than one file modified externally (bis) (#180)

* Create only one dialog even if multiple files has been changed on disk
* fix: the calls to `NumericDialog` weren't working
* use `transient_for` instead of `parent` when creating new `Gtk.Dialog`
* Changelog: I removed the "thanks to" (just kept the mentions to the users), because it felt weird to put "thanks to myself" 😅 

Co-authored-by: Yurii Zolotko <yurii.zolotko@pm.me>

CHANGELOG.md

@@ -9,11 +9,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
-- Language: initial support for Rust language (thanks to @alopatindev)
+- Language: initial support for Rust language (@alopatindev)
 
 ### Changed
 
-- Translation: updated Swedish translation (thanks to @eson57)
+- Translation: updated Swedish translation (@eson57)
+- Dialog: prompt only once if several files needs to be reloaded (@yuriiz)
+
+### Fixed
+
+- fix: "Go to line..." dialog didn't show up (@MightyCreak)
+- Tech debt: use `transient_for` instead of the deprecated `parent` when creating
+  a `Gtk.Widget` (@MightyCreak)
 
 - Documentation: prefer `pip3` over `pip` to ensure it works everywhere (thanks to @krlmlr)
  
@@ -21,7 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
-- Translation: updated Spanish translation (thanks to @oscfdezdz)
+- Translation: updated Spanish translation (@oscfdezdz)
 - Translation: updated POT file
 - Translation: fixed issue with commented string that still needs translation
 
@@ -29,7 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
-- Port to Mac OS (thanks to @hugoholgersson)
+- Port to Mac OS (@hugoholgersson)
 
 ### Changed
 

data/io.github.mightycreak.Diffuse.appdata.xml.in

@@ -48,7 +48,7 @@
         </p>
         <p>Added:</p>
         <ul>
-          <li>Port to Mac OS (thanks to @hugoholgersson)</li>
+          <li>Port to Mac OS (@hugoholgersson)</li>
         </ul>
         <p>Changed:</p>
         <ul>

src/diffuse/dialogs.py

@@ -20,6 +20,7 @@
 import os
 
 from gettext import gettext as _
+from typing import Optional
 
 from diffuse import constants
 from diffuse import utils
@@ -74,7 +75,7 @@ class FileChooserDialog(Gtk.FileChooserDialog):
         FileChooserDialog.last_chosen_folder = widget.get_current_folder()
 
     def __init__(self, title, parent, prefs, action, accept, rev=False):
-        Gtk.FileChooserDialog.__init__(self, title=title, parent=parent, action=action)
+        Gtk.FileChooserDialog.__init__(self, title=title, transient_for=parent, action=action)
         self.add_button(Gtk.STOCK_CANCEL, Gtk.ResponseType.CANCEL)
         self.add_button(accept, Gtk.ResponseType.OK)
         self.prefs = prefs
@@ -82,15 +83,15 @@ class FileChooserDialog(Gtk.FileChooserDialog):
         label = Gtk.Label(label=_('Encoding: '))
         hbox.pack_start(label, False, False, 0)
         label.show()
-        self.encoding = entry = utils.EncodingMenu(
+        self._encoding = utils.EncodingMenu(
-            prefs,
+            prefs=prefs,
-            action in [Gtk.FileChooserAction.OPEN, Gtk.FileChooserAction.SELECT_FOLDER])
+            autodetect=action in [Gtk.FileChooserAction.OPEN, Gtk.FileChooserAction.SELECT_FOLDER])
-        hbox.pack_start(entry, False, False, 5)
+        hbox.pack_start(self._encoding, False, False, 5)
-        entry.show()
+        self._encoding.show()
         if rev:
-            self.revision = entry = Gtk.Entry()
+            self._revision = Gtk.Entry()
-            hbox.pack_end(entry, False, False, 0)
+            hbox.pack_end(self._revision, False, False, 0)
-            entry.show()
+            self._revision.show()
             label = Gtk.Label(label=_('Revision: '))
             hbox.pack_end(label, False, False, 0)
             label.show()
@@ -100,14 +101,14 @@ class FileChooserDialog(Gtk.FileChooserDialog):
         self.set_current_folder(self.last_chosen_folder)
         self.connect('current-folder-changed', self._current_folder_changed_cb)
 
-    def set_encoding(self, encoding):
+    def set_encoding(self, encoding: Optional[str]) -> None:
-        self.encoding.set_text(encoding)
+        self._encoding.set_text(encoding)
 
-    def get_encoding(self) -> str:
+    def get_encoding(self) -> Optional[str]:
-        return self.encoding.get_text()
+        return self._encoding.get_text()
 
     def get_revision(self) -> str:
-        return self.revision.get_text()
+        return self._revision.get_text()
 
     def get_filename(self) -> str:
         # convert from UTF-8 string to unicode
@@ -117,7 +118,7 @@ class FileChooserDialog(Gtk.FileChooserDialog):
 # dialogue used to search for text
 class NumericDialog(Gtk.Dialog):
     def __init__(self, parent, title, text, val, lower, upper, step=1, page=0):
-        Gtk.Dialog.__init__(self, title=title, parent=parent, destroy_with_parent=True)
+        Gtk.Dialog.__init__(self, title=title, transient_for=parent, destroy_with_parent=True)
         self.add_button(Gtk.STOCK_CANCEL, Gtk.ResponseType.REJECT)
         self.add_button(Gtk.STOCK_OK, Gtk.ResponseType.ACCEPT)
 
@@ -128,6 +129,7 @@ class NumericDialog(Gtk.Dialog):
         label = Gtk.Label(label=text)
         hbox.pack_start(label, False, False, 0)
         label.show()
+
         adj = Gtk.Adjustment(
             value=val,
             lower=lower,
@@ -135,10 +137,10 @@ class NumericDialog(Gtk.Dialog):
             step_increment=step,
             page_increment=page,
             page_size=0)
-        self.button = button = Gtk.SpinButton(adjustment=adj, climb_rate=1.0, digits=0)
+        self._button = Gtk.SpinButton(adjustment=adj, climb_rate=1.0, digits=0)
-        button.connect('activate', self.button_cb)
+        self._button.connect('activate', self._button_cb)
-        hbox.pack_start(button, True, True, 0)
+        hbox.pack_start(self._button, True, True, 0)
-        button.show()
+        self._button.show()
 
         vbox.pack_start(hbox, True, True, 0)
         hbox.show()
@@ -146,14 +148,21 @@ class NumericDialog(Gtk.Dialog):
         self.vbox.pack_start(vbox, False, False, 0)
         vbox.show()
 
-    def button_cb(self, widget):
+    def _button_cb(self, widget: Gtk.SpinButton) -> None:
         self.response(Gtk.ResponseType.ACCEPT)
 
+    def get_value(self) -> int:
+        return self._button.get_value_as_int()
+
 
 # dialogue used to search for text
 class SearchDialog(Gtk.Dialog):
     def __init__(self, parent, pattern=None, history=None):
-        Gtk.Dialog.__init__(self, title=_('Find...'), parent=parent, destroy_with_parent=True)
+        Gtk.Dialog.__init__(
+            self,
+            title=_('Find...'),
+            transient_for=parent,
+            destroy_with_parent=True)
         self.add_button(Gtk.STOCK_CANCEL, Gtk.ResponseType.REJECT)
         self.add_button(Gtk.STOCK_OK, Gtk.ResponseType.ACCEPT)
 
@@ -165,11 +174,11 @@ class SearchDialog(Gtk.Dialog):
         hbox.pack_start(label, False, False, 0)
         label.show()
         combo = Gtk.ComboBoxText.new_with_entry()
-        self.entry = combo.get_child()
+        self._entry = combo.get_child()
-        self.entry.connect('activate', self.entry_cb)
+        self._entry.connect('activate', self._entry_cb)
 
         if pattern is not None:
-            self.entry.set_text(pattern)
+            self._entry.set_text(pattern)
 
         if history is not None:
             completion = Gtk.EntryCompletion()
@@ -179,7 +188,7 @@ class SearchDialog(Gtk.Dialog):
             for h in history:
                 liststore.append([h])
                 combo.append_text(h)
-            self.entry.set_completion(completion)
+            self._entry.set_completion(completion)
 
         hbox.pack_start(combo, True, True, 0)
         combo.show()
@@ -200,5 +209,8 @@ class SearchDialog(Gtk.Dialog):
         vbox.show()
 
     # callback used when the Enter key is pressed
-    def entry_cb(self, widget):
+    def _entry_cb(self, widget: Gtk.Entry) -> None:
         self.response(Gtk.ResponseType.ACCEPT)
+
+    def get_search_text(self) -> str:
+        return self._entry.get_text()

src/diffuse/main.py

@@ -177,6 +177,19 @@ class Diffuse(Gtk.Window):
                     self.has_edits = has_edits
                     self.updateTitle()
 
+            # Has the file on disk changed since last time it was loaded?
+            def has_file_changed_on_disk(self) -> bool:
+                if self.info.last_stat is not None:
+                    try:
+                        new_stat = os.stat(self.info.name)
+                        if self.info.last_stat[stat.ST_MTIME] < new_stat[stat.ST_MTIME]:
+                            # update our notion of the most recent modification
+                            self.info.last_stat = new_stat
+                            return True
+                    except OSError:
+                        return False
+                return False
+
         # pane footer
         class PaneFooter(Gtk.Box):
             def __init__(self) -> None:
@@ -465,36 +478,6 @@ class Diffuse(Gtk.Window):
         def reload_file_cb(self, widget, data):
             self.open_file(self.current_pane, True)
 
-        # check changes to files on disk when receiving keyboard focus
-        def focus_in(self, widget, event):
-            for f, h in enumerate(self.headers):
-                info = h.info
-                try:
-                    if info.last_stat is not None:
-                        info = h.info
-                        new_stat = os.stat(info.name)
-                        if info.last_stat[stat.ST_MTIME] < new_stat[stat.ST_MTIME]:
-                            # update our notion of the most recent modification
-                            info.last_stat = new_stat
-                            if info.label is not None:
-                                s = info.label
-                            else:
-                                s = info.name
-                            msg = _(
-                                'The file %s changed on disk. Do you want to reload the file?'
-                            ) % (s, )
-                            dialog = utils.MessageDialog(
-                                self.get_toplevel(),
-                                Gtk.MessageType.QUESTION,
-                                msg
-                            )
-                            ok = (dialog.run() == Gtk.ResponseType.OK)
-                            dialog.destroy()
-                            if ok:
-                                self.open_file(f, True)
-                except OSError:
-                    pass
-
         # save contents of pane 'f' to file
         def save_file(self, f: int, save_as: bool = False) -> bool:
             h = self.headers[f]
@@ -626,10 +609,10 @@ class Diffuse(Gtk.Window):
                 _('Line Number: '),
                 val=1,
                 lower=1,
-                step=self.panes[self.current_pane].max_line_number + 1
+                upper=self.panes[self.current_pane].max_line_number + 1
             )
             okay = (dialog.run() == Gtk.ResponseType.ACCEPT)
-            i = dialog.button.get_value_as_int()
+            i = dialog.get_value()
             dialog.destroy()
             if okay:
                 self.go_to_line(i)
@@ -958,8 +941,50 @@ class Diffuse(Gtk.Window):
     # notifies all viewers on focus changes so they may check for external
     # changes to files
     def focus_in_cb(self, widget, event):
+        changed = []
         for i in range(self.notebook.get_n_pages()):
-            self.notebook.get_nth_page(i).focus_in(widget, event)
+            page = self.notebook.get_nth_page(i)
+            for f, h in enumerate(page.headers):
+                if h.has_file_changed_on_disk():
+                    changed.append((page, f))
+
+        if changed:
+            filenames = []
+            for (page, f) in changed:
+                h = page.headers[f]
+                filename = h.info.label if h.info.label is not None else h.info.name
+                filenames.append(filename)
+
+            primary_text = _("Changes detected")
+            secondary_text = ""
+            if len(filenames) == 1:
+                secondary_text = _(
+                    "The file \"%s\" changed on disk.\n\n"
+                    "Do you want to reload the file?"
+                ) % (filenames[0],)
+            else:
+                secondary_text = _(
+                    "The following files changed on disk:\n%s\n\n"
+                    "Do you want to reload these files?"
+                ) % ("\n".join("- " + filename for filename in filenames),)
+
+            dialog = Gtk.MessageDialog(
+                transient_for=self.get_toplevel(),
+                message_type=Gtk.MessageType.QUESTION,
+                buttons=Gtk.ButtonsType.YES_NO,
+                text=primary_text)
+            dialog.format_secondary_text(secondary_text)
+            dialog.set_default_response(Gtk.ResponseType.YES)
+
+            button = dialog.get_widget_for_response(Gtk.ResponseType.YES)
+            button.get_style_context().add_class(Gtk.STYLE_CLASS_SUGGESTED_ACTION)
+
+            response = dialog.run()
+            dialog.destroy()
+
+            if response == Gtk.ResponseType.YES:
+                for page, f in changed:
+                    page.open_file(f, True)
 
     # record the window's position and size
     def configure_cb(self, widget, event):
@@ -1050,7 +1075,7 @@ class Diffuse(Gtk.Window):
             return True
 
         # ask the user which files should be saved
-        dialog = Gtk.MessageDialog(parent=self.get_toplevel(),
+        dialog = Gtk.MessageDialog(transient_for=self.get_toplevel(),
                                    destroy_with_parent=True,
                                    message_type=Gtk.MessageType.WARNING,
                                    buttons=Gtk.ButtonsType.NONE,
@@ -1487,7 +1512,7 @@ class Diffuse(Gtk.Window):
             upper=16
         )
         okay = (dialog.run() == Gtk.ResponseType.ACCEPT)
-        npanes = dialog.button.get_value_as_int()
+        npanes = dialog.get_value()
         dialog.destroy()
         if okay:
             viewer = self.newFileDiffViewer(npanes)
@@ -1528,7 +1553,7 @@ class Diffuse(Gtk.Window):
             dialog.backwards_button.set_active(self.bool_state['search_backwards'])
             keep = (dialog.run() == Gtk.ResponseType.ACCEPT)
             # persist the search options
-            pattern = dialog.entry.get_text()
+            pattern = dialog.get_search_text()
             match_case = dialog.match_case_button.get_active()
             backwards = dialog.backwards_button.get_active()
             dialog.destroy()

src/diffuse/utils.py

@@ -38,18 +38,18 @@ from gi.repository import Gtk  # type: ignore # noqa: E402
 
 # convenience class for displaying a message dialogue
 class MessageDialog(Gtk.MessageDialog):
-    def __init__(self, parent: Gtk.Widget, message_type: Gtk.MessageType, s: str) -> None:
+    def __init__(self, parent: Gtk.Widget, message_type: Gtk.MessageType, text: str) -> None:
         if message_type == Gtk.MessageType.ERROR:
             buttons = Gtk.ButtonsType.OK
         else:
             buttons = Gtk.ButtonsType.OK_CANCEL
         Gtk.MessageDialog.__init__(
             self,
-            parent=parent,
+            transient_for=parent,
             destroy_with_parent=True,
             message_type=message_type,
             buttons=buttons,
-            text=s)
+            text=text)
         self.set_title(constants.APP_NAME)
 
 
@@ -57,15 +57,15 @@ class MessageDialog(Gtk.MessageDialog):
 class EncodingMenu(Gtk.Box):
     def __init__(self, prefs: Preferences, autodetect: bool = False) -> None:
         Gtk.Box.__init__(self, orientation=Gtk.Orientation.HORIZONTAL)
-        self.combobox = combobox = Gtk.ComboBoxText()
+        self.combobox = Gtk.ComboBoxText()
         self.encodings = prefs.getEncodings()[:]
         for e in self.encodings:
-            combobox.append_text(e)
+            self.combobox.append_text(e)
         if autodetect:
+            self.combobox.prepend_text(_('Auto Detect'))
             self.encodings.insert(0, None)
-            combobox.prepend_text(_('Auto Detect'))
+        self.pack_start(self.combobox, False, False, 0)
-        self.pack_start(combobox, False, False, 0)
+        self.combobox.show()
-        combobox.show()
 
     def set_text(self, encoding: Optional[str]) -> None:
         encoding = norm_encoding(encoding)
@@ -169,7 +169,7 @@ def popenRead(
         cmd: List[str],
         prefs: Preferences,
         bash_pref: str,
-        success_results: List[int] = None) -> bytes:
+        success_results: Optional[List[int]] = None) -> bytes:
     if success_results is None:
         success_results = [0]
 
@@ -239,7 +239,7 @@ def popenReadLines(
         cmd: List[str],
         prefs: Preferences,
         bash_pref: str,
-        success_results: List[int] = None) -> List[str]:
+        success_results: Optional[List[int]] = None) -> List[str]:
     return _strip_eols(splitlines(popenRead(
         cwd, cmd, prefs, bash_pref, success_results).decode('utf-8', errors='ignore')))