search

atom / bracket-matcher

Jump to brackets
MIT License
142 stars 98 forks source link

Deactivate cleanly #384

Open winstliu opened 5 years ago

winstliu commented 5 years ago

Requirements

Description of the Change

bracket-matcher does not deactivate cleanly - that is, it continues to match brackets and provide bracket autocompletion even after it is deactivated. This is because none of its subscriptions are disposed upon deactivation. We now keep track of the observeTextEditors and editor.onDidDestroy subscriptions and dispose of them so that bracket-matcher does not continue matching brackets whenever a new editor is opened. Furthermore, upon deactivation/editor destruction, we now dispose of all the subscriptions that we create per-editor (this additionally means that each class no longer has to individually subscribe to the editor.onDidDestroy).

Alternate Designs

Coming later

Benefits

Deactivation will work.

Possible Drawbacks

None.

Applicable Issues

Fixes #156

Todo:

winstliu commented 5 years ago

@Aerijo I don't think the WeakSet was necessary before, but we might need a WeakMap now so that we can associate editors to subscriptions.

Arcanemagus commented 5 years ago

Is the watchedEditors.has check in observeTextEditors necessary? Seems to me like we're disposing of everything in editor.onDidDestroy, so we shouldn't need a has check when a new editor is added.

It doesn't really hurt having it in there, but I think you're right that it's not actually necessary since on activate it should be empty and an editor should be given to us twice.

winstliu commented 5 years ago

Well, this is complicated by the use of _.adviseBefore to hook into insertText and the like.