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

Investigate how to make persisted cell breakpoints robust #99943

Closed
weinand opened this issue Jun 12, 2020 · 3 comments
Closed

Investigate how to make persisted cell breakpoints robust #99943

weinand opened this issue Jun 12, 2020 · 3 comments
Assignees
Labels
Milestone

Comments

@weinand
Copy link
Contributor

weinand commented Jun 12, 2020

VS Code manages breakpoints outside from debug extensions and debug sessions. This includes breakpoints in notebook cells.

Today cell breakpoints use the cell's number to identify a breakpoint within the notebook.
When notebook cells are rearranged within a notebook (which renumbers cells), the persisted breakpoints no longer refer to the correct cell.

@weinand weinand added bug Issue identified by VS Code Team member as probable bug notebook labels Jun 12, 2020
@weinand weinand added this to the June 2020 milestone Jun 12, 2020
@weinand weinand self-assigned this Jun 12, 2020
@weinand
Copy link
Contributor Author

weinand commented Jun 18, 2020

More details:

After rearranging cells in a notebook, the cell URIs are not changed as long as the notebook stays open (or is not re-read). That means that breakpoints still work. However, it does not make sense to show the URI's fragment in the UI, for example as "Cell 3" in the call stack view because the cell might no longer be cell "3".

Another problem occurs after closing the notebook and reopening it: here cell URIs are reassigned and the fragment of the cell URI changes. Since cell URIs are used in VS Code's breakpoint UI to persist them, they no longer refer to the correct cell.

To address these issues I will do the following:

  • when generating DAP Source objects (used in stack frames), we will stop using the URI's fragment for the display name, but use the cell's real index instead.
  • when the notebook is closed, we will update the URI in the persisted breakpoints to reflect the cell's new real index.

@jrieken does this makes sense or do you have other suggestions?

@weinand weinand removed the bug Issue identified by VS Code Team member as probable bug label Jun 18, 2020
@jrieken
Copy link
Member

jrieken commented Jun 18, 2020

Yeah, that sound reasonable. I do have a similar, but less severe, issue when moving cells that have problems - in that case error navigation is jumping back and forth between cells (see #93938). I was thinking if that problem can be tackled by doing a rename, e.g cells change their names when being moved. However, we aren't very good with renames, e.g documents cannot change their uri and we would need to send close and open events... Still, that might be a solution that works for more use-cases.

we will stop using the URI's fragment for the display name, but use the cell's real index instead.

For language features we aren't showing the cell index at anymore. I think the motivation was that we don't show indexes in the UI and therefore users cannot relate to them. I think that works well when showing cell source, like reference search, but is more confusing in the problems panel where we show names.

@weinand
Copy link
Contributor Author

weinand commented Jun 18, 2020

I still think that showing cell indices in the UI would be helpful.

Today we show the cell in the Call Stack (because we can), but not in the Breakpoints view (because we cannot):

2020-06-18_12-20-34

@weinand weinand closed this as completed Jun 29, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Aug 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants