Added more usage of disposables

Author: edgardmessias

src/commands.ts

@@ -14,7 +14,8 @@ import {
   WorkspaceEdit,
   Range,
   Position,
-  TextEditor
+  TextEditor,
+  Disposable
 } from "vscode";
 import { inputCommitMessage } from "./messages";
 import { Svn, Status, SvnErrorCodes } from "./svn";
@@ -27,6 +28,7 @@ import * as path from "path";
 import { start } from "repl";
 import { getConflictPickOptions } from "./conflictItems";
 import { applyLineChanges } from "./lineChanges";
+import { IDisposable } from "./util";
 
 interface CommandOptions {
   repository?: boolean;
@@ -136,11 +138,11 @@ class NewChangeListItem implements QuickPickItem {
   }
 }
 
-export class SvnCommands {
-  private commands: any[] = [];
+export class SvnCommands implements IDisposable {
+  private disposables: Disposable[];
 
   constructor(private model: Model) {
-    Commands.map(({ commandId, method, options }) => {
+    this.disposables = Commands.map(({ commandId, method, options }) => {
       const command = this.createCommand(method, options);
       if (options.diff) {
         return commands.registerDiffInformationCommand(commandId, command);
@@ -1089,4 +1091,8 @@ export class SvnCommands {
 
     return Promise.all(promises);
   }
+
+  dispose(): void {
+    this.disposables.forEach(d => d.dispose());
+  }
 }

src/decorationProvider.ts

@@ -146,26 +146,33 @@ class SvnDecorationProvider implements DecorationProvider {
 }
 
 export class SvnDecorations {
+  private enabled: boolean = false;
   private configListener: Disposable;
   private modelListener: Disposable[] = [];
   private providers = new Map<Repository, Disposable>();
 
   constructor(private model: Model) {
-    // this.configListener = workspace.onDidChangeConfiguration(
-    //   e => e.affectsConfiguration("svn.decorations.enabled") && this.update()
-    // );
+    this.configListener = workspace.onDidChangeConfiguration(() =>
+      this.update()
+    );
     this.update();
   }
 
   private update(): void {
     const enabled = workspace
       .getConfiguration()
-      .get<boolean>("svn.decorations.enabled");
+      .get<boolean>("svn.decorations.enabled", true);
+
+    if (this.enabled === enabled) {
+      return;
+    }
+
     if (enabled) {
       this.enable();
     } else {
       this.disable();
     }
+    this.enabled = enabled;
   }
 
   private enable(): void {
@@ -175,11 +182,11 @@ export class SvnDecorations {
       this,
       this.modelListener
     );
-    // this.model.onDidCloseRepository(
-    //   this.onDidCloseRepository,
-    //   this,
-    //   this.modelListener
-    // );
+    this.model.onDidCloseRepository(
+      this.onDidCloseRepository,
+      this,
+      this.modelListener
+    );
     this.model.repositories.forEach(this.onDidOpenRepository, this);
   }
 
@@ -205,8 +212,6 @@ export class SvnDecorations {
 
   dispose(): void {
     this.configListener.dispose();
-    this.modelListener.forEach(d => d.dispose());
-    this.providers.forEach(value => value.dispose);
-    this.providers.clear();
+    this.disable();
   }
 }

src/extension.ts

@@ -33,7 +33,7 @@ async function init(context: ExtensionContext, disposables: Disposable[]) {
   const model = new Model(svn);
   const contentProvider = new SvnContentProvider(model);
   const svnCommands = new SvnCommands(model);
-  disposables.push(model, contentProvider);
+  disposables.push(model, contentProvider, svnCommands);
 
   // First, check the vscode has support to DecorationProvider
   if (hasSupportToDecorationProvider()) {
@@ -54,10 +54,6 @@ async function init(context: ExtensionContext, disposables: Disposable[]) {
 
   outputChannel.appendLine("Using svn " + info.version + " from " + info.path);
 
-  context.subscriptions.push(
-    new Disposable(() => Disposable.from(...disposables).dispose())
-  );
-
   const onOutput = (str: string) => outputChannel.append(str);
   svn.onOutput.addListener("log", onOutput);
   disposables.push(

src/model.ts

@@ -10,9 +10,9 @@ import {
 import * as fs from "fs";
 import * as path from "path";
 import * as micromatch from "micromatch";
-import { Repository } from "./repository";
+import { Repository, RepositoryState } from "./repository";
 import { Svn } from "./svn";
-import { dispose, anyEvent, filterEvent } from "./util";
+import { dispose, anyEvent, filterEvent, IDisposable } from "./util";
 import { sequentialize } from "./decorators";
 
 export interface ModelChangeEvent {
@@ -29,7 +29,7 @@ interface OpenRepository extends Disposable {
   repository: Repository;
 }
 
-export class Model {
+export class Model implements IDisposable {
   private _onDidOpenRepository = new EventEmitter<Repository>();
   readonly onDidOpenRepository: Event<Repository> = this._onDidOpenRepository
     .event;
@@ -284,11 +284,19 @@ export class Model {
   }
 
   private open(repository: Repository): void {
+    const onDidDisappearRepository = filterEvent(
+      repository.onDidChangeState,
+      state => state === RepositoryState.Disposed
+    );
+    const disappearListener = onDidDisappearRepository(() => dispose());
+
     const changeListener = repository.onDidChangeRepository(uri =>
       this._onDidChangeRepository.fire({ repository, uri })
     );
 
     const dispose = () => {
+      disappearListener.dispose();
+      changeListener.dispose();
       changeListener.dispose();
       repository.dispose();
 
@@ -303,6 +311,16 @@ export class Model {
     this._onDidOpenRepository.fire(repository);
   }
 
+  close(repository: Repository): void {
+    const openRepository = this.getOpenRepository(repository);
+
+    if (!openRepository) {
+      return;
+    }
+
+    openRepository.dispose();
+  }
+
   async pickRepository() {
     if (this.openRepositories.length === 0) {
       throw new Error("There are no available repositories");

src/repository.ts

@@ -137,9 +137,6 @@ export class Repository {
   private _onDidChangeStatus = new EventEmitter<void>();
   readonly onDidChangeStatus: Event<void> = this._onDidChangeStatus.event;
 
-  private _onDidChangeBranch = new EventEmitter<void>();
-  readonly onDidChangeBranch: Event<void> = this._onDidChangeBranch.event;
-
   private _onDidChangeNewsCommit = new EventEmitter<void>();
   readonly onDidChangeNewsCommit: Event<void> = this._onDidChangeNewsCommit
     .event;
@@ -265,13 +262,24 @@ export class Repository {
     this.external.hideWhenEmpty = true;
     this.conflicts.hideWhenEmpty = true;
 
+    this.disposables.push(this.changes);
+    this.disposables.push(this.unversioned);
+    this.disposables.push(this.external);
+    this.disposables.push(this.conflicts);
+
     const svnConfig = workspace.getConfiguration("svn");
 
     const updateFreqNews = svnConfig.get<number>("svn.newsCommits.update");
     if (updateFreqNews) {
-      setInterval(() => {
+      const interval = setInterval(() => {
         this.updateNewsCommits();
       }, 1000 * 60 * updateFreqNews);
+
+      this.disposables.push(
+        toDisposable(() => {
+          clearInterval(interval);
+        })
+      );
     }
 
     this.updateNewsCommits();
@@ -431,6 +439,7 @@ export class Repository {
           `Changelist "${changelist}"`
         ) as SvnResourceGroup;
         group.hideWhenEmpty = true;
+        this.disposables.push(group);
 
         this.changelists.set(changelist, group);
       }
@@ -507,9 +516,7 @@ export class Repository {
   }
 
   async addFiles(files: string[]) {
-    return await this.run(Operation.Add, () =>
-      this.repository.addFiles(files)
-    );
+    return await this.run(Operation.Add, () => this.repository.addFiles(files));
   }
 
   async addChangelist(files: string[], changelist: string) {

src/svn.ts

@@ -7,6 +7,7 @@ import * as path from "path";
 import { Repository } from "./svnRepository";
 import { parseInfoXml } from "./infoParser";
 import { SpawnOptions } from "child_process";
+import { IDisposable, toDisposable, dispose } from "./util";
 
 // List: https://github.com/apache/subversion/blob/1.6.x/subversion/svn/schema/status.rnc#L33
 export enum Status {
@@ -180,25 +181,47 @@ export class Svn {
 
     let process = cp.spawn(this.svnPath, args, options);
 
+    const disposables: IDisposable[] = [];
+
+    const once = (
+      ee: NodeJS.EventEmitter,
+      name: string,
+      fn: (...args: any[]) => void
+    ) => {
+      ee.once(name, fn);
+      disposables.push(toDisposable(() => ee.removeListener(name, fn)));
+    };
+
+    const on = (
+      ee: NodeJS.EventEmitter,
+      name: string,
+      fn: (...args: any[]) => void
+    ) => {
+      ee.on(name, fn);
+      disposables.push(toDisposable(() => ee.removeListener(name, fn)));
+    };
+
     let [exitCode, stdout, stderr] = await Promise.all<any>([
       new Promise<number>((resolve, reject) => {
-        process.once("error", reject);
-        process.once("exit", resolve);
+        once(process, "error", reject);
+        once(process, "exit", resolve);
       }),
       new Promise<Buffer>(resolve => {
         const buffers: Buffer[] = [];
-        process.stdout.on("data", (b: Buffer) => buffers.push(b));
-        process.stdout.once("close", () => resolve(Buffer.concat(buffers)));
+        on(process.stdout, "data", (b: Buffer) => buffers.push(b));
+        once(process.stdout, "close", () => resolve(Buffer.concat(buffers)));
       }),
       new Promise<string>(resolve => {
         const buffers: Buffer[] = [];
-        process.stderr.on("data", (b: Buffer) => buffers.push(b));
-        process.stderr.once("close", () =>
+        on(process.stderr, "data", (b: Buffer) => buffers.push(b));
+        once(process.stderr, "close", () =>
           resolve(Buffer.concat(buffers).toString())
         );
       })
     ]);
 
+    dispose(disposables);
+
     let encoding = "utf8";
 
     // SVN with '--xml' always return 'UTF-8', and jschardet detects this encoding: 'TIS-620'

src/svnContentProvider.ts

@@ -14,7 +14,8 @@ import {
   filterEvent,
   eventToPromise,
   isDescendant,
-  toDisposable
+  toDisposable,
+  IDisposable
 } from "./util";
 
 interface CacheRow {
@@ -29,7 +30,8 @@ interface Cache {
 const THREE_MINUTES = 1000 * 60 * 3;
 const FIVE_MINUTES = 1000 * 60 * 5;
 
-export class SvnContentProvider implements TextDocumentContentProvider {
+export class SvnContentProvider
+  implements IDisposable, TextDocumentContentProvider {
   private _onDidChange = new EventEmitter<Uri>();
   get onDidChange(): Event<Uri> {
     return this._onDidChange.event;

src/test/testUtil.ts

@@ -156,7 +156,8 @@ export async function destroyPath(fullPath: string) {
 }
 
 export function destroyAllTempPaths() {
-  for (let path of tempDirList) {
+  let path;
+  while ((path = tempDirList.shift())) {
     destroyPath(path);
   }
 }

src/util.ts

@@ -1,7 +1,7 @@
 import { Event, window } from "vscode";
 import { sep } from "path";
 
-export interface BaseDisposable {
+export interface IDisposable {
   dispose(): void;
 }
 
@@ -37,12 +37,12 @@ export function dispose(disposables: any[]): any[] {
 }
 
 export function combinedDisposable(
-  disposables: BaseDisposable[]
-): BaseDisposable {
+  disposables: IDisposable[]
+): IDisposable {
   return toDisposable(() => dispose(disposables));
 }
 
-export function toDisposable(dispose: () => void): BaseDisposable {
+export function toDisposable(dispose: () => void): IDisposable {
   return { dispose };
 }