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.

Sign up for GitHub

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

Jump to bottom

Add "Go to Last Failed Cell" Button #154443

Merged
merged 8 commits into from
Jul 15, 2022
Merged

Add "Go to Last Failed Cell" Button #154443

merged 8 commits into from
Jul 15, 2022

Conversation

andreamah
Copy link
Contributor

@andreamah andreamah commented Jul 8, 2022 •
edited
Loading

Adds a button that says "See Last Failure" on the toolbar if the last ran cell failed. This focuses on the failed cell.

Recording 2022-07-11 at 16 02 11

Also available as a command Notebook: Go to Last Failed Cell.
EDIT: it now says Go to Most Recently Failed Cell to reduce ambiguity.
image

This PR fixes #142898

@andreamah andreamah self-assigned this Jul 8, 2022
@andreamah andreamah changed the title Add go to last failed cell function Add "Go to Last Failed Cell" Button Jul 8, 2022
@andreamah andreamah requested a review from roblourens July 8, 2022 13:11
@andreamah andreamah marked this pull request as ready for review July 8, 2022 13:11
@vscodenpa vscodenpa added this to the July 2022 milestone Jul 8, 2022
@andreamah
Copy link
Contributor Author

andreamah commented Jul 11, 2022 •
edited
Loading

I moved the button so it now has the same order (order: 0) as the Go To Running Cell button. Now, this new button just replaces the Go To Running Cell button when cells finish executing with an error. You can see the updated screenshot in the first post.

When both are shortened to Go To, it sort of seems like only the button's icon changes when it's actually a new button. Not too sure if that's fine?

@andreamah andreamah requested a review from roblourens July 11, 2022 23:09
@roblourens
Copy link
Member

it sort of seems like only the button's icon changes when it's actually a new button. Not too sure if that's fine?

I think that's exactly what I want

roblourens
roblourens reviewed Jul 13, 2022
View reviewed changes
Copy link
Member

@roblourens roblourens left a comment

Choose a reason for hiding this comment

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

I shouldn't see this when an execution is running, otherwise I could see both buttons. Maybe that means clearing the last failed cell in the service when an execution starts, rather than when it's completed. Or by changing the context keys.

@andreamah andreamah requested a review from roblourens July 13, 2022 20:55
roblourens
roblourens approved these changes Jul 13, 2022
View reviewed changes
Copy link
Member

@roblourens roblourens left a comment

Choose a reason for hiding this comment

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

Could you join the notebook standup call to show this off tomorrow?

@andreamah
Copy link
Contributor Author

Could you join the notebook standup call to show this off tomorrow?

sure! should I merge first?

@roblourens
Copy link
Member

Go ahead and merge whenever you're ready

@andreamah andreamah merged commit 6779fd9 into main Jul 15, 2022
@andreamah andreamah deleted the andreamah/issue142898 branch July 15, 2022 16:52
jrieken pushed a commit that referenced this pull request Jul 18, 2022
@andreamah @jrieken
Add "Go to Last Failed Cell" Button (#154443)
c7b7d2b 
* Add go to last failed cell function
@github-actions github-actions bot locked and limited conversation to collaborators Aug 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@roblourens roblourens roblourens approved these changes

Assignees

@andreamah andreamah

Labels
None yet
Projects
None yet
Milestone
July 2022
Development

Successfully merging this pull request may close these issues.

Go to Running Cell is great... but... could be improved to "Go to Last Run Cell".
3 participants
@andreamah @roblourens @vscodenpa