From b0f939af322ffa5a8ac33a9e00a231d2674ebe77 Mon Sep 17 00:00:00 2001
From: Veronica Berglyd Olsen <1619840+vkbo@users.noreply.github.com>
Date: Fri, 31 Dec 2021 19:51:26 +0100
Subject: [PATCH] Apply open document shortcuts to the tree that has focus
 (#945)

* Better handling of Enter/Return on GUI trees, resolves #913
* Update test
* Generalise open selected handle functions
* Improve test coverage
* Update docstring
---
 novelwriter/gui/noveltree.py         | 18 +++---
 novelwriter/gui/outline.py           | 23 ++++---
 novelwriter/guimain.py               | 94 ++++++++++++++--------------
 tests/test_gui/test_gui_guimain.py   | 94 +++++++++++++++++++++++++++-
 tests/test_gui/test_gui_mainmenu.py  |  8 +--
 tests/test_gui/test_gui_noveltree.py |  2 +-
 tests/test_gui/test_gui_outline.py   | 23 +++++++
 7 files changed, 188 insertions(+), 74 deletions(-)

diff --git a/novelwriter/gui/noveltree.py b/novelwriter/gui/noveltree.py
index 3ab9d7c2..0f7ac02f 100644
--- a/novelwriter/gui/noveltree.py
+++ b/novelwriter/gui/noveltree.py
@@ -135,7 +135,7 @@ class GuiNovelTree(QTreeWidget):
         logger.verbose("Requesting refresh of the novel tree")
         treeChanged = self.theParent.treeView.changedSince(self._lastBuild)
         indexChanged = self.theIndex.novelChangedSince(self._lastBuild)
-        if not (treeChanged or indexChanged):
+        if not (treeChanged or indexChanged or overRide):
             logger.verbose("No changes have been made to the novel index")
             return
 
@@ -175,10 +175,13 @@ class GuiNovelTree(QTreeWidget):
         selected, return the first.
         """
         selItem = self.selectedItems()
+        tHandle = None
+        tLine = 0
         if selItem:
-            return selItem[0].data(self.C_TITLE, Qt.UserRole)[0]
+            tHandle = selItem[0].data(self.C_TITLE, Qt.UserRole)[0]
+            tLine = checkInt(selItem[0].data(self.C_TITLE, Qt.UserRole)[1], 1) - 1
 
-        return None
+        return tHandle, tLine
 
     ##
     #  Events
@@ -201,7 +204,7 @@ class GuiNovelTree(QTreeWidget):
             if not isinstance(selItem, QTreeWidgetItem):
                 return
 
-            tHandle = self.getSelectedHandle()
+            tHandle, _ = self.getSelectedHandle()
             if tHandle is None:
                 return
 
@@ -218,13 +221,8 @@ class GuiNovelTree(QTreeWidget):
         clicked, and send it to the main gui class for opening in the
         document editor.
         """
-        theData = tItem.data(self.C_TITLE, Qt.UserRole)
-        tHandle = theData[0]
-        tLine = checkInt(theData[1], 1)
-
-        logger.verbose("User selected entry with handle '%s' on line %s", tHandle, tLine)
+        tHandle, tLine = self.getSelectedHandle()
         self.theParent.openDocument(tHandle, tLine=tLine-1, doScroll=True)
-
         return
 
     def _itemSelected(self):
diff --git a/novelwriter/gui/outline.py b/novelwriter/gui/outline.py
index 687ba393..1ac83ff1 100644
--- a/novelwriter/gui/outline.py
+++ b/novelwriter/gui/outline.py
@@ -34,6 +34,7 @@ from PyQt5.QtWidgets import (
 )
 
 from novelwriter.enum import nwItemLayout, nwItemType, nwOutline
+from novelwriter.common import checkInt
 from novelwriter.constants import trConst, nwKeyWords, nwLabels
 
 logger = logging.getLogger(__name__)
@@ -196,6 +197,19 @@ class GuiOutline(QTreeWidget):
         self._firstView = True
         return
 
+    def getSelectedHandle(self):
+        """Get the currently selected handle. If multiple items are
+        selected, return the first.
+        """
+        selItem = self.selectedItems()
+        tHandle = None
+        tLine = 0
+        if selItem:
+            tHandle = selItem[0].data(self._colIdx[nwOutline.TITLE], Qt.UserRole)
+            tLine = checkInt(selItem[0].text(self._colIdx[nwOutline.LINE]), 1) - 1
+
+        return tHandle, tLine
+
     ##
     #  Slots
     ##
@@ -206,15 +220,8 @@ class GuiOutline(QTreeWidget):
         clicked, and send it to the main gui class for opening in the
         document editor.
         """
-        tHandle = tItem.data(self._colIdx[nwOutline.TITLE], Qt.UserRole)
-        try:
-            tLine = int(tItem.text(self._colIdx[nwOutline.LINE]))
-        except Exception:
-            tLine = 1
-
-        logger.verbose("User selected entry with handle '%s' on line %s", tHandle, tLine)
+        tHandle, tLine = self.getSelectedHandle()
         self.theParent.openDocument(tHandle, tLine=tLine-1, doScroll=True)
-
         return
 
     @pyqtSlot()
diff --git a/novelwriter/guimain.py b/novelwriter/guimain.py
index 2185e63c..5e1a3c53 100644
--- a/novelwriter/guimain.py
+++ b/novelwriter/guimain.py
@@ -262,9 +262,13 @@ class GuiMain(QMainWindow):
         # Shortcuts and Actions
         self._connectMenuActions()
 
-        keyReturn = QShortcut(self.treeView)
+        keyReturn = QShortcut(self)
         keyReturn.setKey(QKeySequence(Qt.Key_Return))
-        keyReturn.activated.connect(self._treeKeyPressReturn)
+        keyReturn.activated.connect(self._keyPressReturn)
+
+        keyEnter = QShortcut(self)
+        keyEnter.setKey(QKeySequence(Qt.Key_Enter))
+        keyEnter.activated.connect(self._keyPressReturn)
 
         keyEscape = QShortcut(self)
         keyEscape.setKey(QKeySequence(Qt.Key_Escape))
@@ -350,7 +354,7 @@ class GuiMain(QMainWindow):
     ##
 
     def newProject(self, projData=None):
-        """Create new project via the new project wizard.
+        """Create a new project via the new project wizard.
         """
         if self.hasProject:
             if not self.closeProject():
@@ -396,9 +400,9 @@ class GuiMain(QMainWindow):
         return True
 
     def closeProject(self, isYes=False):
-        """Closes the project if one is open. isYes is passed on from
-        the close application event so the user doesn't get prompted
-        twice to confirm.
+        """Close the project if one is open. isYes is passed on from the
+        close application event so the user doesn't get prompted twice
+        to confirm.
         """
         if not self.hasProject:
             # There is no project loaded, everything OK
@@ -598,6 +602,10 @@ class GuiMain(QMainWindow):
             logger.error("No project open")
             return False
 
+        if not self.theProject.projTree.checkType(tHandle, nwItemType.FILE):
+            logger.debug("Requested item '%s' is not a document", tHandle)
+            return False
+
         self.closeDocument()
         self.mainTabs.setCurrentWidget(self.splitDocs)
         if self.docEditor.loadText(tHandle, tLine):
@@ -798,22 +806,28 @@ class GuiMain(QMainWindow):
     ##
 
     def openSelectedItem(self):
-        """Open the selected documents.
+        """Open the selected item from the tree that is currently
+        active. It is not checked that the item is actually a document.
+        That should be handled by the openDocument function.
         """
         if not self.hasProject:
             logger.error("No project open")
             return False
 
-        tHandle = self.treeView.getSelectedHandle()
-        if tHandle is None:
+        tHandle = None
+        tLine = None
+        if self.treeView.hasFocus():
+            tHandle = self.treeView.getSelectedHandle()
+        elif self.novelView.hasFocus():
+            tHandle, tLine = self.novelView.getSelectedHandle()
+        elif self.projView.hasFocus():
+            tHandle, tLine = self.projView.getSelectedHandle()
+        else:
             logger.warning("No item selected")
             return False
 
-        logger.verbose("Opening item '%s'", tHandle)
-        if self.theProject.projTree.checkType(tHandle, nwItemType.FILE):
-            self.openDocument(tHandle, doScroll=False)
-        else:
-            logger.verbose("Requested item '%s' is not a file", tHandle)
+        if tHandle is not None:
+            self.openDocument(tHandle, tLine=tLine, changeFocus=False, doScroll=False)
 
         return True
 
@@ -986,7 +1000,7 @@ class GuiMain(QMainWindow):
         """
         if not self.hasProject:
             logger.error("No project open")
-            return
+            return False
 
         dlgProj = GuiProjectSettings(self)
         dlgProj.exec_()
@@ -996,14 +1010,14 @@ class GuiMain(QMainWindow):
             self.docEditor.setDictionaries()
             self._updateWindowTitle(self.theProject.projName)
 
-        return
+        return True
 
     def showProjectDetailsDialog(self):
         """Open the project details dialog.
         """
         if not self.hasProject:
             logger.error("No project open")
-            return
+            return False
 
         self.treeView.flushTreeOrder()
 
@@ -1016,14 +1030,14 @@ class GuiMain(QMainWindow):
         dlgDetails.raise_()
         dlgDetails.updateValues()
 
-        return
+        return True
 
     def showBuildProjectDialog(self):
         """Open the build project dialog.
         """
         if not self.hasProject:
             logger.error("No project open")
-            return
+            return False
 
         dlgBuild = getGuiItem("GuiBuildNovel")
         if dlgBuild is None:
@@ -1035,14 +1049,14 @@ class GuiMain(QMainWindow):
         qApp.processEvents()
         dlgBuild.viewCachedDoc()
 
-        return
+        return True
 
     def showProjectWordListDialog(self):
         """Open the project word list dialog.
         """
         if not self.hasProject:
             logger.error("No project open")
-            return
+            return False
 
         dlgWords = GuiWordList(self)
         dlgWords.exec_()
@@ -1051,14 +1065,14 @@ class GuiMain(QMainWindow):
             logger.debug("Reloading word list")
             self.docEditor.setDictionaries()
 
-        return
+        return True
 
     def showWritingStatsDialog(self):
         """Open the session stats dialog.
         """
         if not self.hasProject:
             logger.error("No project open")
-            return
+            return False
 
         dlgStats = getGuiItem("GuiWritingStats")
         if dlgStats is None:
@@ -1070,7 +1084,7 @@ class GuiMain(QMainWindow):
         qApp.processEvents()
         dlgStats.populateGUI()
 
-        return
+        return True
 
     def showAboutNWDialog(self, showNotes=False):
         """Show the about dialog for novelWriter.
@@ -1555,9 +1569,9 @@ class GuiMain(QMainWindow):
         """Single click on a project tree item just updates the details
         panel below the tree.
         """
-        sHandle = self.treeView.getSelectedHandle()
-        if sHandle is not None:
-            self.treeMeta.updateViewBox(sHandle)
+        tHandle = self.treeView.getSelectedHandle()
+        if tHandle is not None:
+            self.treeMeta.updateViewBox(tHandle)
         return
 
     @pyqtSlot("QTreeWidgetItem*", int)
@@ -1565,14 +1579,9 @@ class GuiMain(QMainWindow):
         """The user double-clicked an item in the tree. If it is a file,
         we open it. Otherwise, we do nothing.
         """
-        tHandle = tItem.data(self.treeView.C_NAME, Qt.UserRole)
-        logger.verbose("User double clicked tree item with handle '%s'", tHandle)
-
-        if self.theProject.projTree.checkType(tHandle, nwItemType.FILE):
+        tHandle = self.treeView.getSelectedHandle()
+        if tHandle is not None:
             self.openDocument(tHandle, changeFocus=False, doScroll=False)
-        else:
-            logger.verbose("Requested item '%s' is a folder", tHandle)
-
         return
 
     @pyqtSlot()
@@ -1589,18 +1598,11 @@ class GuiMain(QMainWindow):
         return
 
     @pyqtSlot()
-    def _treeKeyPressReturn(self):
-        """The user pressed return on an item in the tree. If it is a
-        file, we open it. Otherwise, we do nothing. Pressing return does
-        not change focus to the editor as double click does.
+    def _keyPressReturn(self):
+        """Forward the return/enter keypress to the function that opens
+        the currently selected item.
         """
-        tHandle = self.treeView.getSelectedHandle()
-        logger.verbose("User pressed return on tree item with handle '%s'", tHandle)
-        if self.theProject.projTree.checkType(tHandle, nwItemType.FILE):
-            self.openDocument(tHandle, changeFocus=False, doScroll=False)
-        else:
-            logger.verbose("Requested item '%s' is a folder", tHandle)
-
+        self.openSelectedItem()
         return
 
     @pyqtSlot()
@@ -1642,7 +1644,7 @@ class GuiMain(QMainWindow):
             logger.verbose("Novel tree tab activated")
             if self.hasProject:
                 self.novelView.refreshTree()
-                sHandle = self.novelView.getSelectedHandle()
+                sHandle, _ = self.novelView.getSelectedHandle()
 
         self.treeMeta.updateViewBox(sHandle)
 
diff --git a/tests/test_gui/test_gui_guimain.py b/tests/test_gui/test_gui_guimain.py
index 63cb8625..4db9ed7e 100644
--- a/tests/test_gui/test_gui_guimain.py
+++ b/tests/test_gui/test_gui_guimain.py
@@ -28,16 +28,104 @@ from tools import cmpFiles
 from PyQt5.QtCore import Qt
 from PyQt5.QtWidgets import QMessageBox, QDialog
 
-from novelwriter.dialogs.itemeditor import GuiItemEditor
-from novelwriter.gui.doceditor import GuiDocEditor
-from novelwriter.gui.projtree import GuiProjectTree
+from novelwriter.gui import (
+    GuiDocEditor, GuiProjectTree, GuiNovelTree, GuiOutline
+)
 from novelwriter.enum import nwItemType, nwWidget
+from novelwriter.dialogs.itemeditor import GuiItemEditor
 
 keyDelay = 2
 typeDelay = 1
 stepDelay = 20
 
 
+@pytest.mark.gui
+def testGuiMain_ProjectBlocker(monkeypatch, nwGUI):
+    """Test the blocking of features when there's no project open.
+    """
+    monkeypatch.setattr(QMessageBox, "question", lambda *a: QMessageBox.Yes)
+
+    # Test no-project blocking
+    assert nwGUI.closeProject() is True
+    assert nwGUI.saveProject() is False
+    assert nwGUI.closeDocument() is False
+    assert nwGUI.openDocument(None) is False
+    assert nwGUI.openNextDocument(None) is False
+    assert nwGUI.saveDocument() is False
+    assert nwGUI.viewDocument(None) is False
+    assert nwGUI.importDocument() is False
+    assert nwGUI.mergeDocuments() is False
+    assert nwGUI.splitDocument() is False
+    assert nwGUI.openSelectedItem() is False
+    assert nwGUI.editItem() is False
+    assert nwGUI.requestNovelTreeRefresh() is False
+    assert nwGUI.rebuildIndex() is False
+    assert nwGUI.rebuildOutline() is False
+    assert nwGUI.showProjectSettingsDialog() is False
+    assert nwGUI.showProjectDetailsDialog() is False
+    assert nwGUI.showBuildProjectDialog() is False
+    assert nwGUI.showProjectWordListDialog() is False
+    assert nwGUI.showWritingStatsDialog() is False
+
+# END Test testGuiMain_NoProject
+
+
+@pytest.mark.gui
+def testGuiMain_ProjectTreeItems(qtbot, monkeypatch, nwGUI, fncProj):
+    """Test handling of project tree items based on GUI focus states.
+    """
+    monkeypatch.setattr(QMessageBox, "question", lambda *a: QMessageBox.Yes)
+
+    nwGUI.theProject.projTree.setSeed(42)
+    assert nwGUI.newProject({"projPath": fncProj}) is True
+    assert nwGUI.saveProject() is True
+
+    sHandle = "0e17daca5f3e1"
+    assert nwGUI.openSelectedItem() is False
+
+    # Project Tree has focus
+    nwGUI.switchFocus(nwWidget.TREE)
+    nwGUI.projTabs.setCurrentIndex(0)
+    with monkeypatch.context() as mp:
+        mp.setattr(GuiProjectTree, "hasFocus", lambda *a: True)
+        assert nwGUI.docEditor.docHandle() is None
+        nwGUI.treeView._getTreeItem(sHandle).setSelected(True)
+        nwGUI._keyPressReturn()
+        assert nwGUI.docEditor.docHandle() == sHandle
+        assert nwGUI.closeDocument() is True
+
+    # Novel Tree has focus
+    nwGUI.projTabs.setCurrentIndex(1)
+    nwGUI.novelView.refreshTree(True)
+    with monkeypatch.context() as mp:
+        mp.setattr(GuiNovelTree, "hasFocus", lambda *a: True)
+        assert nwGUI.docEditor.docHandle() is None
+        actItem = nwGUI.novelView.topLevelItem(0)
+        chpItem = actItem.child(0)
+        selItem = chpItem.child(0)
+        nwGUI.novelView.setCurrentItem(selItem)
+        nwGUI._keyPressReturn()
+        assert nwGUI.docEditor.docHandle() == sHandle
+        assert nwGUI.closeDocument() is True
+
+    # Project Outline has focus
+    nwGUI.switchFocus(nwWidget.OUTLINE)
+    with monkeypatch.context() as mp:
+        mp.setattr(GuiOutline, "hasFocus", lambda *a: True)
+        assert nwGUI.docEditor.docHandle() is None
+        actItem = nwGUI.projView.topLevelItem(0)
+        chpItem = actItem.child(0)
+        selItem = chpItem.child(0)
+        nwGUI.projView.setCurrentItem(selItem)
+        nwGUI._keyPressReturn()
+        assert nwGUI.docEditor.docHandle() == sHandle
+        assert nwGUI.closeDocument() is True
+
+    # qtbot.stopForInteraction()
+
+# END Test testGuiMain_ProjectTreeItems
+
+
 @pytest.mark.gui
 def testGuiMain_Editing(qtbot, monkeypatch, nwGUI, fncProj, refDir, outDir):
     """Test the document editor.
diff --git a/tests/test_gui/test_gui_mainmenu.py b/tests/test_gui/test_gui_mainmenu.py
index daa564e1..15282fda 100644
--- a/tests/test_gui/test_gui_mainmenu.py
+++ b/tests/test_gui/test_gui_mainmenu.py
@@ -29,7 +29,7 @@ from PyQt5.QtWidgets import QAction, QFileDialog, QMessageBox
 from tools import writeFile
 
 from novelwriter.gui.doceditor import GuiDocEditor
-from novelwriter.enum import nwDocAction, nwDocInsert, nwWidget
+from novelwriter.enum import nwDocAction, nwDocInsert
 from novelwriter.constants import nwKeyWords, nwUnicode
 
 keyDelay = 2
@@ -471,11 +471,7 @@ def testGuiMenu_Insert(qtbot, monkeypatch, nwGUI, fncDir, fncProj):
     assert nwGUI.newProject({"projPath": fncProj})
 
     assert nwGUI.treeView._getTreeItem("0e17daca5f3e1") is not None
-
-    nwGUI.switchFocus(nwWidget.TREE)
-    nwGUI.treeView.clearSelection()
-    nwGUI.treeView._getTreeItem("0e17daca5f3e1").setSelected(True)
-    assert nwGUI.openSelectedItem()
+    assert nwGUI.openDocument("0e17daca5f3e1") is True
     nwGUI.docEditor.clear()
 
     # Test Faulty Inserts
diff --git a/tests/test_gui/test_gui_noveltree.py b/tests/test_gui/test_gui_noveltree.py
index 1135de87..fab2e0d2 100644
--- a/tests/test_gui/test_gui_noveltree.py
+++ b/tests/test_gui/test_gui_noveltree.py
@@ -74,7 +74,7 @@ def testGuiNovelTree_TreeItems(qtbot, monkeypatch, nwGUI, nwMinimal):
     assert not topItem.isSelected()
     topItem.setSelected(True)
     assert nwTree.selectedItems()[0] == topItem
-    assert nwTree.getSelectedHandle() == "a35baf2e93843"
+    assert nwTree.getSelectedHandle() == ("a35baf2e93843", 0)
 
     nwTree.refreshTree()
     assert nwTree.topLevelItem(0).isSelected()
diff --git a/tests/test_gui/test_gui_outline.py b/tests/test_gui/test_gui_outline.py
index 057f929f..972e7456 100644
--- a/tests/test_gui/test_gui_outline.py
+++ b/tests/test_gui/test_gui_outline.py
@@ -77,6 +77,10 @@ def testGuiOutline_Main(qtbot, monkeypatch, nwGUI, nwLipsum):
     selItem = chpItem.child(0)
 
     nwGUI.projView.setCurrentItem(selItem)
+    tHandle, tLine = nwGUI.projView.getSelectedHandle()
+    assert tHandle == "88243afbe5ed8"
+    assert tLine == 0
+
     assert nwGUI.projMeta.titleLabel.text() == "<b>Scene</b>"
     assert nwGUI.projMeta.titleValue.text() == "Scene One"
     assert nwGUI.projMeta.fileValue.text() == "Scene One"
@@ -87,6 +91,25 @@ def testGuiOutline_Main(qtbot, monkeypatch, nwGUI, nwLipsum):
     nwGUI.projMeta._tagClicked("#pov=Bod")
     assert nwGUI.docViewer.docHandle() == "4c4f28287af27"
 
+    # Scene One, Section Two
+    actItem = nwGUI.projView.topLevelItem(1)
+    chpItem = actItem.child(0)
+    scnItem = chpItem.child(0)
+    selItem = scnItem.child(0)
+
+    nwGUI.projView.setCurrentItem(selItem)
+    tHandle, tLine = nwGUI.projView.getSelectedHandle()
+    assert tHandle == "88243afbe5ed8"
+    assert tLine == 12
+
+    assert nwGUI.projMeta.titleLabel.text() == "<b>Section</b>"
+    assert nwGUI.projMeta.titleValue.text() == "Scene One, Section Two"
+    assert nwGUI.projMeta.fileValue.text() == "Scene One"
+    assert nwGUI.projMeta.itemValue.text() == "Finished"
+
+    nwGUI.projView._treeDoubleClick(selItem, 0)
+    assert nwGUI.docEditor.docHandle() == "88243afbe5ed8"
+
     # qtbot.stopForInteraction()
 
 # END Test testGuiOutline_Main