Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Adding CLC version checks as well as localizing #72

Merged
merged 2 commits into from
Jan 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/helpers/strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,5 +48,12 @@ export class Strings {
static StatusCode401: string = "Unauthorized. Check your authentication credentials and try again.";
static StatusCodeOffline: string = "It appears Visual Studio Code is offline. Please connect and try again.";
static ProxyUnreachable: string = "It appears the configured proxy is not reachable. Please check your connection and try again.";

// TFVC messages/errors
static ChooseItemQuickPickPlaceHolder: string = "Choose a file to open it.";
static TfvcLocationMissingError: string = "The path to the TFVC command line was not found in the user settings. Please set this value (tfvc.location) and try again.";
static TfMissingError: string = "Unable to find the TF executable at the expected location. Please verify the installation and location of TF. Expected path: ";
static TfExecFailedError: string = "Execution of the TFVC command line failed unexpectedly.";
static TfVersionWarning: string = "The installed version of the TF command line does not meet the minimum suggested version. You may run into errors or limitations with certain commands until you upgrade. Minimum suggested version: ";
}
/* tslint:enable:variable-name */
30 changes: 19 additions & 11 deletions src/tfvc/repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,23 @@ var _ = require("underscore");
* by the repositoryRootFolder.
*/
export class Repository {
Copy link
Member Author

Choose a reason for hiding this comment

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

some of these changes are just renaming the private variables to have "_"

private _tfvc: Tfvc;
private _repositoryRootFolder: string;
private _env: any;
private _versionAlreadyChecked = false;

public constructor(
private _tfvc: Tfvc,
private repositoryRootFolder: string,
private env: any = {}
) { }
public constructor(tfvc: Tfvc, repositoryRootFolder: string, env: any = {}) {
this._tfvc = tfvc;
this._repositoryRootFolder = repositoryRootFolder;
this._env = env;
}

public get Tfvc(): Tfvc {
return this._tfvc;
}

public get Path(): string {
return this.repositoryRootFolder;
return this._repositoryRootFolder;
}

public async FindWorkspace(localPath: string): Promise<IWorkspace> {
Expand All @@ -43,9 +47,13 @@ export class Repository {
new Status(ignoreFiles === undefined ? true : ignoreFiles));
}

public async Version(): Promise<string> {
return this.RunCommand<string>(
new GetVersion());
public async CheckVersion(): Promise<void> {
if (!this._versionAlreadyChecked) {
// Set the versionAlreadyChecked flag first in case one of the other lines throws
this._versionAlreadyChecked = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd do the version check before setting _versionAlreadyChecked to true.

Copy link
Member Author

Choose a reason for hiding this comment

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

So, CheckVersion will throw the error if the version isn't correct. And I don't want to throw it twice, so I need to set the boolean before it might throw. I will add a comment.

const version: string = await this.RunCommand<string>(new GetVersion());
this._tfvc.CheckVersion(version);
}
}

public async RunCommand<T>(cmd: ITfvcCommand<T>): Promise<T> {
Expand All @@ -55,7 +63,7 @@ export class Repository {

private async exec(args: string[], options: any = {}): Promise<IExecutionResult> {
options.env = _.assign({}, options.env || {});
options.env = _.assign(options.env, this.env);
return await this.Tfvc.Exec(this.repositoryRootFolder, args, options);
options.env = _.assign(options.env, this._env);
return await this.Tfvc.Exec(this._repositoryRootFolder, args, options);
}
}
12 changes: 11 additions & 1 deletion src/tfvc/tfvc-extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,22 @@ export class TfvcExtension {
try {
const tfvc: Tfvc = new Tfvc();
const repo: Repository = tfvc.Open(workspace.rootPath);
await this.checkVersion(repo);

const chosenItem: IPendingChange = await UIHelper.ChoosePendingChange(await repo.GetStatus());
if (chosenItem) {
window.showTextDocument(await workspace.openTextDocument(chosenItem.localItem));
}
} catch (err) {
VsCodeUtils.ShowErrorMessage(err);
VsCodeUtils.ShowErrorMessage(err.message);
}
}

private async checkVersion(repo: Repository) {
try {
await repo.CheckVersion();
} catch (err) {
VsCodeUtils.ShowWarningMessage(err.message);
}
}
}
44 changes: 34 additions & 10 deletions src/tfvc/tfvc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@

import * as cp from "child_process";
import { EventEmitter, Event } from "vscode";
import { Strings } from "../helpers/strings";
import { IDisposable, toDisposable, dispose } from "./util";
import { IExecutionResult } from "./interfaces";
import { TfvcError, TfvcErrorCodes } from "./tfvcerror";
import { Repository } from "./repository";
import { TfvcSettings } from "./tfvcsettings";
import { TfvcVersion } from "./tfvcversion";

var _ = require("underscore");
var fs = require("fs");

export class Tfvc {

Expand All @@ -29,16 +32,42 @@ export class Tfvc {
const settings = new TfvcSettings();
this._tfvcPath = settings.Location;
if (!this._tfvcPath) {
//TODO localize the message once this code stops changing so much
throw new TfvcError({
message: "The path to the TFVC command line was not found in the user settings. Please set this value.",
message: Strings.TfvcLocationMissingError,
tfvcErrorCode: TfvcErrorCodes.TfvcNotFound
});
}
}
//TODO check to make sure that the file exists in that location
//TODO check the version of TFVC command line

// check to make sure that the file exists in that location
const stats: any = fs.lstatSync(this._tfvcPath);
if (!stats || !stats.isFile()) {
throw new TfvcError({
Copy link
Contributor

Choose a reason for hiding this comment

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

We should chat about our understanding of how TfvcError works. It is a way to create objects that we can throw but we can't catch(){} a particular type... which reduces its effectiveness. Nothing to do for this comment, just FYI.

message: Strings.TfMissingError,
tfvcErrorCode: TfvcErrorCodes.TfvcNotFound
});
}
}

/**
* This method checks the version of the CLC against the minimum version that we expect.
* It throws an error if the version does not meet or exceed the minimum.
*/
public CheckVersion(version: string) {
if (!version) {
// If the version isn't set just return
return;
}

// check the version of TFVC command line
const minVersion: TfvcVersion = TfvcVersion.FromString("14.0.4");
const curVersion: TfvcVersion = TfvcVersion.FromString(version);
if (TfvcVersion.Compare(curVersion, minVersion) < 0) {
throw new TfvcError({
message: Strings.TfVersionWarning + minVersion.ToString(),
tfvcErrorCode: TfvcErrorCodes.TfvcMinVersionWarning
});
}
}

public Open(repositoryRootFolder: string, env: any = {}): Repository {
Expand All @@ -52,10 +81,6 @@ export class Tfvc {
}

private spawn(args: string[], options: any = {}): cp.ChildProcess {
if (!this._tfvcPath) {
throw new Error("tfvc could not be found in the system.");
}

if (!options) {
options = {};
}
Expand Down Expand Up @@ -104,7 +129,7 @@ export class Tfvc {
}

return Promise.reject<IExecutionResult>(new TfvcError({
message: "Failed to execute tfvc",
message: Strings.TfExecFailedError,
stdout: result.stdout,
stderr: result.stderr,
exitCode: result.exitCode,
Expand Down Expand Up @@ -155,4 +180,3 @@ export class Tfvc {
this._onOutput.fire(output);
}
}

5 changes: 4 additions & 1 deletion src/tfvc/tfvcerror.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*--------------------------------------------------------------------------------------------*/
"use strict";

import { Strings } from "../helpers/strings";
import { ITfvcErrorData } from "./interfaces";

export class TfvcError {
Expand All @@ -18,6 +19,7 @@ export class TfvcError {

public static createArgumentMissingError(argumentName: string): TfvcError {
return new TfvcError({
// This is a developer error - no need to localize
message: `Argument is required: ${argumentName}`,
tfvcErrorCode: TfvcErrorCodes.MissingArgument
});
Expand All @@ -31,7 +33,7 @@ export class TfvcError {
this.error = undefined;
}

this.message = this.message || data.message || "Tfvc error";
this.message = this.message || data.message || Strings.TfExecFailedError;
this.stdout = data.stdout;
this.stderr = data.stderr;
this.exitCode = data.exitCode;
Expand Down Expand Up @@ -76,6 +78,7 @@ export class TfvcErrorCodes {
public static get DirtyWorkTree(): string { return "DirtyWorkTree"; }
public static get CantOpenResource(): string { return "CantOpenResource"; }
public static get TfvcNotFound(): string { return "TfvcNotFound"; }
public static get TfvcMinVersionWarning(): string { return "TfvcMinVersionWarning"; }
public static get CantCreatePipe(): string { return "CantCreatePipe"; }
public static get CantAccessRemote(): string { return "CantAccessRemote"; }
public static get RepositoryNotFound(): string { return "RepositoryNotFound"; }
Expand Down
56 changes: 56 additions & 0 deletions src/tfvc/tfvcversion.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
"use strict";

/**
* This class represents the Version of the TF command line.
*/
export class TfvcVersion {
private static separator: string = ".";

private _major: number;
private _minor: number;
private _revision: number;
private _build: string;

public static FromString(version: string): TfvcVersion {
const parts: string[] = version ? version.split(TfvcVersion.separator) : [];
let major = parts.length >= 1 ? Number(parts[0]) : 0;
let minor = parts.length >= 2 ? Number(parts[1]) : 0;
let revision = parts.length >= 3 ? Number(parts[2]) : 0;
let build = parts.length >= 4 ? parts.slice(3).join(TfvcVersion.separator) : "";
return new TfvcVersion(major, minor, revision, build);
}

public static Compare(version1: TfvcVersion, version2: TfvcVersion): number {
if (version1._major !== version2._major) {
return version1._major - version2._major;
}
if (version1._minor !== version2._minor) {
return version1._minor - version2._minor;
}
if (version1._revision !== version2._revision) {
return version1._revision - version2._revision;
}
return 0;
}

public constructor(major: number, minor: number, revision: number, build: string) {
this._major = major;
this._minor = minor;
this._revision = revision;
this._build = build;
}

public get Major(): number { return this._major; }
public get Minor(): number { return this._minor; }
public get Revision(): number { return this._revision; }
public get Build(): string { return this._build; }

public ToString(): string {
return this._major + TfvcVersion.separator + this._minor + TfvcVersion.separator + this._revision +
(this._build ? TfvcVersion.separator + this._build : "");
}
}
4 changes: 2 additions & 2 deletions src/tfvc/uihelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"use strict";

import { QuickPickItem, window, workspace } from "vscode";
import { Strings } from "../helpers/strings";
import { IPendingChange } from "./interfaces";

var path = require("path");
Expand All @@ -22,9 +23,8 @@ export class UIHelper {
});
}
// Then, show the quick pick window and get back the one they chose
//TODO localize strings
let item: QuickPickItem = await window.showQuickPick(
items, { matchOnDescription: true, placeHolder: "Choose a file to open the file." });
items, { matchOnDescription: true, placeHolder: Strings.ChooseItemQuickPickPlaceHolder });

// Finally, find the matching pending change and return it
if (item) {
Expand Down
60 changes: 60 additions & 0 deletions test/tfvc/tfvcversion.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
"use strict";

import { assert } from "chai";

import { TfvcVersion } from "../../src/tfvc/tfvcversion";

describe("Tfvc-Version", function() {
beforeEach(function() {
//
});

it("should verify constructor", function() {
let version: TfvcVersion = new TfvcVersion(12, 11, 10, "");
assert.equal(version.ToString(), "12.11.10");
});

it("should verify constructor - with build", function() {
let version: TfvcVersion = new TfvcVersion(12, 11, 10, "buildpart");
assert.equal(version.ToString(), "12.11.10.buildpart");
});

it("should verify constructor - with dotted build", function() {
let version: TfvcVersion = new TfvcVersion(12, 11, 10, "build.part.");
assert.equal(version.ToString(), "12.11.10.build.part.");
});

it("should verify FromString", function() {
let version: TfvcVersion = TfvcVersion.FromString("12.11.10.build.part.");
assert.equal(version.ToString(), "12.11.10.build.part.");
});

it("should verify FromString - missing build", function() {
let version: TfvcVersion = TfvcVersion.FromString("12.11.10");
assert.equal(version.ToString(), "12.11.10");
});

it("should verify FromString - missing revision", function() {
let version: TfvcVersion = TfvcVersion.FromString("12.11");
assert.equal(version.ToString(), "12.11.0");
});

it("should verify Compare", function() {
let version1: TfvcVersion = TfvcVersion.FromString("12.11");
let version2: TfvcVersion = TfvcVersion.FromString("12.11.10");
assert.isTrue(TfvcVersion.Compare(version1, version2) < 0);
assert.isTrue(TfvcVersion.Compare(version2, version1) > 0);
});

it("should verify Compare - equals", function() {
let version1: TfvcVersion = TfvcVersion.FromString("12.11.10");
let version2: TfvcVersion = TfvcVersion.FromString("12.11.10");
assert.isTrue(TfvcVersion.Compare(version1, version2) === 0);
assert.isTrue(TfvcVersion.Compare(version2, version1) === 0);
});

});