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

Do not break additional DocumentFilters #7903

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Oct 23, 2024

This PR improves behaviour of CloneableEditorSupport in that it will not break additional DocumentFilters which may be attached to the AbstractDocument.

If a plugin registers a filter for a document, the CloneableEditorSupport will erase that filter on document's reload, basically at some unexpected and uncontrolled point. This PR changes the impl so it will not remove the DocumentFilter which would unset the client's DocumentFilter and breaks the pass-through chain), but rather disables it.

When the document is listened on again, and it's the same document (!) as the existing filter works with, the filter is re-enabled.

If the document reference changes, a new filter is created; the user must detect that and restore its own filter as the old Document is gone. This should happen mainly if the Document expires and is garbage-collected and is loaded again.

@sdedic sdedic added Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) Platform [ci] enable platform tests (platform/*) labels Oct 23, 2024
@sdedic sdedic added this to the NB25 milestone Oct 23, 2024
@sdedic sdedic requested review from dbalek and lahodaj October 23, 2024 17:32
@sdedic sdedic self-assigned this Oct 23, 2024
@sdedic sdedic force-pushed the lsp/document-filter branch from 2b4a688 to 9b75397 Compare October 23, 2024 19:23
Copy link
Contributor

@lahodaj lahodaj left a comment

Choose a reason for hiding this comment

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

Looks OK to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) Platform [ci] enable platform tests (platform/*)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants