[WIP] James/create project #197

Sweetman · 2017-10-30T23:33:22Z

What does this PR do?

Add command to create a new DX project

What issues does this PR fix or reference?

@W-4284294@

Sweetman · 2017-10-30T23:39:09Z

packages/salesforcedx-vscode-core/package.json

-  "activationEvents": ["workspaceContains:sfdx-project.json"],
+  "activationEvents": [
+    "workspaceContains:sfdx-project.json,",
+    "onCommand:sfdx.force.project.create"


Adding an additional activation event for the extension to allow the create command to execute. Two scenarios arise from this:

Project create command is ran but does not go execute (i.e. user cancels out of input), all other commands are still hidden because of their when clause.

Project create command is ran and executes all the way through causing the host to refresh with the new folder and go through the extension loading again. Since the command creates sfdx project, sfdx-project.json will be present and the extension will activate, showing all the commands.

Sweetman · 2017-10-30T23:39:35Z

packages/salesforcedx-vscode-core/package.nls.json

@@ -44,5 +44,6 @@
    "SFDX: Execute Anonymous Apex with Currently Selected Text",
  "feature_previews_title": "Salesforce Feature Previews",
  "enable_refresh_description":
-    "Enables refresh of SObject definitions from a scratch org"
+    "Enables refresh of SObject definitions from a scratch org",
+  "force_project_create_text": "SFDX: Create Project"


@ruthemmanuelle please confirm that this text is ok.

Sweetman · 2017-10-30T23:40:47Z

packages/salesforcedx-vscode-core/src/commands/commands.ts

@@ -117,6 +117,12 @@ export class SfdxWorkspaceChecker implements PreconditionChecker {
  }
 }

+export class EmptyPreChecker implements PreconditionChecker {


Command does not require any preconditions to be ran (unlike the other commands which all needed an sfdx workspace).

Sweetman · 2017-10-30T23:41:51Z

packages/salesforcedx-vscode-core/src/commands/forceProjectCreate.ts

+
+    execution.processExitSubject.subscribe(async data => {
+      if (data != undefined && data.toString() === '0') {
+        await vscode.commands.executeCommand(


This will close and reopen vscode with the new project in the workspace.

Sweetman · 2017-10-30T23:43:29Z

packages/salesforcedx-vscode-core/src/commands/forceProjectCreate.ts

+  public async gather(): Promise<
+    CancelResponse | ContinueResponse<ProjectURI>
+    > {
+    const projectUri = await vscode.window.showOpenDialog({


Opens native file picker with the settings I set. Only letting it select one folder. Hitting cancel will return undefined.

Sweetman · 2017-10-30T23:45:47Z

packages/salesforcedx-vscode-core/src/commands/forceProjectCreate.ts

+  }
+}
+
+export class PathExistsChecker


Globs for the directory path about to be created, should provide an override prompt if the path already exists.

ruthemmanuelle

The new command's text looks good. I didn't review anything else.

codecov · 2017-10-30T23:58:36Z

Codecov Report

Merging #197 into develop will decrease coverage by 0.19%.
The diff coverage is 67.64%.

@@            Coverage Diff             @@
##           develop     #197     +/-   ##
==========================================
- Coverage    80.66%   80.46%   -0.2%     
==========================================
  Files          110      111      +1     
  Lines         4453     4515     +62     
  Branches       727      740     +13     
==========================================
+ Hits          3592     3633     +41     
- Misses         706      726     +20     
- Partials       155      156      +1

Impacted Files	Coverage Δ
...ages/salesforcedx-vscode-core/src/messages/i18n.ts	`100% <ø> (ø)`	⬆️
packages/salesforcedx-vscode-core/src/index.ts	`100% <100%> (ø)`	⬆️
...ges/salesforcedx-vscode-core/src/commands/index.ts	`100% <100%> (ø)`	⬆️
.../salesforcedx-vscode-core/src/commands/commands.ts	`83.66% <33.33%> (-1.01%)`	⬇️
...edx-vscode-core/src/commands/forceProjectCreate.ts	`64.15% <64.15%> (ø)`
...esforcedx-vscode-core/src/scratch-org-decorator.ts	`87.5% <75%> (-2.98%)`	⬇️
...s/salesforcedx-apex-debugger/src/commands/index.ts	`41.66% <0%> (-1.82%)`	⬇️
...ges/salesforcedx-vscode-apex-debugger/src/index.ts	`25.74% <0%> (+0.49%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ceccbb0...3f52dc4. Read the comment docs.

Sweetman · 2017-10-31T00:00:54Z

packages/salesforcedx-vscode-core/src/index.ts

  console.log('SFDX CLI Extension Activated');

  // Context
-  vscode.commands.executeCommand('setContext', 'sfdx:project_opened', true);
+  const sfdxProjectOpened = await vscode.workspace.findFiles(


Previously always set to true because extension would only activate if sfdx-project.json was present. If the project create command is the activation reason, this may not be true.

Sweetman · 2017-10-31T00:05:14Z

packages/salesforcedx-vscode-core/src/scratch-org-decorator.ts

 import * as path from 'path';
 import { StatusBarAlignment, StatusBarItem, window, workspace } from 'vscode';

-const CONFIG_FILE = path.join(workspace.rootPath!, '.sfdx/sfdx-config.json');
+const CONFIG_FILE = workspace.rootPath


Since the command can be activated without a workspace, the non-null assertion would cause an error. I set it to the top level sfdx-config.json. If this file does not exist, we will still be okay since displayDefaultUsername essentially ignores files that do not exist.

Sweetman · 2017-11-01T23:44:40Z

packages/salesforcedx-vscode-core/src/commands/forceProjectCreate.ts

+  projectName: string;
+}
+
+export class SelectProjectName implements ParametersGatherer<ProjectName> {


Since these ParametersGatherers and Condition checker are currently only used in this class, I am leaving them here. If you would prefer to move them to commands.ts, please comment and let me know.

Sweetman · 2017-11-01T23:47:56Z

packages/salesforcedx-vscode-core/src/messages/i18n.ts

@@ -46,6 +46,7 @@ export const messages = {
    'Enter desired directory (Press Enter to confirm or Esc to cancel)',
  parameter_gatherer_enter_username_name: 'Enter target username',
  parameter_gatherer_enter_alias_name: 'Enter a scratch org alias',
+  parameter_gatherer_enter_project_name: 'Enter project name',


@ruthemmanuelle these messages were also here when I last pinged you but forgot to @mention you here. Just wanted to double check that you were able to take a look at this file as well.

Thanks, @Sweetman . I hadn't seen these other messages, but they look fine.

DatGuyJonathan · 2017-11-03T17:01:03Z

packages/salesforcedx-vscode-core/src/commands/forceProjectCreate.ts

+      canSelectFiles: false,
+      canSelectFolders: true,
+      canSelectMany: false,
+      openLabel: 'Create Project'


Should this be externalized?

DatGuyJonathan · 2017-11-03T17:02:01Z

packages/salesforcedx-vscode-core/src/scratch-org-decorator.ts

-const CONFIG_FILE = path.join(workspace.rootPath!, '.sfdx/sfdx-config.json');
+const CONFIG_FILE = workspace.rootPath
+  ? path.join(workspace.rootPath, '.sfdx/sfdx-config.json')
+  : path.join(os.homedir(), '.sfdx/sfdx-config.json');


You changed a hardcoded path to not use "/" in another PR. Might as well do that again here.

DatGuyJonathan · 2017-11-03T17:17:35Z

packages/salesforcedx-vscode-core/src/commands/forceProjectCreate.ts

+    inputs: ContinueResponse<ProjectNameAndPath> | CancelResponse
+  ): Promise<ContinueResponse<ProjectNameAndPath> | CancelResponse> {
+    if (inputs.type === 'CONTINUE') {
+      const listOfDirs = new glob.GlobSync(


Would fs.existsSync() suffice?

DatGuyJonathan · 2017-11-03T17:26:14Z

packages/salesforcedx-vscode-core/test/commands/forceProjectCreate.test.ts

+      expect(response.type).to.equal('CANCEL');
+    });
+
+    it('Should return cancel if user input is empty string', async () => {


Try a string with just white spaces also.

@kaplanjosh @DatGuyJonathan Should we allow a user to enter just white space? If not, do we show them some form of info/error message? The CLI command works with just white space as a project name. I think we should prompt if they want to continue if the name is all whitespace but open to other ideas if y'all have any.

I see, so force-language-services allows it. It could be by-design that we don't trim the name; @GaryFNC can offer his thoughts on that. If anything is going to be fixed, it should go into force-language-services .

The original discussion with Mike Miller, etc. was that any valid "folder" name was OK - so not much checking in force-language-services.... BUT, it shouldn't really allow empty input or just white spaces..... That should be caught in FLS, but it is a bit harder for a user to input that on the CLI command line, so didn't get noticed. We don't generally trim the names since spaces are allowed in folder names (although yet, it is odd and hard to read if the spaces all come at the end).
So, I would suggest catching it either in FLS and surfacing the error message and prevalidating in the vscode command if you want as well (like the long discussion we had on that issue with IDE2 and commands)

kaplanjosh · 2017-11-07T21:29:09Z

Why do we allow empty input for project name? If it's by mistake we should fix that. If there's a reason, let's find out so we can discuss.

On Tue, Nov 7, 2017 at 12:57 PM Jonathan Widjaja ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In packages/salesforcedx-vscode-core/test/commands/forceProjectCreate.test.ts <#197 (comment)> : > + inputBoxSpy.onCall(1).returns(''); + inputBoxSpy.onCall(2).returns(PROJECT_NAME); + }); + + after(() => { + inputBoxSpy.restore(); + }); + + it('Should return cancel if project name is undefined', async () => { + const gatherer = new SelectProjectName(); + const response = await gatherer.gather(); + expect(inputBoxSpy.calledOnce).to.be.true; + expect(response.type).to.equal('CANCEL'); + }); + + it('Should return cancel if user input is empty string', async () => { I see, so force-language-services allows it. It could be by-design that we don't trim the name; @GaryFNC <https://github.com/garyfnc> can offer his thoughts on that. If anything is going to be fixed, it should go into force-language-services . — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#197 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AE-dc7GjISyFAH-Vtg1t1zqFs2uZgGZXks5s0MRTgaJpZM4QMADG> .

-- -- *Josh Kaplan* | Senior Director, Product Management | salesforce.com | Phone: 415.836.3623

Sweetman · 2017-11-08T23:42:40Z

@kaplanjosh @DatGuyJonathan I spoke to Gary about allowing whitespace. He said that he spoke with Mike Miller and others about it a while back. The conclusion was that any legitimate folder name is ok. This varies from OS to OS which is why strong validation was not implemented.

/cc @GaryFNC

DatGuyJonathan · 2017-11-10T18:22:50Z

LGTM. Let's rebase and verify appveyor build is good.

…ce project names

Sweetman requested review from vazexqi and DatGuyJonathan October 30, 2017 23:33

Sweetman commented Oct 30, 2017

View reviewed changes

ruthemmanuelle approved these changes Oct 30, 2017

View reviewed changes

Sweetman commented Oct 31, 2017

View reviewed changes

Sweetman changed the title ~~James/create project~~ [WIP] James/create project Oct 31, 2017

Sweetman force-pushed the james/createProject branch from 080c483 to 25a1618 Compare November 1, 2017 23:39

Sweetman commented Nov 1, 2017

View reviewed changes

Sweetman changed the title ~~[WIP] James/create project~~ James/create project Nov 1, 2017

Sweetman commented Nov 1, 2017

View reviewed changes

Sweetman force-pushed the james/createProject branch from 25a1618 to 52488f7 Compare November 2, 2017 19:02

Sweetman changed the title ~~James/create project~~ [WIP] James/create project Nov 2, 2017

DatGuyJonathan reviewed Nov 3, 2017

View reviewed changes

vazexqi mentioned this pull request Nov 7, 2017

Folders without sfdx-project.json files should see an option to "Initialize Salesforce DX Project" #207

Closed

Sweetman force-pushed the james/createProject branch 2 times, most recently from 8405a16 to 2ce7983 Compare November 13, 2017 19:26

Add create project command

bb6f8a8

james-sweetman added 10 commits November 13, 2017 16:05

Allow command to run outside of sfdx project, add overwrite prompt

d7be543

Make glob explicitly search for folders

e5215d1

Make CONFIG_FILE default to top level sfdx-config.json

9d292cd

Add tests for project create

7c513f6

Add more tests

ce02dab

Fix some tests

f950acd

Uncomment tests

a698618

Add more expects for quick open elements

227edb0

Remove hardcoded paths, use fs.existsSync, no validation for whitespa…

a6a8195

…ce project names

getQuickOpenElementsText returns element instead of array if size is 1

3f52dc4

Sweetman force-pushed the james/createProject branch from 2ce7983 to 3f52dc4 Compare November 14, 2017 00:05

Sweetman merged commit 23cfb4f into develop Nov 14, 2017

Sweetman deleted the james/createProject branch November 14, 2017 00:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] James/create project #197

[WIP] James/create project #197

Sweetman commented Oct 30, 2017

Sweetman Oct 30, 2017

Sweetman Oct 30, 2017

Sweetman Oct 30, 2017

Sweetman Oct 30, 2017

Sweetman Oct 30, 2017

Sweetman Oct 30, 2017

ruthemmanuelle left a comment

codecov bot commented Oct 30, 2017 •

edited

Loading

Sweetman Oct 31, 2017

Sweetman Oct 31, 2017

Sweetman Nov 1, 2017

Sweetman Nov 1, 2017

ruthemmanuelle Nov 2, 2017

DatGuyJonathan Nov 3, 2017

DatGuyJonathan Nov 3, 2017

DatGuyJonathan Nov 3, 2017

DatGuyJonathan Nov 3, 2017

Sweetman Nov 7, 2017

DatGuyJonathan Nov 7, 2017

GaryFNC Nov 9, 2017 •

edited

Loading

kaplanjosh commented Nov 7, 2017 via email

Sweetman commented Nov 8, 2017 •

edited

Loading

DatGuyJonathan commented Nov 10, 2017

[WIP] James/create project #197

[WIP] James/create project #197

Conversation

Sweetman commented Oct 30, 2017

What does this PR do?

What issues does this PR fix or reference?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruthemmanuelle left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 30, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GaryFNC Nov 9, 2017 • edited Loading

Choose a reason for hiding this comment

kaplanjosh commented Nov 7, 2017 via email

Sweetman commented Nov 8, 2017 • edited Loading

DatGuyJonathan commented Nov 10, 2017

codecov bot commented Oct 30, 2017 •

edited

Loading

GaryFNC Nov 9, 2017 •

edited

Loading

Sweetman commented Nov 8, 2017 •

edited

Loading