New menu for selecting output directory for "create" commands #1187

brpowell · 2019-03-24T20:27:10Z

What does this PR do?

Revises the menu for selecting an output directory for the create commands that generates templates. Users are now prompted with a list of default output options. If they want to choose a custom directory, they can select the last option of the menu to show a list of all directories in the project. For components that require their parent folder to be named something specific, it is automatically created whether the chosen output directory has that name or not.

What issues does this PR fix or reference?

W-5815448
#852 #998

brpowell · 2019-03-24T20:28:24Z

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

@@ -92,7 +88,7 @@ const fileNameGatherer = new SelectFileName();
 const lightningFilePathExistsChecker = new LightningFilePathExistsChecker();

 export async function forceLightningAppCreate(explorerDir?: any) {
-  const outputDirGatherer = new SelectStrictDirPath(explorerDir, 'aura');
+  const outputDirGatherer = new SelectOutputDir('aura', true);


Because aura bundles require the "aura" parent folder, we let the constructor know this requirement by specifying "true".

brpowell · 2019-03-24T20:29:08Z

packages/salesforcedx-vscode-core/src/sfdxProject/sfdxPackageDirectories.ts

@@ -27,7 +27,11 @@ export default class SfdxPackageDirectories {
            if (dirPath.startsWith(path.sep)) {
              dirPath = dirPath.substring(1);
            }
-            packageDirectoryPaths.push(dirPath);
+            if (packageDirectory.default) {
+              packageDirectoryPaths = [dirPath].concat(packageDirectoryPaths);


Slight modification to have the default package directory in a project be the first element of the list.

brpowell · 2019-03-24T20:29:56Z

packages/salesforcedx-vscode-lwc/src/commands/forceLightningLwcCreate.ts

-    outputDirGatherer,
-    fileNameGatherer
+    fileNameGatherer,
+    outputDirGatherer


Switched the order of these gatherers to keep the experience consistent with the other create commands.

I would expect this change to break Should create Lightning LWC on the scaffolding.test.ts I fixed a few days ago.

brpowell · 2019-03-24T20:34:12Z

@lcampos @ntotten Should we add telemetry data to measure when default folders are chosen vs. custom? I did something similar for the SObject refresh by appending the source to the command execution event - would that be sufficient?

codecov · 2019-03-24T20:49:27Z

Codecov Report

Merging #1187 into develop will decrease coverage by 0.01%.
The diff coverage is 46.73%.

@@            Coverage Diff             @@
##           develop   #1187      +/-   ##
==========================================
- Coverage    71.01%     71%   -0.02%     
==========================================
  Files          196     196              
  Lines         7379    7397      +18     
  Branches       780     781       +1     
==========================================
+ Hits          5240    5252      +12     
- Misses        1983    1989       +6     
  Partials       156     156

Impacted Files	Coverage Δ
...vscode-lwc/src/commands/forceLightningLwcCreate.ts	`0% <0%> (ø)`	⬆️
...ode-core/src/sfdxProject/sfdxPackageDirectories.ts	`94.59% <100%> (+0.3%)`	⬆️
...re/src/commands/forceVisualforceComponentCreate.ts	`42.85% <28.57%> (+0.43%)`	⬆️
...ode-core/src/commands/forceLightningEventCreate.ts	`42.85% <28.57%> (+0.43%)`	⬆️
...core/src/commands/forceLightningInterfaceCreate.ts	`42.85% <28.57%> (+0.43%)`	⬆️
...scode-core/src/commands/forceLightningAppCreate.ts	`42.85% <28.57%> (+0.43%)`	⬆️
...core/src/commands/forceLightningComponentCreate.ts	`42.85% <28.57%> (+0.43%)`	⬆️
...vscode-core/src/commands/forceApexTriggerCreate.ts	`47.22% <28.57%> (+0.16%)`	⬆️
...de-core/src/commands/forceVisualforcePageCreate.ts	`40% <33.33%> (-0.63%)`	⬇️
...x-vscode-core/src/commands/forceApexClassCreate.ts	`40% <33.33%> (-0.63%)`	⬇️
... and 1 more

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 fb1ed41...aabd51f. Read the comment docs.

sfsholden

LGTM

lcampos · 2019-03-25T18:37:21Z

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

-    this.globKeyWord = globKeyWord;
+  private typeDir: string;
+  private typeDirRequired: boolean | undefined;
+  public readonly defaultOutput = '/main/default';


I think this (use of defaultOutput's value as it is) is going to break on Windows.

Yikes good catch

lcampos · 2019-03-25T18:37:43Z

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

-      let relativePath = path.relative(srcPath, path.join(value, '/'));
-      relativePath = path.join(relativePath, '');
+  public getCustomOptions(rootPath: string): string[] {
+    return new glob.GlobSync(path.join(rootPath, '**/')).found.map(value => {


I think this is going to break on Windows, can you verify ?

@lcampos do we need a path utility in our core library maybe? This seems to be a common problem. Maybe if we prevented the use of require('path)` and used our own utility, it would cut down on these windows bugs?

No, using path is the right way to address this since its a Nodejs module and it does a good job across platforms.

lcampos · 2019-03-25T18:38:59Z

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

@@ -235,5 +235,6 @@ export const messages = {
  error_no_default_username:
    'No default org is set. Run "SFDX: Create a Default Scratch Org" or "SFDX: Authorize an Org" to set one.',
  error_no_default_devhubusername:
-    'No default Dev Hub is set. Run "SFDX: Authorize a Dev Hub" to set one.'
+    'No default Dev Hub is set. Run "SFDX: Authorize a Dev Hub" to set one.',
+  custom_output_directory: 'Select a Custom Directory'


cc @ruthemmanuelle

lcampos · 2019-03-25T18:57:58Z

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

-    if (rootPath) {
+    let packageDirs: string[] = [];
+    try {
+      packageDirs = await SfdxPackageDirectories.getPackageDirectoryPaths();


Cool handling for multiple packages.

lcampos · 2019-03-25T19:02:21Z

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

@@ -90,10 +86,7 @@ const fileNameGatherer = new SelectFileName();
 const filePathExistsChecker = new FilePathExistsChecker(APEX_TRIGGER_EXTENSION);

 export async function forceApexTriggerCreate(explorerDir?: any) {


Do we need to keep the explorerDir parameter now that's not being used ? Same question applies to the other create commands being modified.

I don't think so? I wasn't really sure what it was being used for in the first place...

lcampos · 2019-03-25T19:11:11Z

@brpowell I do think adding telemetry to see how the feature is used would help. I know we already have some to track the execution time. You can either add more details to that one or add a new log line, it's up to you.

ntotten · 2019-03-25T19:51:30Z

@lcampos @ntotten Should we add telemetry data to measure when default folders are chosen vs. custom? I did something similar for the SObject refresh by appending the source to the command execution event - would that be sufficient?

We should track overall use and maybe the result folder is worth doing - although don't track the folder name because it might contain PII, just track default vs custom. I'm not sure what it will really tell us? I would be interested in perf numbers though on how long creates take so we can figure out if any of them are slow, but i think we can do that as separate work.

ntotten · 2019-03-25T20:24:47Z

@brpowell Does the new aura extension that was just merged on friday also need this change applied to it? //cc @nrkruk @lcampos

lcampos · 2019-03-25T23:08:17Z

@brpowell Does the new aura extension that was just merged on friday also need this change applied to it? //cc @nrkruk @lcampos

These changes already cover the create commands for aura and lwc.

brpowell · 2019-03-26T18:55:49Z

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

@@ -56,7 +57,12 @@ class ForceApexClassCreateExecutor extends SfdxCommandletExecutor<
    }).execute(cancellationToken);

    execution.processExitSubject.subscribe(async data => {
-      this.logMetric(execution.command.logName, startTime);
+      const dirType = response.data.outputdir.endsWith(


These template commands badly need to be refactored to avoid having so much duplicated code. I started doing so but stuck it in another branch to separate it from this work. I'll make a story for it.

ruthemmanuelle

Thanks for the ping and the screenshot, @lcampos . I added a commit that changed "Select" to "Choose" in order to be more in line with our style guidelines. We use "select" for things like checkboxes, and "choose" when the choice is from a broader set of possibilities. Cc: @brpowell

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

…into bp/defaultTemplates

@lcampos

@lcampos or @brpowell - Please check this updated text for accuracy: Add a menu for selecting an output directory for commands that create metadata from a template; create the `default` directory, if it doesn’t exist, when running these commands ([PR #1187](#1187), [Issue #852](#852), [Issue #998](#998))

Bryan and others added 5 commits March 22, 2019 09:42

new output directory selection

75aabd2

update commands to use new selector

16baade

tests

6c362cb

Don't show default dirs if no packageDirs

8436189

remove some extra code from getPackageDirectories

732e6ca

brpowell commented Mar 24, 2019

View reviewed changes

brpowell requested review from allileong, AnanyaJha, JimSuplizio, lcampos, praksb and sfsholden March 24, 2019 20:30

Merge branch 'develop' into bp/defaultTemplates

551a6a6

Bryan added 2 commits March 24, 2019 15:24

system-test tweaks

26e1445

merge develop

7cade4e

sfsholden approved these changes Mar 25, 2019

View reviewed changes

lcampos reviewed Mar 25, 2019

View reviewed changes

os agnostic paths and logging

411d2e3

brpowell commented Mar 26, 2019

View reviewed changes

brpowell requested a review from lcampos March 26, 2019 18:56

Apply style guidelines to message

811a75c

ruthemmanuelle approved these changes Mar 27, 2019

View reviewed changes

JimSuplizio reviewed Mar 27, 2019

View reviewed changes

packages/salesforcedx-vscode-core/src/commands/forceLightningInterfaceCreate.ts Show resolved Hide resolved

JimSuplizio approved these changes Mar 27, 2019

View reviewed changes

brpowell and others added 3 commits March 27, 2019 11:31

Merge branch 'develop' into bp/defaultTemplates

face10c

Merge branch 'develop' of github.com:forcedotcom/salesforcedx-vscode …

8a7cd9c

…into bp/defaultTemplates

fix lwc system test

aabd51f

lcampos approved these changes Mar 27, 2019

View reviewed changes

brpowell merged commit e102ca8 into develop Mar 28, 2019

brpowell deleted the bp/defaultTemplates branch March 28, 2019 16:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New menu for selecting output directory for "create" commands #1187

New menu for selecting output directory for "create" commands #1187

brpowell commented Mar 24, 2019

brpowell Mar 24, 2019

brpowell Mar 24, 2019

brpowell Mar 24, 2019

lcampos Mar 27, 2019

brpowell commented Mar 24, 2019

codecov bot commented Mar 24, 2019 •

edited

Loading

sfsholden left a comment

lcampos Mar 25, 2019 •

edited

Loading

brpowell Mar 25, 2019

lcampos Mar 25, 2019

ntotten Mar 25, 2019

lcampos Mar 25, 2019

lcampos Mar 25, 2019 •

edited

Loading

lcampos Mar 25, 2019

lcampos Mar 25, 2019

brpowell Mar 25, 2019

lcampos commented Mar 25, 2019

ntotten commented Mar 25, 2019 •

edited

Loading

ntotten commented Mar 25, 2019

lcampos commented Mar 25, 2019

brpowell Mar 26, 2019

ruthemmanuelle left a comment

		@@ -90,10 +86,7 @@ const fileNameGatherer = new SelectFileName();
		const filePathExistsChecker = new FilePathExistsChecker(APEX_TRIGGER_EXTENSION);

		export async function forceApexTriggerCreate(explorerDir?: any) {

New menu for selecting output directory for "create" commands #1187

New menu for selecting output directory for "create" commands #1187

Conversation

brpowell commented Mar 24, 2019

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

brpowell commented Mar 24, 2019

codecov bot commented Mar 24, 2019 • edited Loading

Codecov Report

sfsholden left a comment

Choose a reason for hiding this comment

lcampos Mar 25, 2019 • edited Loading

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

lcampos Mar 25, 2019 • edited Loading

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

lcampos commented Mar 25, 2019

ntotten commented Mar 25, 2019 • edited Loading

ntotten commented Mar 25, 2019

lcampos commented Mar 25, 2019

Choose a reason for hiding this comment

ruthemmanuelle left a comment

Choose a reason for hiding this comment

codecov bot commented Mar 24, 2019 •

edited

Loading

lcampos Mar 25, 2019 •

edited

Loading

lcampos Mar 25, 2019 •

edited

Loading

ntotten commented Mar 25, 2019 •

edited

Loading