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

Add "Debug Cell" #7059

Merged
merged 7 commits into from
Aug 11, 2021
Merged

Add "Debug Cell" #7059

merged 7 commits into from
Aug 11, 2021

Conversation

roblourens
Copy link
Member

@roblourens roblourens commented Aug 11, 2021

For microsoft/vscode#130524

And #1652

  • Add a "Debug Cell" command
  • Remove the "Debug" button from the toolbar

A little flakier than RBL, I keep having this issue where I send the 'disconnect' request but the Debug Session does not actually terminate, seen that before? Seems to work fine for RBL.

@roblourens roblourens requested a review from a team as a code owner August 11, 2021 00:40
@roblourens roblourens requested a review from DavidKutu August 11, 2021 00:41
@roblourens roblourens changed the title roblou/debugCellCommand Add "Debug Cell" Aug 11, 2021
@codecov-commenter
Copy link

codecov-commenter commented Aug 11, 2021

Codecov Report

Merging #7059 (af5cea2) into main (8e06fe9) will decrease coverage by 1%.
The diff coverage is 10%.

❗ Current head af5cea2 differs from pull request most recent head 9cb9e63. Consider uploading reports for the commit 9cb9e63 to get more accurate results

@@          Coverage Diff           @@
##            main   #7059    +/-   ##
======================================
- Coverage     64%     63%    -2%     
======================================
  Files        362     362            
  Lines      22873   22891    +18     
  Branches    3420    3421     +1     
======================================
- Hits       14854   14438   -416     
- Misses      6726    7242   +516     
+ Partials    1293    1211    -82     
Impacted Files Coverage Δ
src/client/debugger/types.ts 100% <ø> (ø)
src/client/debugger/jupyter/debuggingManager.ts 27% <5%> (-6%) ⬇️
src/client/debugger/jupyter/kernelDebugAdapter.ts 6% <10%> (+1%) ⬆️
src/client/datascience/constants.ts 99% <100%> (+<1%) ⬆️
...ience/variablesView/variableViewMessageListener.ts 22% <0%> (-78%) ⬇️
...ent/common/application/webviewViews/webviewView.ts 14% <0%> (-72%) ⬇️
...c/client/datascience/variablesView/variableView.ts 17% <0%> (-61%) ⬇️
src/client/datascience/webviews/webviewViewHost.ts 21% <0%> (-58%) ⬇️
src/client/common/application/webviews/webview.ts 13% <0%> (-54%) ⬇️
src/client/datascience/themeFinder.ts 13% <0%> (-48%) ⬇️
... and 29 more


this.runByLineThreadId = content.body.threadId;
}
this.runByLineThreadId = content.body.threadId;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this variable be renamed too (as it's also not just for run by line)?

@DavidKutu
Copy link

The Lint test is failing because you need to run prettier on:
src/client/debugger/jupyter/debuggingManager.ts
src/client/debugger/jupyter/kernelDebugAdapter.ts

internalConsoleOptions: 'neverOpen',
justMyCode: true,
// add the doc uri to the config
__document: doc.uri.toString(),
Copy link
Member Author

Choose a reason for hiding this comment

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

@DavidKutu is this used anywhere? I couldn't actually find a usage

Choose a reason for hiding this comment

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

Yeah, I guess I removed it. We send the NotebookDocument as a parameter anyway. I'll remove it on my next PR.

@roblourens
Copy link
Member Author

I keep having this issue where I send the 'disconnect' request but the Debug Session does not actually terminate, seen that before? Seems to work fine for RBL.

@DavidKutu any idea about that?

@roblourens roblourens force-pushed the roblou/debugCellCommand branch from 9cb9e63 to d6a0713 Compare August 11, 2021 22:30
@roblourens roblourens merged commit 4721aaa into main Aug 11, 2021
@roblourens roblourens deleted the roblou/debugCellCommand branch August 11, 2021 22:38
@DavidKutu
Copy link

I don't see how you're sending the disconnect. Is it with the normal debugging widget?
I haven't had issues with neither normal debugging nor run by line

@roblourens
Copy link
Member Author

The disconnect method gets called in the same way it is for RBL when the cell execution is finished. If I click the disconnect button, it works correctly

@DavidKutu
Copy link

That should work, we're going to have to debug it :/

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.

5 participants