-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[marker] store data and clean on file deletion #541
Conversation
@@ -117,6 +126,69 @@ export abstract class MarkerManager<D extends object> { | |||
protected readonly uri2MarkerCollection = new Map<string, MarkerCollection<D>>(); | |||
protected readonly onDidChangeMarkersEmitter = new Emitter<void>(); | |||
|
|||
initialized: Promise<void>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
readonly?
window.onunload = () => { | ||
for (const contribution of this.contributions.getContributions()) { | ||
if (contribution.onStop) { | ||
contribution.onStop(this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we do try/catch here? that once contribution does not break others
e8c2c84
to
15666ac
Compare
…#540) Signed-off-by: Sven Efftinge <sven.efftinge@typefox.io>
15666ac
to
9487a1b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I need this change for the next PR. Can I merge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just nits
*/ | ||
onStart?(app: FrontendApplication): void; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
empty line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://palantir.github.io/tslint/rules/no-consecutive-blank-lines/ can catch such cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice I'll add that thx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would also be good if our autoformatting would clean that already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes let's fix that as part of #413
*/ | ||
onStart(app: FrontendApplication): void; | ||
onStop?(app: FrontendApplication): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing semi-column
constructor( | ||
@inject(WidgetManager) protected widgetManager: WidgetManager, | ||
@inject(ILogger) protected logger: ILogger, | ||
@inject(StorageService) protected storageService: StorageService) { } | ||
|
||
onStart(app: FrontendApplication): void { | ||
const storageKey = 'layout'; | ||
this.storageService.getData(storageKey, undefined).then((serializedLayoutData: string | undefined) => { | ||
this.storageService.getData(this.storageKey, undefined).then((serializedLayoutData: string | undefined) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to specify the undefined default... but I don't mind
@@ -117,6 +126,69 @@ export abstract class MarkerManager<D extends object> { | |||
protected readonly uri2MarkerCollection = new Map<string, MarkerCollection<D>>(); | |||
protected readonly onDidChangeMarkersEmitter = new Emitter<void>(); | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
empty line
Signed-off-by: Sven Efftinge <sven.efftinge@typefox.io>
9487a1b
to
f9c7197
Compare
A marker-service will remove markers when a file gets deleted.
A marker-service will persist marker information between sessions.