Skip to content
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: Move shortcuts to new API #13097

Merged
merged 1 commit into from
Aug 20, 2020
Merged

Conversation

goanpeca
Copy link
Member

@goanpeca goanpeca commented Jun 24, 2020

Description of Changes

  • Wrote at least one-line docstrings (for any new functions)
  • Added unit test(s) covering the changes (if testable)
  • Included a screenshot or animation (if affecting the UI, see Licecap)

Issue(s) Resolved

Fixes #13096
Fixes #10722

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct: @goanpeca

@goanpeca goanpeca added this to the Sprint June milestone Jun 24, 2020
@goanpeca goanpeca self-assigned this Jun 24, 2020
@goanpeca goanpeca force-pushed the enh/api-shortcuts branch 2 times, most recently from b113c76 to 312eba2 Compare June 24, 2020 22:55
@goanpeca goanpeca marked this pull request as ready for review June 24, 2020 23:29
@goanpeca
Copy link
Member Author

Hi @dalthviz, this one is ready for QA testing. Thanks!

@dalthviz
Copy link
Member

After an initial test I found the following:

  • Setting shortcuts for actions without a default shortcut seems like isn't working (for example, setting a shortcut for the toggle_automatic_import_action in the Help pane shows the new shortcut in the action label when opening the menu, but it doesn't change when the shortcut is used):
    shortcuts

  • Using for example Ctrl+C when a cell of the shortcuts table is selected causes an error report dialog to pop-up (seems like it happens when pressing any key):

Traceback (most recent call last):
  File "C:\Users\Daniel\Documents\Spyder\Spyder otros\gonzalo\spyder\spyder\plugins\shortcuts\widgets.py", line 813, in keyPressEvent
    if re.search(VALID_FINDER_CHARS, text) is not None:
NameError: name 're' is not defined

shortcuts2

@goanpeca
Copy link
Member Author

@dalthviz thanks. To double check. Are these issues related to this PR or issues present also in Master?

@dalthviz
Copy link
Member

@goanpeca checking with master:

  • The assign of the shortcuts to the actions doesn't happen in master too (then not caused by this PR, but since is master, maybe the fix could be in the scope of this PR?).
  • The error report dialog doesn't pop-up in master (from the traceback seems like a missing import of the re module in the PR?)

@goanpeca
Copy link
Member Author

Thanks for checking @dalthviz. I can make the fixes on this PR 👍

@goanpeca
Copy link
Member Author

goanpeca commented Jul 6, 2020

@dalthviz I fixed the issues you found. Thanks.

@dalthviz
Copy link
Member

dalthviz commented Jul 6, 2020

Checked and it seems like the things are working now except for the breakpoints plugin (changing for example the undock action shortcut doesn't trigger the action). For other plugins it works:

short

Also seems like some tests are failing, no idea why :/

@goanpeca
Copy link
Member Author

goanpeca commented Jul 6, 2020

Checked and it seems like the things are working now except for the breakpoints plugin (changing for example the undock action shortcut doesn't trigger the action). For other plugins it works:

Thanks for looking into it @dalthviz. The fact that those actions appear as shortcuts to be changed is a bug. Dock/Undock should not be configurable

@goanpeca
Copy link
Member Author

goanpeca commented Jul 6, 2020

@dalthviz fixed!

@dalthviz
Copy link
Member

dalthviz commented Jul 7, 2020

Checked again and now is working 👍 (although seems like the number of failing tests increased :/)

@goanpeca
Copy link
Member Author

goanpeca commented Jul 7, 2020

The pip test failures seem related to some new released package 🤷

The win failure is the usual for the Online Help server.

@goanpeca
Copy link
Member Author

goanpeca commented Jul 7, 2020

This is ready for review @ccordoba12

@goanpeca
Copy link
Member Author

This is ready for review @ccordoba12

@goanpeca
Copy link
Member Author

@dalthviz and @ccordoba12 this is ready for review. Failure is unrelated to the changes in this PR.

@goanpeca goanpeca force-pushed the enh/api-shortcuts branch 2 times, most recently from 5cffcd6 to 408c513 Compare August 13, 2020 20:43
@ccordoba12
Copy link
Member

This one has a conflict now @goanpeca.

@goanpeca
Copy link
Member Author

Fixed.

self.variableexplorer,
self.onlinehelp,
self.explorer,
self.findinfiles] + self.thirdparty_plugins:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remaining PRs will keep generating conflicts. So I made the list like this to avoid that.

Also, this list will be gone once all is migrated since Plugins will be loaded from entry points instead of hardcoded.

@ccordoba12
Copy link
Member

/show binder

@github-actions
Copy link

Binder 👈 Launch a Binder instance on this branch

@ccordoba12
Copy link
Member

Some issues I found while testing this:

  • Assigning new shortcuts to focus/show a plugin without a previous shortcut is not working. For instance, assigning Ctrl+Shift+N to the Profiler doesn't show it nor gives it focus.

  • There are entries that I have no idea to what they correspond in a plugin. For instance, where is the stop_action in Help?

    Selección_023

  • There are plugins whose actions must not be shown. For instance the Internal Console:

    Selección_022

I understand these issues are not caused by this PR, by since we found them while testing it (and some are bugs anyway), I'd like to see them fixed here.

@goanpeca
Copy link
Member Author

goanpeca commented Aug 14, 2020

Assigning new shortcuts to focus/show a plugin without a previous shortcut is not working. For instance, assigning Ctrl+Shift+N to the Profiler doesn't show it nor gives it focus.

Will fix.

There are entries that I have no idea to what they correspond in a plugin. For instance, where is the stop_action in Help?

I will disable registering a shortcut by default, so that it becomes an opt-in feature (if the user wants the shortcut to be available as a configurable shortcut when creating the action.

I understand these issues are not caused by this PR, by since we found them while testing it (and some are bugs anyway), I'd like to see them fixed here.

Stop action refers to the browser that lives in the Help plguin

This is the list of currently all shortcuts listed in master, @ccordoba12 please tick the ones you wish to be removed.

- [ ] _ / close pane - [ ] _ / configure - [ ] _ / debug - [ ] _ / debug continue - [ ] _ / debug exit - [ ] _ / debug step into - [ ] _ / debug step over - [ ] _ / debug step return - [ ] _ / file switcher - [ ] _ / fullscreen mode - [ ] _ / layout preferences - [ ] _ / lock unlock panes - [ ] _ / maximize pane - [ ] _ / preferences - [ ] _ / quit - [ ] _ / re-run last script - [ ] _ / restart - [ ] _ / run - [ ] _ / save current layout - [ ] _ / show toolbars - [ ] _ / spyder documentation - [ ] _ / switch to ate - [ ] _ / switch to breakpoints - [ ] _ / switch to editor - [ ] _ / switch to explorer - [ ] _ / switch to find_in_files - [ ] _ / switch to help - [ ] _ / switch to historylog - [ ] _ / switch to internal_console - [ ] _ / switch to ipython_console - [ ] _ / switch to onlinehelp - [ ] _ / switch to outline_explorer - [ ] _ / switch to plots - [ ] _ / switch to profiler - [ ] _ / switch to project_explorer - [ ] _ / switch to variable_explorer - [ ] _ / symbol finder - [ ] _ / use next layout - [ ] _ / use previous layout - [ ] array_builder / enter array inline - [ ] array_builder / enter array table - [ ] ate / rune - [ ] breakpoints / clear_all_breakpoints_action - [ ] breakpoints / clear_breakpoint_action - [ ] breakpoints / edit_breakpoint_action - [ ] breakpoints / list_breakpoints_action - [ ] editor / blockcomment - [ ] editor / breakpoint - [ ] editor / close all - [ ] editor / close file 1 - [ ] editor / close file 2 - [ ] editor / close split panel - [ ] editor / code completion - [ ] editor / conditional breakpoint - [ ] editor / copy - [ ] editor / cut - [ ] editor / cycle to next file - [ ] editor / cycle to previous file - [ ] editor / debug cell - [ ] editor / delete - [ ] editor / delete line - [ ] editor / docstring - [ ] editor / duplicate line down - [ ] editor / duplicate line up - [ ] editor / end of document - [ ] editor / end of line - [ ] editor / go to definition - [ ] editor / go to line - [ ] editor / go to new line - [ ] editor / go to next cell - [ ] editor / go to next file - [ ] editor / go to previous cell - [ ] editor / go to previous file - [ ] editor / indent - [ ] editor / inspect current object - [ ] editor / kill next word - [ ] editor / kill previous word - [ ] editor / kill to line end - [ ] editor / kill to line start - [ ] editor / last edit location - [ ] editor / move line down - [ ] editor / move line up - [ ] editor / new file - [ ] editor / next char - [ ] editor / next cursor position - [ ] editor / next line - [ ] editor / next warning - [ ] editor / next word - [ ] editor / open file - [ ] editor / open last closed - [ ] editor / paste - [ ] editor / previous char - [ ] editor / previous cursor position - [ ] editor / previous line - [ ] editor / previous warning - [ ] editor / previous word - [ ] editor / re-run last cell - [ ] editor / redo - [ ] editor / rotate kill ring - [ ] editor / run cell - [ ] editor / run cell and advance - [ ] editor / run selection - [ ] editor / save all - [ ] editor / save as - [ ] editor / save file - [ ] editor / select all - [ ] editor / show in external file explorer - [ ] editor / split horizontally - [ ] editor / split vertically - [ ] editor / start of document - [ ] editor / start of line - [ ] editor / toggle comment - [ ] editor / transform to lowercase - [ ] editor / transform to uppercase - [ ] editor / unblockcomment - [ ] editor / undo - [ ] editor / unindent - [ ] editor / yank - [ ] editor / zoom in 1 - [ ] editor / zoom in 2 - [ ] editor / zoom out - [ ] editor / zoom reset - [ ] explorer / copy absolute path - [ ] explorer / copy file - [ ] explorer / copy relative path - [ ] explorer / paste file - [ ] find_in_files / max_results_action - [ ] find_in_files / toggle_case_action - [ ] find_in_files / toggle_exclude_case_action - [ ] find_in_files / toggle_more_options_action - [ ] find_in_files / toggle_use_regex_on_search_action - [ ] find_in_files / togle_use_regex_on_exlude_action - [ ] find_replace / find next - [ ] find_replace / find previous - [ ] find_replace / find text - [ ] find_replace / hide find and replace - [ ] find_replace / replace text - [ ] help / back_action - [ ] help / copy_action - [ ] help / forward_action - [ ] help / inspect_action - [ ] help / refresh_action - [ ] help / select_all_action - [ ] help / stop_action - [ ] help / toggle_automatic_import_action - [ ] help / toggle_locked_action - [ ] help / toggle_plain_mode_action - [ ] help / toggle_rich_mode_action - [ ] help / toggle_show_source_action - [ ] help / toggle_wrap_action - [ ] help / zoom_in_action - [ ] help / zoom_out_action - [ ] historylog / maximum_history_entries_action - [ ] historylog / toggle_line_numbers_action - [ ] historylog / toggle_wrap_action - [ ] internal_console / environment_action - [ ] internal_console / external_editor_action - [ ] internal_console / max_line_count_action - [ ] internal_console / quit_action - [ ] internal_console / run_action - [ ] internal_console / sys_path_action - [ ] internal_console / toggle_code_completion_action - [ ] internal_console / toggle_wrap_action - [ ] ipython_console / new tab - [ ] ipython_console / reset namespace - [ ] ipython_console / restart kernel - [ ] onlinehelp / back_action - [ ] onlinehelp / copy_action - [ ] onlinehelp / find_action - [ ] onlinehelp / forward_action - [ ] onlinehelp / home_action - [ ] onlinehelp / inspect_action - [ ] onlinehelp / refresh_action - [ ] onlinehelp / select_all_action - [ ] onlinehelp / stop_action - [ ] onlinehelp / zoom_in_action - [ ] onlinehelp / zoom_out_action - [ ] plots / close - [ ] plots / close_all - [ ] plots / copy - [ ] plots / next_figure - [ ] plots / previous_figure - [ ] plots / save - [ ] plots / save_all - [ ] plots / toggle_auto_fit_plotting_action - [ ] plots / toggle_mute_inline_plotting_action - [ ] plots / toggle_show_plot_outline_action - [ ] plots / zoom_in - [ ] plots / zoom_out - [ ] profiler / browse_action - [ ] profiler / clear_action - [ ] profiler / collapse_action - [ ] profiler / expand_action - [ ] profiler / load_data_action - [ ] profiler / profile_current_filename_action - [ ] profiler / run_action - [ ] profiler / save_data_action - [ ] profiler / show_output_action - [ ] pylint / run analysis - [ ] shortcuts / shortcuts_summary_action - [ ] variable_explorer / copy - [ ] variable_explorer / refresh - [ ] variable_explorer / search - [ ] workingdir / browse_action - [ ] workingdir / next_action - [ ] workingdir / parent_action - [ ] workingdir / previous_action

@goanpeca goanpeca force-pushed the enh/api-shortcuts branch 5 times, most recently from 151b96a to 4aa96ca Compare August 14, 2020 15:46
@goanpeca
Copy link
Member Author

Assigning new shortcuts to focus/show a plugin without a previous shortcut is not working.

Fixed.

There are plugins whose actions must not be shown. For instance the Internal Console:

See the previous comment.

@ccordoba12
Copy link
Member

This is the list of currently all shortcuts listed in master, @ccordoba12 please tick the ones you wish to be removed.

This is something that I can't decide by myself. For now, please remove shortcuts for all actions that don't have one in 4.x.

Later we'll go pane by pane with the rest of the team and decide which actions really deserve a shortcut.

@goanpeca goanpeca force-pushed the enh/api-shortcuts branch 3 times, most recently from 79fb70b to 7503d81 Compare August 20, 2020 01:17
@goanpeca
Copy link
Member Author

goanpeca commented Aug 20, 2020

Thanks for the feedback @ccordoba12.

Added all shortcuts from 4.x except:

  • console clear line
  • console clear shell
  • console inspect current object

These ones will be added in the IPython Plugin PR. Tracking on #13527

@goanpeca goanpeca force-pushed the enh/api-shortcuts branch 2 times, most recently from dce1bda to 43e1580 Compare August 20, 2020 01:33
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @goanpeca for your latest update. Everything is working fine for me now.

I left one tiny comment, then this should be ready.

spyder/app/mainwindow.py Outdated Show resolved Hide resolved
@goanpeca
Copy link
Member Author

goanpeca commented Aug 20, 2020

Thanks for the feedback @ccordoba12.

Made the requested fix.

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @goanpeca!

@ccordoba12 ccordoba12 merged commit 3bf9253 into spyder-ide:master Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants