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: Add debug cell icon to the debug toolbar #18877

Merged
merged 1 commit into from
Aug 1, 2022

Conversation

stevetracvc
Copy link
Contributor

I recreated this after the CRLF PR

Description of Changes

I added the "Debug Cell" icon to the toolbar.

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

Before
debug

After
debug2

I find it extremely annoying that all of the major Debug tools are in the toolbar except for the Debug Cell tool. And sometimes, when trying to click on it from the Debug menu, I accidentally hit the "Run File" toolbar icon instead. This PR adds the icon, keeping the same ordering that is used in the Debug menu.

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: stevetracvc

@stevetracvc stevetracvc changed the title added debug cell icon to the debug toolbar PR: add debug cell icon to the debug toolbar Jul 30, 2022
@ccordoba12 ccordoba12 changed the title PR: add debug cell icon to the debug toolbar PR: Add debug cell icon to the debug toolbar Jul 30, 2022
@ccordoba12 ccordoba12 changed the base branch from master to 5.x July 30, 2022 23:05
@ccordoba12 ccordoba12 changed the base branch from 5.x to master July 30, 2022 23:05
@impact27
Copy link
Contributor

looks good to me

Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

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

Thanks @stevetracvc ! This LGTM 👍

However, just in case, could be worthy to do this over 5.x? What do you think @impact27 @ccordoba12 ?

If not, then I think we should proceed with this and merge 👍🏼

@impact27
Copy link
Contributor

impact27 commented Aug 1, 2022

5.x sounds good to me

@impact27 impact27 changed the base branch from master to 5.x August 1, 2022 15:44
@impact27 impact27 changed the base branch from 5.x to master August 1, 2022 15:45
@stevetracvc
Copy link
Contributor Author

@impact27 looks like @ccordoba12 did that too, switched back and forth immediately. Is there something I need to do to make it work with 5.x instead?

@dalthviz
Copy link
Member

dalthviz commented Aug 1, 2022

Yep @stevetracvc to move to 5.x this will need a rebase. For that I think the command is something like git rebase --onto 5.x master feat/debug-cell-toolbar (your 5.x branch should be up to date before doing this).

Also, sorry for all the hassle in order to land a one line change 😅. Hopefully after the rebase this could be merged without problem 🤞🏼

@dalthviz dalthviz removed this from the v6.0alpha1 milestone Aug 1, 2022
@stevetracvc
Copy link
Contributor Author

hmmmmm I think the issue is the CRLF stuff again 🤣 hang on

@dalthviz
Copy link
Member

dalthviz commented Aug 1, 2022

Could it be better to just recreate this PR again but this time using as base branch 5.x for your feature branch locally @stevetracvc ?

@ccordoba12
Copy link
Member

ccordoba12 commented Aug 1, 2022

I'm not sure about adding this button to 5.x because it'd change the default interface and so it'd make harder for users to follow our docs.

So I'd say we should leave it for master (i.e. the future Spyder 6). That's something we agreed on with @impact27 already.

@dalthviz
Copy link
Member

dalthviz commented Aug 1, 2022

Then this LGTM 👍 Thanks @stevetracvc !

@dalthviz dalthviz added this to the v6.0alpha1 milestone Aug 1, 2022
@dalthviz dalthviz merged commit c5b52dd into spyder-ide:master Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants