-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PR: Add Ctrl+Shift+T shortcut to reopen the last closed Editor tab #3620
Conversation
@mariacamilaremolinagutierrez, did you verify that @thewhitetulip's additions are working correctly? |
@ccordoba12 I'd like to know what I did wrong back then :-) |
Once Maria Camila finishes here, you'll know :-) |
So @thewhitetulip you were using the recent_files list, so you were re-opening the wrong files (that were not recently closed). Also when a file is closed it must be added to the list you will use to open from. I just created a new list that works exactly as in a browser. It has the same size as the recent_files one. If you close spyder and open it again you can still re open last closed tabs. Which I think might me a good thing to have. |
@@ -1191,6 +1198,18 @@ def close_all_but_this(self): | |||
self.close_all_right() | |||
for i in range(0, self.get_stack_count()-1 ): | |||
self.close_file(0) | |||
|
|||
def add_last_closed_file(self, fname): | |||
"""Add to last closed file list""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docstrings are sentences so a .
is needed at the end :-)
"""Add to last closed file list."""
|
||
def add_last_closed_file(self, fname): | ||
"""Add to last closed file list""" | ||
if fname is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this if really needed?
@mariacamilaremolinagutierrez, please finish this one before Friday :-) |
@@ -631,6 +632,15 @@ def get_plugin_actions(self): | |||
self.register_shortcut(self.new_action, context="Editor", | |||
name="New file", add_sc_to_tip=True) | |||
|
|||
add_shortcut_to_tooltip(self.new_action, context="Editor", | |||
name="New file") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this couple of lines, and the import of add_shortcut_to_tooltip
. They are not needed anymore.
add_shortcut_to_tooltip(self.new_action, context="Editor", | ||
name="New file") | ||
|
||
self.open_last_closed = create_action(self, _("O&pen last closed"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please call this self.open_last_closed_action
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And that's because you named the method associated with this action exactly as the action, so we need to disambiguate them :-)
@@ -976,7 +986,7 @@ def get_plugin_actions(self): | |||
self.recent_file_menu.aboutToShow.connect(self.update_recent_file_menu) | |||
|
|||
file_menu_actions = [self.new_action, | |||
None, | |||
self.open_last_closed, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't remove None
here and move self.open_last_closed
after self.open_action
.
We use None
to create separators in our menus :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# spyder.utils.qt_helpers.py
class MenuSeparator:
pass
# some_file.py
from spyder.utils.qt_helpers import MenuSeparator
file_menu_actions = [self.new_action,
MenuSeparator,
self.open_last_closed]
I think we could add something like MenuSeparator
so that it is clear what None
really does... (for another PR)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some minor comments, but this is almost ready. Good work @mariacamilaremolinagutierrez! |
Looking good, thanks @mariacamilaremolinagutierrez! |
Fixes #2415
I rebased @thewhitetulip 's branch. He did the changes for this new feature.