spyder-ide / spyder

Official repository for Spyder - The Scientific Python Development Environment
https://www.spyder-ide.org
MIT License
8.29k stars 1.61k forks source link

Feature request: keyboard shortcut for "Show in external file explorer" #13158

Closed ghost closed 4 years ago

ghost commented 4 years ago

Feature request: keyboard shortcut for "Show in external file explorer"

I use this feature quite often, and as a fan of keyboard shortcuts it would be very useful. I am using the "win + E" to open explorer all the time on windows, and I believe using something like "Ctrl + E" makes some amount of sense to open an explorer window to that file. I think it would also be totally fine to leave it with no pre-bound shortcut, and leave it to the user to decide to bind one.

I tried adding the shortcut myself, but I have not been successful in figuring out how to add new shortcuts, so I have no pull request to submit. I got so far as to get a listing in the settings menu, but nothing happened when the key combination was pressed. I did this by adding an entry to config.main.DEFAULTS:

DEFAULTS = [
    ...
    ("shortcuts", 
        {
         ...
         'editor/show in external file explorer': "Ctrl+E",

and plugins.editor.widgets.editor.EditorStack.create_shortcuts

 external_fileexp = CONF.config_shortcut(
     self.show_in_external_file_explorer,
     context="Editor",
     name="Show in external file explorer",
     parent=self)
dalthviz commented 4 years ago

Hi @athompson6735 thanks for the suggestion and trying to implement it! Indeed it could be nice to have a shortcut for that action. Maybe if you want to contribute we could guide you into how that could be done :) what do you guys think @spyder-ide/core-developers ?

ccordoba12 commented 4 years ago

I agree that it'd be really nice to have. Thanks @athompson6735 for offering your help with that!

Since you already found out how to add that shortcut, my only recommendation is for you to read our Contributing guide, so you understand how to create a successful pull request.

By the way, since this feature is not a major addition to Spyder, you can do it against our 4.x branch, so it's available in our next version (4.2.0 at this point, because 4.1.4 is about to be released).

jitseniesen commented 4 years ago

At least on my Linux system, Ctrl+E moves the cursor to the end of line. It's not a problem for me, I don't think I use that short cut, but if there are users that do use that short cut it might be annoying for them to change it.

CAM-Gerlach commented 4 years ago

I do use Win-E myself when I have to use Windows Explorer (I normally use MultiCommander as my file manager), but if a shortcut is added by default, IMO should be something a little less likely to conflict with others, e.g. adding additional modifier keys.

Although, per @jitseniesen 's concern, if the shorcut is set to belong to the File Explorer pane, wouldn't it only apply there and not override Ctrl-E other places?

ghost commented 4 years ago

@CAM-Gerlach my intent was actually for the context to be the editor. I tend to use native windows explorer rather than the built in file browser and I think those using this feature would likely be doing the same. Would it be acceptable to simply let the default be unbound? I certainly don't want to stomp on any other default or common shortcuts.

I have the shortcut working now, but I cannot figure out how to add the shortcut hint to the context menu. Can anyone point me in the right direction? EDIT solved this one. Untitled

I am also having the problem that after setting a binding and re-opening preferences, the shortcuts menu shows the binding as \. The shortcut still works, and continues to do so after re-starting spyder, but the preferences menu suggests it is unbound.

CAM-Gerlach commented 4 years ago

Ah, I see @athompson6735 — sorry for the misunderstanding. I actually do the same thing, just with MultiCommander instead of the stock Windows Explorer. I assume you're using something like openURL from QDesktopServices to do so in a cross-platform way that will respect the user's default file explorer preferences. I don't think macOS has a way to configure this and while you technically can on Windows, it doesn't work in 100% of contexts but I can verify that it does work here.

I have the shortcut working now, but I cannot figure out how to add the shortcut hint to the context menu.

Were you not able to get it working with self.register_shortcut(fileexplorer_action, context="Editor", name="Show in external file explorer") after creating your action fileexplorer_action, just like is done for gotoline_action right above it in spyder/plugins/editor/plugin.py?

bcolsen commented 4 years ago

I think that this sounds like a short cut that would be best left unset be default. At least it can't be ctrl-e. Going to the end of the line in the editor will likely be used more than this. I know there is already an 'end' key but it usually quite out of the way.

ghost commented 4 years ago

@CAM-Gerlach I figured out the shortcut hint. I had to modify the existing definition for the action in EditorStack.__init__ and add the shortcut and context keywords. I also found I had to register the callback with the plugin.

external_fileexp_action = create_action(self, text, #XXX here
                        triggered=self.show_in_external_file_explorer,
                        shortcut=CONF.get_shortcut(context="Editor",
                                name="show in external file explorer"),
                        context=Qt.WidgetShortcut)

parent.plugin.register_shortcut(external_fileexp_action, 
                                context="Editor",
                                name="show in external file explorer", 
                                add_shortcut_to_tip=True)

I'm still tracking down a number of other issues, and I'm running out of ideas. The two big problems I can find are solved by not attempting to set the default to nothing.

Here are the issues:

    • There has to be an entry for 'editor/show in external file explorer' in the DEFAULTS or spyder won't start with a configparser error.
      • A default value of '' in config.main.DEFAULTS will initialize the command to be un-set, but attempting to restore from default will throw an error.
      • Similarly if None is used, the command is un-set, but then if attempting to "restore from default" an AttributeError is thrown.
      • I can't find any other definitions that have no default binding, so I wonder if it's even possible to do so? It seems like "Ctrl+E" is no good for some people.

Untitled

  1. The entry in the settings menu always shows blank whether a shortcut is set or not. Setting a new shortcut will cause it to display in settings only until the window is closed. This is solved by setting the default to a valid key combination. Untitled

  2. Clearing a set shortcut does not take effect until after a restart.

ghost commented 4 years ago

oops, wrong button 😐

CAM-Gerlach commented 4 years ago

Wow, thanks for all your work and your thorough description of the issues you've faced. Seems like all of them trace back to having a default shortcut as unset.. .@ccordoba12 is that even possible with our current config architecture? If not, we could just set it to something suitably esoteric I guess.

oops, wrong button

Haha, happens to all of us ^_^

ghost commented 4 years ago

I'm looking at spyder\preferences\shortcuts.py ShortcutEditor.set_sequence_to_default and think adding a simple test to check if the default is an empty string should suffice. That makes the most sense to me so that all entries are strings. If this solves it, I would like to point out that I'm no good at using git, and would appreciate if someone more familiar with it could just do a bit of copy paste for me.

ghost commented 4 years ago

I think it's pretty much working. There is now a new capability to add default keybinds as an empty string to make them not set by default, as well as a new keybind for "Show in external file explorer". I wouldn't expect any of these edits to have side effects, but a second set of eyes always helps. I did notice that any keybind when cleared isn't actually cleared until after spyder is restarted. I didn't immediately find out how to fix that, and it doesn't bother me that much. Maybe another time...

Because I don't know how to use git.... here's the edits I made.

replace this with:

        external_fileexp_action = create_action(self, text, #XXX here
                                triggered=self.show_in_external_file_explorer,
                                shortcut=CONF.get_shortcut(context="Editor",
                                        name="show in external file explorer"),
                                context=Qt.WidgetShortcut)

        parent.plugin.register_shortcut(external_fileexp_action, 
                                        context="Editor",
                                        name="show in external file explorer", 
                                        add_shortcut_to_tip=True)

add this here

        external_fileexp = CONF.config_shortcut( #XXX here
            self.show_in_external_file_explorer,
            context="Editor",
            name="show in external file explorer",
            parent=self)

and be sure to add it to the function return here

                prevtab, nexttab, external_fileexp]

Now we add the capability for default unset shortcut with an empty string:

replace this with

        if sequence:
            self._qsequences = sequence.split(', ')
            self.update_warning()
        else:
            self.unbind_shortcut()

and finally add an entry to the default config around here

              'editor/show in external file explorer': '',
ccordoba12 commented 4 years ago

Wow, thanks for all your work and your thorough description of the issues you've faced. Seems like all of them trace back to having a default shortcut as unset.. .@ccordoba12 is that even possible with our current config architecture? If not, we could just set it to something suitably esoteric I guess.

There shouldn't be a problem with unset shortcuts, but I'm not really sure.

ghost commented 4 years ago

@ccordoba12 the problem with unset shortcuts is only when it's the default. Assuming an empty string represents a default of unset, ShortcutEditor.set_sequence_to_default will set self.new_qsequence = QKeySequence(''). Then when calling ShortcutEditor.check_conflicts, it will match every single other shortcut bound or not. This will then display a list of conflicts a mile long which breaks the gui element (dialog buttons are pushed off screen with no way to get at them). This could be solved by handling the case where self.new_qsequence.isEmpty() in check_conflicts or by handling an empty default in set_sequence_to_default . I chose the latter, which actually makes it work with any false-like default not just an empty string.

(all referring to "spyder/preferences/shortcuts.py")

ccordoba12 commented 4 years ago

I chose the latter, which actually makes it work with any false-like default not just an empty string.

Sounds good, Thanks for your help with this @athompson6735!

athompson673 commented 4 years ago

@ccordoba12 I ended up having to add a little bit to make the tests all pass for the PR. Hopefully this can get added to 4.2.0 :) also don't mind the duplicate account. IDK how that happened, but I need to figure out how to delete the extra one.