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

[marker] store data and clean on file deletion #541

Merged
merged 2 commits into from
Sep 22, 2017
Merged

[marker] store data and clean on file deletion #541

merged 2 commits into from
Sep 22, 2017

Conversation

svenefftinge
Copy link
Contributor

A marker-service will remove markers when a file gets deleted.
A marker-service will persist marker information between sessions.

@@ -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>;
Copy link
Member

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);
Copy link
Member

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

…#540)

Signed-off-by: Sven Efftinge <sven.efftinge@typefox.io>
Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

LGTM

@svenefftinge svenefftinge requested a review from hexa00 September 22, 2017 11:11
@svenefftinge
Copy link
Contributor Author

I need this change for the next PR. Can I merge?

Copy link

@hexa00 hexa00 left a 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;


Copy link

Choose a reason for hiding this comment

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

empty line

Copy link
Member

Choose a reason for hiding this comment

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

Copy link

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

Copy link
Contributor Author

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.

Copy link

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
Copy link

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) => {
Copy link

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>();

Copy link

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>
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.

3 participants