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

Refactor how the notebook changes are picked up, events bound and unbound #421

Merged
merged 8 commits into from
Dec 13, 2020

Conversation

krassowski
Copy link
Member

@krassowski krassowski commented Dec 11, 2020

Previously the binding of events was only performed upon the user entering a code cell. Moreover, updates to the virtual document were only triggered on code edition, and not on cell edition (rearrangement, cell pasting etc). This was kind-of ok for the initial demo, but a number of scenarios require a better management of the events binding [E] and document updates [D]:

  • code cell being converted to markdown or raw cell:
    • [E] we do not want to trigger LSP there
    • [D] we want to refresh virtual document to exclude the converted fragment
  • code cells being pasted or converted from markdown/raw cells:
    • [E] user should be able to get context menu on those without the need to enter them first
    • [D] the diagnostics should be shown in these just after paste
  • cells being moved around:
    • [D] document should get updated to reflect the changes (and query diagnostics) associated with the change

Overall this small refactoring has a hug potential to squash numerous bugs and stability issues that arouse only after a longer work with a notebook (i.e. involving the use of feature such as moving cells, pasting, conversion of types) and were difficult to pinpoint so far.

References

Fixes #393.

Code changes

  • Added editorAdded and editorRemoved events to WidgetAdapter.
  • Added callback for removal of events from editors and made use of it.
  • Started tracking Notebook.model.cells and trigger document updates, or editor events from there, as needed.

User-facing changes

before after
before-markdown after-markdown
before-move after-move

Plus pasting of cells works better with LSP, but a third set of GIFs would be redundant.

Backwards-incompatible changes

None

Chores

  • linted
  • tested
  • documented
  • changelog entry

@krassowski krassowski merged commit 2a7291a into master Dec 13, 2020
@krassowski krassowski deleted the fix-editor-removal branch February 16, 2021 23:23
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.

On keypress event is not unbound after the cell is changed to markdown
1 participant