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: Use menu and item identifiers to add items to the main menu plugin #16205

Merged
merged 6 commits into from
Aug 18, 2021

Conversation

andfoy
Copy link
Member

@andfoy andfoy commented Aug 11, 2021

Description of Changes

This PR updates the Main Menu plugin methods to exclusively use string identifiers as opposed to using object references when adding items to a specific menu and/or before a given menu item. This is a breaking API change.

  • 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 #

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: @andfoy

@andfoy andfoy added this to the v5.2.0 milestone Aug 11, 2021
@andfoy andfoy self-assigned this Aug 11, 2021
@andfoy andfoy changed the title PR: Main menu: Use menu and item identifiers to add actions and menus PR: Use menu and item identifiers to add items to the main menu plugin Aug 11, 2021
@andfoy andfoy changed the base branch from master to 5.x August 11, 2021 21:30
@pep8speaks
Copy link

pep8speaks commented Aug 11, 2021

Hello @andfoy! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 72:1: E402 module level import not at top of file

Comment last updated at 2021-08-17 21:46:48 UTC

@andfoy andfoy force-pushed the use_identifiers_menus branch from 8248e15 to 78968de Compare August 12, 2021 21:27
@andfoy andfoy marked this pull request as ready for review August 13, 2021 01:23
@andfoy andfoy force-pushed the use_identifiers_menus branch from 09f1b45 to 6b89a8d Compare August 13, 2021 21:43
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.

This looks pretty good, thanks @andfoy! Besides the small suggestions I left for you, I also think you need to change the places where we use before to move from actions to id's (like you did for menus).

A couple of examples are:

if shortcuts:
from spyder.plugins.shortcuts.plugin import ShortcutActions
shortcuts_summary_action = shortcuts.get_action(
ShortcutActions.ShortcutSummaryAction)
for documentation_action in [
self.documentation_action, self.video_action]:
mainmenu.add_item_to_application_menu(
documentation_action,
menu_id=ApplicationMenus.Help,
section=HelpMenuSections.Documentation,
before=shortcuts_summary_action,
before_section=HelpMenuSections.Support)

and

docs_action = self.get_action(
ApplicationActions.SpyderDocumentationAction,
plugin=Plugins.Application)
mainmenu.add_item_to_application_menu(
self.get_container().tour_action,
menu_id=ApplicationMenus.Help,
section=HelpMenuSections.Documentation,
before=docs_action)

spyder/api/widgets/menus.py Outdated Show resolved Hide resolved
spyder/api/widgets/main_widget.py Outdated Show resolved Hide resolved
spyder/app/mainwindow.py Outdated Show resolved Hide resolved
spyder/plugins/mainmenu/plugin.py Outdated Show resolved Hide resolved
spyder/plugins/mainmenu/plugin.py Show resolved Hide resolved
spyder/plugins/toolbar/container.py Show resolved Hide resolved
spyder/plugins/toolbar/container.py Show resolved Hide resolved
@ccordoba12 ccordoba12 modified the milestones: v5.2.0, v5.1.2 Aug 15, 2021
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.

@andfoy, I did a manual check and some entries are missing:

Debug menu: List breakpoints is missing:

This PR:

imagen

5.1.1:

imagen


Help menu: The first section is missing

This PR:

imagen

5.1.1:

imagen

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 @andfoy! Looks good to me now.

@ccordoba12 ccordoba12 merged commit 785a8b7 into spyder-ide:5.x Aug 18, 2021
ccordoba12 added a commit that referenced this pull request Aug 18, 2021
@andfoy andfoy deleted the use_identifiers_menus branch August 18, 2021 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants