Allowed to switch with the last view with Ctrl+Tab #651
Conversation
|
Hi Evandro, Any ideas about the current build failure? Looks like it's pulling in an older PyQt instead of grabbing the latest on PyPI. Regarding this PR, why not just use ctrl+1/2/3? That way you always know which tab will appear |
When pressing Also, is it much simpler to keep using Meaning,
It is not instantly pyqt 5.15 because we have forced pyqt 5.13.2 on: If the checks are passing on your computer, it means you probably have the old pyqt stubs laying around somewhere. Try running I checkout on the new master and run I managed to reduce the errors by installing the package python -m mypy aqt
aqt\qt.py:16: error: Skipping analyzing 'PyQt5.QtWebEngineWidgets': found module but no type hints
or library stubs [import]
from PyQt5.QtWebEngineWidgets import *
^
aqt\pinnedmodules.py:23: error: Skipping analyzing 'PyQt5.QtSvg': found module but no type hints
or library stubs [import]
import PyQt5.QtSvg
^
aqt\pinnedmodules.py:23: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
aqt\tagedit.py:13: error: Incompatible types in assignment (expression has type
"Union[QCompleter, TagCompleter]", base class "QLineEdit" defined the type as
"Callable[[QLineEdit], QCompleter]") [assignment]
completer: Union[QCompleter, TagCompleter]
^
aqt\main.py:101: error: Unsupported operand types for & ("KeyboardModifiers" and
"KeyboardModifier") [operator]
self.safeMode = self.app.queryKeyboardModifiers() & Qt.ShiftModifier
^
aqt\main.py:964: error: Incompatible types in assignment (expression has type "List[QShortcut]",
variable has type "Sequence[Tuple[str, Callable[..., Any]]]") [assignment]
self.stateShortcuts = self.applyShortcuts(shortcuts)
^
aqt\main.py:968: error: Argument 1 to "delete" has incompatible type
"Tuple[str, Callable[..., Any]]"; expected "simplewrapper" [arg-type]
sip.delete(qs)
^
aqt\webview.py:24: error: Name 'QWebEnginePage' is not defined [name-defined]
class AnkiWebPage(QWebEnginePage):
^
aqt\webview.py:186: error: Name 'QWebEngineView' is not defined [name-defined]
class AnkiWebView(QWebEngineView):
^
aqt\webview.py:190: error: Name 'QWebEngineView' is not defined [name-defined]
QWebEngineView.__init__(self, parent=parent)
^
aqt\webview.py:203: error: Name 'QWebEngineProfile' is not defined [name-defined]
self._page.profile().setHttpCacheType(QWebEngineProfile.NoCache)
^
aqt\webview.py:235: error: "QEvent" has no attribute "button" [attr-defined]
if evt.button() == Qt.MidButton and isLin:
^
aqt\deckchooser.py:21: error: "Callable[[], QWidget]" has no attribute "setLayout" [attr-defined]
self.widget.setLayout(self)
^
aqt\browser.py:1052: error: "SidebarTreeView" has no attribute "mw" [attr-defined]
self.sidebarTree.mw = self.mw
^
aqt\studydeck.py:80: error: "QEvent" has no attribute "key" [attr-defined]
if evt.key() == Qt.Key_Up:
^
aqt\studydeck.py:87: error: "QEvent" has no attribute "key" [attr-defined]
elif evt.key() == Qt.Key_Down:
^
Found 15 errors in 8 files (checked 96 source files)
make[1]: *** [Makefile:107: .build/mypy] Error 1
make[1]: Leaving directory '/cygdrive/f/anki/qt'
make: *** [Makefile:154: check] Error 2
f:\anki> |
|
Thanks Evandro - I should have looked into it further before assuming it was a caching issue. I'd forgotten the Qt version was pinned! I've pushed some other tweaks that should mean we don't need the separate -stubs package. |
| """ | ||
| if self.current_editor_index == 0: | ||
| if self.last_editor_index == 0: | ||
| self.tform.back_button.setChecked(True) |
There was a problem hiding this comment.
Is this complexity necessary? Won't current_editor_index == last_editor_index only be the case before the user has switched between any tabs?
There was a problem hiding this comment.
No, if the user pressed Ctrl+1 or clicked in the first tag twice in a row, the last index will be equal to the current index. As we are hardcoding this small "memory", it is simple to just list all cases because if someday we actually replace it by a full MRU algorithm, I hope it should be simpler to migrate only by changing this method.
There was a problem hiding this comment.
Couldn't that be addressed with less code by not changing last_editor_index when the user clicks on the same tab again?
There was a problem hiding this comment.
I would not call these lines code, but a combination matrix. If we added that if on last_editor_index, we could remove one inner combination from each outter combination, but if somehow our last_editor_index if fail someday or we start changing the last_editor_index somewhere else and forget to add an if last_editor_index the code would break. As it is now, with the full matrix combination set, the code is more robust and less prone to fail (less fragile). Keeping the codebase more robust/prepared to failures should be better than removing a few lines and by adding an if somewhere else.
There was a problem hiding this comment.
IMHO listing out all the possibilities as you've done is more prone to accidental mistakes, and it makes it harder to maintain the code moving forward. I'm also still a bit skeptical as to the demand for this feature - I think the best option is to leave this PR open for a while and see if other people want to add their 2c.
There was a problem hiding this comment.
Closing for now; let's see if demand presents itself in the future
9c6413e to
b0ce4b0
Compare
acf39b6 to
f3afaa1
Compare
2191a24 to
ac1755e
Compare
For a full implementation example, see: https://github.com/spyder-ide/spyder/pull 4302
ac1755e to
124d708
Compare
I did not implement a full MRU (Most recently used) thing because it would be more complex and we just have 3 views to switch so just selecting the next on this hardcoded MRU should be more than enough.