Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

Remove onNewRenderLine #160

Merged
merged 1 commit into from
Nov 7, 2019
Merged

Conversation

jtpio
Copy link
Member

@jtpio jtpio commented Nov 7, 2019

Fixes #159.

Not exactly sure why we had this in the first place. But the onNewRenderLine was modifying the breakpoints model directly and triggering CodeMirror issues (#159).

Instead we'll want to send the dumpCell and setBreakpoints requests via the service when a user modifies the content of a cell.

remove-renderline

@jtpio jtpio added this to the 0.1.0 milestone Nov 7, 2019
@jtpio jtpio requested a review from KsavinN November 7, 2019 11:08
@jtpio
Copy link
Member Author

jtpio commented Nov 7, 2019

cc @KsavinN if you want to have a look

@KsavinN
Copy link
Collaborator

KsavinN commented Nov 7, 2019

There is good news and bad:

Realy great is that fix #159

but its seems we need found solutions for this:
example in vs-code:
Peek 2019-11-07 15-43

and now in this PR:
bad

@jtpio
Copy link
Member Author

jtpio commented Nov 7, 2019

Yes that's right.

But since we need to sync the content of the cell when it is modified on the frontend, we'll also have to update the breakpoints. The flow would be something like this:

  • Add a signal connection to activeCell.model.contentChanged.
  • When a user changes the content of the cell, the signal will be emitted.
  • Send dumpCell and setBreakpoints, probably behind an ActivityMonitor or Debouncer to avoid sending to many requests.
  • Since the service is responsible for updating the breakpoints, the view will refresh itself when a response has been received from the kernel and the model changed.

@jtpio
Copy link
Member Author

jtpio commented Nov 7, 2019

Opened #162 so we can track that in a separate issue.

@KsavinN KsavinN merged commit 760932e into jupyterlab:master Nov 7, 2019
@jtpio jtpio deleted the remove-renderline branch November 7, 2019 15:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when adding new lines to cells with breakpoints
2 participants