-
Notifications
You must be signed in to change notification settings - Fork 1.2k
clear execution count of all cells when user presses 'Clear All Output' #8318
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
Conversation
presses 'Clear All Output'
Codecov Report
@@ Coverage Diff @@
## master #8318 +/- ##
==========================================
- Coverage 59.33% 59.33% -0.01%
==========================================
Files 504 505 +1
Lines 23096 23147 +51
Branches 3737 3745 +8
==========================================
+ Hits 13704 13734 +30
- Misses 8500 8520 +20
- Partials 892 893 +1
Continue to review full report at Codecov.
|
@@ -401,6 +401,9 @@ export class MainStateController implements IMessageHandler { | |||
const newList = this.pendingState.cellVMs.map(cellVM => { | |||
return immutable.updateIn(cellVM, ['cell', 'data', 'outputs'], () => []); | |||
}); | |||
newList.forEach(cellVM => { | |||
cellVM.cell.data.execution_count = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't Don just add this? Also this would need to use the immutable functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah or make a copy of all of the cells. Something like so:
const newList = this.pendingState.cellVMs.map(cellVM => {
return {...celVM, cell: {...cellVM.cell, data: {...cellVM.cell.data, execution_count: null, outputs: []}}};
})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meaning you should set the execution count at the same time you clear the outputs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the review where don already added this:
https://github.com/microsoft/vscode-python/pull/8315/files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't Don just add this? Also this would need to use the immutable functions.
Yes & no. I started working on this (forgot to assign to myself). When I went to assign it, @DavidKutu had it.
Here is the review where don already added this:
https://github.com/microsoft/vscode-python/pull/8315/files
Oops, used the same branch of code.
Didn't mean to work on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry @DavidKutu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you review again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🕐
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🕐
oops |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops
@@ -401,6 +401,9 @@ export class MainStateController implements IMessageHandler { | |||
const newList = this.pendingState.cellVMs.map(cellVM => { | |||
return immutable.updateIn(cellVM, ['cell', 'data', 'outputs'], () => []); | |||
}); | |||
newList.forEach(cellVM => { | |||
cellVM.cell.data.execution_count = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't Don just add this? Also this would need to use the immutable functions.
Yes & no. I started working on this (forgot to assign to myself). When I went to assign it, @DavidKutu had it.
Here is the review where don already added this:
https://github.com/microsoft/vscode-python/pull/8315/files
Oops, used the same branch of code.
Didn't mean to work on it.
@@ -401,6 +401,9 @@ export class MainStateController implements IMessageHandler { | |||
const newList = this.pendingState.cellVMs.map(cellVM => { | |||
return immutable.updateIn(cellVM, ['cell', 'data', 'outputs'], () => []); | |||
}); | |||
newList.forEach(cellVM => { | |||
cellVM.cell.data.execution_count = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry @DavidKutu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For #7853
package-lock.json
has been regenerated by runningnpm install
(if dependencies have changed)