Feature: "SFDX: Open Documentation" (on web) #4414

jeffb-sfdc · 2022-09-08T23:55:47Z

What does this PR do?

New feature to open up our docs

What issues does this PR fix or reference?

#https://github.com/forcedotcom/easywriter/issues/85, @W-11683184, W-1168318r@

packages/salesforcedx-apex-debugger/src/messages/i18n.ja.ts

jeffb-sfdc · 2022-09-09T00:03:07Z

packages/salesforcedx-vscode-apex-replay-debugger/src/messages/i18n.ts

@@ -22,7 +22,7 @@ export const messages = {
  up_to_five_checkpoints:
    'You have %s of the maximum 5 enabled checkpoints. Remove or disable checkpoints until 5 or fewer are active.',
  no_enabled_checkpoints:
-    'You don\'t have any checkpoints enabled. Enable a checkpoint and try again.',
+    "You don't have any checkpoints enabled. Enable a checkpoint and try again.",


This change was auto-made when I saved.

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

gbockus-sf · 2022-09-09T15:14:47Z

packages/salesforcedx-apex-debugger/src/messages/i18n.ja.ts

@@ -13,7 +13,7 @@
 * decorations, e.g., $(x) uses the https://octicons.github.com/ and should not
 * be localized
 *
- * If ommitted, we will assume _message.
+ * If omitted, we will assume _message.


ha! that explains the 26 files...I was like 👀

gbockus-sf · 2022-09-09T15:25:25Z

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

+const functionsPath = '/functions/';
+
+const apexExtension = '.apex';
+const soqlExtension = '.soql';


The const above seem like they are reusable outside this command (esp the path/extension values). Think we could move these all to packages/salesforcedx-utils-vscode/src/constants.ts?

It turns out there are 11 files named, "constants.ts". There doesn't seem to be any occurrences of constants being using from other packages (like utils) so I placed the extension constants in packages/salesforcedx-vscode-core/src/constants.ts, Let me know if you think they should be in a different file.

👍 that works. we can figure out the right place for all the constants later.

gbockus-sf · 2022-09-09T15:26:19Z

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

+export const lwcDocUrl =
+  'https://developer.salesforce.com/tools/vscode/en/lwc/writing';
+export const functionsDocUrl =
+  'https://developer.salesforce.com/tools/vscode/en/functions/overview';


Should we add these to the messages files since they include the language?

@sbudhirajadoc are there localized versions of these pages?

I think only ja and en are supported looking at the site (basically en or ja in the URL). @sbudhirajadoc please correctly me if I'm wrong.

gbockus-sf · 2022-09-09T15:27:03Z

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

+
+  if (docUrl === '') {
+    docUrl = defaultDocUrl;
+  }


nice job handling the case where it doesn't get an active editor.

gbockus-sf · 2022-09-09T15:28:37Z

...ges/salesforcedx-vscode-core/test/vscode-integration/commands/forceOpenDocumentation.test.ts

+  let openExternalStub: SinonStub;
+
+  beforeEach(() => {
+    sb = createSandbox();


This should go outside the before each I think https://sinonjs.org/releases/latest/sandbox/

That's not what we have in our code. Most(and I thought all) of our test code have the createSandbox() call in the beforeEach(). Searching the codebase, I found...
packages/salesforcedx-utils-vscode/test/unit/commands/channelService.test.ts
packages/salesforcedx-utils-vscode/test/unit/commands/commandletExecutor.test.ts
packages/salesforcedx-utils-vscode/test/unit/helpers/traceFlags.test.ts
.
.
.

There are some with it outside the beforeEach(), like in packages/salesforcedx-utils-vscode/test/unit/helpers/paths.test.ts, but the majority of our code has it in there.

Yup, I think it's wrong in most of our code :)

gbockus-sf · 2022-09-09T15:33:04Z

...ges/salesforcedx-vscode-core/test/vscode-integration/commands/forceOpenDocumentation.test.ts

+    expect(openExternalStub.getCall(0).args[0].toString()).to.equal(
+      defaultDocUrl
+    );
+  });


nice tests. Are there any failure conditions we can verify? What happens if vscode.env.openExternal throws?

I wasn't able to think of any failure conditions, and I haven't found cases where openExternal fails.

It's a corner case, but what if the user doesn't have a default browser? It's usually good to know the behaviour (via a unit test) even if we don't expect failure. It documents what we expect to happen. Like if we can't open a browser we should probably be outputting to the user that something went wrong verse just silently doing nothing.

This is something we get automatically from VSC, and I think writing a unit test for it is writing a test validating VS Code, and doesn't validate our code. As an example, I ran this:

export async function forceOpenDocumentation() { let docUrl = ''; const editor = vscode.window.activeTextEditor; if (editor) { const filePath = editor.document.fileName; const extension = path.extname(filePath); throw Error('OK dont do that'); . . .

And the user sees this:

...so we get the error notification "for free" from VS Code.

cool. The main thing I'd have expected in the unit test would be validating that the error thrown by the called function is what is get thrown from our calling code. Granted there's not error handling here so we don't have anything extra to verify so I'm cool with just calling it a day. Error conditions is just always something to consider in unit tests.

gbockus-sf

a few comments to consider. I can QE after you check em out.

…/salesforcedx-vscode into jb/open-documentation-on-web

gbockus-sf · 2022-09-09T19:30:47Z

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

+const auraPath = '/force-app/main/default/aura/';
+const apexClassesPath = '/force-app/main/default/classes/';
+const lwcPath = '/force-app/main/default/lwc/';
+const functionsPath = '/functions/';


nit: FWIW I would have also put these in the constants file. Same deal where any time we need the auraPath we should always use the same value across the codebase. First other place it could be used is in the unit tests to build the stub responses.

I'll go and move them

gbockus-sf

Thanks for the updates...started QE

gbockus-sf

Command run: SFDX: Open Documentation

✅ Open Apex section from Apex code
✅ Aura from an Aura Component
✅ Main site from a random non-SF file
✅ Soql doc from soql builder
✅ Functions
✅ Lightning

…/salesforcedx-vscode into jb/open-documentation-on-web

jeffb-sfdc added 3 commits September 8, 2022 16:43

feat: added new SFDX: Open documentation command

550d263

feat: added new SFDX: Open documentation command

81ef97c

feat: added new SFDX: Open documentation command

db91028

jeffb-sfdc requested a review from a team as a code owner September 8, 2022 23:55

jeffb-sfdc requested a review from randi274 September 8, 2022 23:55

feat: added new SFDX: Open documentation command

5e57223

jeffb-sfdc commented Sep 9, 2022

View reviewed changes

packages/salesforcedx-apex-debugger/src/messages/i18n.ja.ts Show resolved Hide resolved

jeffb-sfdc commented Sep 9, 2022

View reviewed changes

packages/salesforcedx-vscode-core/package.nls.json Show resolved Hide resolved

Merge branch 'develop' into jb/open-documentation-on-web

9678251

gbockus-sf reviewed Sep 9, 2022

View reviewed changes

jeffb-sfdc added 4 commits September 9, 2022 11:37

feat: added new SFDX: Open documentation command

49d99c2

feat: added new SFDX: Open documentation command

c709636

Merge branch 'jb/open-documentation-on-web' of github.com:forcedotcom…

19d8167

…/salesforcedx-vscode into jb/open-documentation-on-web

feat: added new SFDX: Open documentation command

8000e6a

gbockus-sf reviewed Sep 9, 2022

View reviewed changes

gbockus-sf approved these changes Sep 9, 2022

View reviewed changes

jeffb-sfdc and others added 4 commits September 9, 2022 14:22

feat: added new SFDX: Open documentation command

4f6a5e9

Merge branch 'develop' into jb/open-documentation-on-web

e71e3b3

Merge branch 'jb/open-documentation-on-web' of github.com:forcedotcom…

4bbce39

…/salesforcedx-vscode into jb/open-documentation-on-web

feat: added new SFDX: Open documentation command

a8f63f6

jeffb-sfdc merged commit 20f77fa into develop Sep 9, 2022

jeffb-sfdc deleted the jb/open-documentation-on-web branch September 9, 2022 22:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: "SFDX: Open Documentation" (on web) #4414

Feature: "SFDX: Open Documentation" (on web) #4414

jeffb-sfdc commented Sep 8, 2022

jeffb-sfdc Sep 9, 2022

gbockus-sf Sep 9, 2022

gbockus-sf Sep 9, 2022

jeffb-sfdc Sep 9, 2022

gbockus-sf Sep 9, 2022

gbockus-sf Sep 9, 2022

jeffb-sfdc Sep 9, 2022 •

edited

Loading

gbockus-sf Sep 9, 2022

jeffb-sfdc Sep 9, 2022

gbockus-sf Sep 9, 2022

gbockus-sf Sep 9, 2022

jeffb-sfdc Sep 9, 2022

gbockus-sf Sep 9, 2022

jeffb-sfdc Sep 9, 2022

gbockus-sf Sep 9, 2022

jeffb-sfdc Sep 9, 2022 •

edited

Loading

gbockus-sf Sep 9, 2022

jeffb-sfdc Sep 9, 2022 •

edited

Loading

gbockus-sf Sep 9, 2022

gbockus-sf left a comment

gbockus-sf Sep 9, 2022

jeffb-sfdc Sep 9, 2022

gbockus-sf left a comment

gbockus-sf left a comment

Feature: "SFDX: Open Documentation" (on web) #4414

Feature: "SFDX: Open Documentation" (on web) #4414

Conversation

jeffb-sfdc commented Sep 8, 2022

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

jeffb-sfdc Sep 9, 2022 • 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

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

jeffb-sfdc Sep 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeffb-sfdc Sep 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gbockus-sf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gbockus-sf left a comment

Choose a reason for hiding this comment

gbockus-sf left a comment

Choose a reason for hiding this comment

jeffb-sfdc Sep 9, 2022 •

edited

Loading

jeffb-sfdc Sep 9, 2022 •

edited

Loading

jeffb-sfdc Sep 9, 2022 •

edited

Loading