Change Get-Help behavior to return local help when no online help available #721

corbob · 2018-08-10T14:17:36Z

Current behavior is to attempt to show online help. This returns an error when there is no online help. Would be better to return full output of online help if there is some.

Furthermore: if you try to get help for a command that doesn't exist, currently it takes 4-5 seconds and doesn't do anything. This will change that to less than a second and return an error.

Addresses some of the discussion around PowerShell/vscode-powershell#884

TylerLeonhardt

This is looking good but I have a few ideas to make it a bit more readable :)

TylerLeonhardt · 2018-08-27T17:28:40Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+            IEnumerable<object> isCommand = await editorSession.PowerShellContext.ExecuteCommand<object>(psCommand);
+            if (isCommand != null && isCommand.Count() > 0){
+                psCommand = new PSCommand();
+                psCommand.AddCommand("Get-Help");


You should be able to chain this:

psCommand.AddCommand("Get-Help") .AddArgument(helpParams) .AddCommand("Select-Object") .AddArgument("RelatedLinks")

etc...

TylerLeonhardt · 2018-08-27T17:33:35Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+            PSCommand psCommand = new PSCommand();
+            // Check if it's an actual command first. This returns near instant. Get-Help takes a good 5 seconds to return when not a command.
+            psCommand.AddCommand("Get-Command").AddArgument(helpParams);
+            IEnumerable<object> isCommand = await editorSession.PowerShellContext.ExecuteCommand<object>(psCommand);


do you think you could use something instead of object here? It should be a CmdletInfo I think

TylerLeonhardt · 2018-08-27T17:35:12Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+                IEnumerable<object> relatedLinks = await editorSession.PowerShellContext.ExecuteCommand<object>(psCommand);
+                psCommand = new PSCommand();
+                psCommand.AddCommand("Get-Help");
+                psCommand.AddArgument(helpParams);


chain ALL the thingssss

TylerLeonhardt · 2018-08-27T17:39:38Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+                psCommand.AddArgument(helpParams);
+                psCommand.AddCommand("Select-Object").AddArgument("RelatedLinks");
+                IEnumerable<object> relatedLinks = await editorSession.PowerShellContext.ExecuteCommand<object>(psCommand);
+                psCommand = new PSCommand();


you could probably be a bit more descriptive here :) have a new variable instead of reusing psCommand and call it like... getHelpCommand... call the previous one getRelatedLinksCommand etc.

TylerLeonhardt · 2018-08-27T17:57:04Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+                bool sendOutput = false;
+                // Check if the first returned item converted to string is an empty object.
+                // From experience this is the return when there are no RelatedLinks.
+                if (relatedLinks != null && relatedLinks.First().ToString() != "@{RelatedLinks=}")


I think that we could probably handle this a little better.

Also something I noticed:

A module could have a related links section but Online version would be empty:

(Get-Help AdvancedHistory).RelatedLinks Online Version:

(Get-Help AdvancedHistory | Select-Object RelatedLinks) relatedLinks ------------ @{navigationLink=@{linkText=Online Version:; uri=}}

So comparing the ToString value might not be enough. You can use PSCustomObjects in C# and drill into them like a hashtable so maybe try to do that.

(Get-Help AdvancedHistory | Select-Object RelatedLinks)[0].relatedLinks.navigationLink.uri for example returns nothing. @omniomi is to blame for that module :)

another option (which I'm liking better) is to convert 95% of this logic into a PowerShell script and just use AddScript. Especially since this function doesn't return anything, this would be a good time to do it.

Funny, I asked about the related links in slack and was told that there would never be a related link without a online version. Clearly this is false, so this needs to be fixed. I like the idea of the powershell script instead. I might even still have one in the commit history (or I might have removed it...)

Drilling down a PSObject is kind of obtuse in C#, but it's still probably better to keep it in C#. I'd just make it a static helper method like this

internal static bool TryGetHelpUri(PSObject helpInfo, out string helpUri) { PSObject links = helpInfo.Properties['relatedLinks']?.Value as PSObject; if (links == null) { return false; } PSObject[] navigation = links.Properties['navigationLink']?.Value as PSObject[]; if (navigation == null) { return false; } helpUri = navigation.FirstOrDefault()?.Properties['uri']?.Value as string; return !string.IsNullOrWhiteSpace(helpUri); }

PSObject helpInfo = MethodThatInvokesGetHelp(); if (TryGetHelpUri(helpInfo, out string helpUri) { // use help uri }

Yeah that's better. 👌

corbob · 2018-09-03T13:38:17Z

I've refactored the code to use a PowerShell function to check for if the command exists instead of executing a bunch of times and then delving into a property that doesn't seem to be completely consistent.

The PowerShell function operates off of the presumption that the Get-Command and Get-Help cmdlets will error with predictable exceptions and we catch those exceptions only for our handling.

SeeminglyScience

Looking good :) just a couple changes

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

SeeminglyScience · 2018-09-04T12:58:32Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+            Get-Command $Command -ErrorAction Stop
+            $CommandExists = $true
+        } catch [System.Management.Automation.CommandNotFoundException] {
+            Write-Warning ""$Command does not exist.""


Instead of a warning, I'd prefer if it just let the exception come through.

Maybe to clean up the error message a little bit by using ThrowTerminatingError (requires [CmdletBinding()])

} catch [System.Management.Automation.CommandNotFoundException] { $PSCmdlet.ThrowTerminatingError($PSItem) return }

This is great! I was trying to do Write-Error, but it was too much information, and it output the __Check-Help function name. The only thing perhaps is that this causes an uncaught exception in the debugger. Should we be wrapping the script execution in a try catch block?

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

Also output to Host. This is an expiriment on vscode-powershell issue 884.

If not a command, Get-Help takes 4-5 seconds to return, and we will crash PSES if we don't account for the return of a 0 count.

TylerLeonhardt

Looking good. I just have a few comments :)

TylerLeonhardt · 2018-09-05T04:51:02Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

-            await editorSession.PowerShellContext.ExecuteCommand<object>(psCommand);
-
+            if (helpParams == null) { helpParams = "Get-Help"; }
+            string script = @"


You should pull this script out to a static private property so it's not created every time the function is run.

I've moved this to a static private property, not sure if positioning it at the top of the file is the best spot for it, or what the convention is for this type of scenario.

TylerLeonhardt · 2018-09-05T04:55:10Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+Try {
+    Microsoft.PowerShell.Core\Get-Command $CommandName -ErrorAction Stop | Out-Null
+} catch [System.Management.Automation.CommandNotFoundException] {
+    $PSCmdlet.ThrowTerminatingError($PSItem)


I think, based on your logic here, that this this ThrowTerminatingError is redundant if you already have ErrorAction Stop

@SeeminglyScience correct me if I'm wrong here.

in other words you might be able to ditch the try/catch all together.

I like it with the try catch because without it looks like this:

and with it looks like this:

But I will go with whichever you and @SeeminglyScience want.

@tylerl0706 the benefit of using ThrowTerminatingError over letting the exception propagate is that it changes the "context" of the error from the callee(?) to the caller.

Here's a little function you can use to check it out

function Test-TerminatingError { [CmdletBinding()] param( [Parameter(Position = 0, Mandatory)] [ValidateSet('NormalThrow', 'ThrowTerminatingError')] [string] $ThrowType ) end { if ($ThrowType -eq 'NormalThrow') { j Get-Command DefinitelyDoesntExist -ErrorAction Stop } try { Get-Command DefinitelyDoesntExist -ErrorAction Stop } catch { $PSCmdlet.ThrowTerminatingError($PSItem) } } }

If NormalThrow is specified, this is what is displayed

Get-Command : The term 'DefinitelyDoesntExist' is not recognized as the name of a cmdlet, function, script file, or operable program. Check the spelling of the name, or if a path was included, verify that the path is correct and try again. At line:11 char:13 + Get-Command DefinitelyDoesntExist -ErrorAction Stop + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + CategoryInfo : ObjectNotFound: (DefinitelyDoesntExist:String) [Get-Command], CommandNotFoundException + FullyQualifiedErrorId : CommandNotFoundException,Microsoft.PowerShell.Commands.GetCommandCommand

It shows the line in the function/script itself that failed, which is implementation detail the user doesn't care about. Contrast that to this:

Test-TerminatingError : The term 'DefinitelyDoesntExist' is not recognized as the name of a cmdlet, function, script file, or operable program. Check the spelling of the name, or if a path was included, verify that the path is correct and try again. At line:1 char:1 + Test-TerminatingError ThrowTerminatingError + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + CategoryInfo : ObjectNotFound: (DefinitelyDoesntExist:String) [Test-TerminatingError], CommandNotFoundE xception + FullyQualifiedErrorId : CommandNotFoundException,Test-TerminatingError

It also lets you fully (well mostly) customize the displayed error by creating your own ErrorRecord

All that said, @corbob that is definitely not how I expected that to look... something in our host does not handle that right...

We should still stick with it, but we should definitely look into why that's doing that.

@SeeminglyScience should we be opening an issue for "something in our host does not handle that right...
...
we should definitely look into why that's doing that." ?

SeeminglyScience

Just a few more things ;)

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

Also: * make CheckHelpScript constant * assign output of Get-Command to $null * Move CheckHelpScript to top of class.

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

TylerLeonhardt

LGTM 🎉 thanks for putting up with our comments 😄

rjmholt

LGTM!

rjmholt · 2018-09-06T04:37:58Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

@@ -251,8 +251,9 @@ protected Task Stop()
        {
            if (helpParams == null) { helpParams = "Get-Help"; }

-            PSCommand checkHelpPSCommand = new PSCommand();
-            checkHelpPSCommand.AddScript(CheckHelpScript, useLocalScope: true).AddArgument(helpParams);
+            PSCommand checkHelpPSCommand = new PSCommand()


I was about to complain about the lack of var here, but technically the type here is the return type of the method. So all I feel is ambivalence.

I used PSCommand due to all the other comments about being explicit about the variable type 😁

Wholly agree. In general you wouldn't lose explicit variable type constraints with var when the constructor is on the RHS of the assignment (var x = new Banana();), but here it's more nuanced because of a sort of Builder pattern scenario...

rjmholt · 2018-09-06T04:48:56Z

src/PowerShellEditorServices.Protocol/LanguageServer/ShowHelpRequest.cs

    {
        public static readonly
            RequestType<string, object, object, object> Type =
-            RequestType<string, object, object, object>.Create("powerShell/showOnlineHelp");
+            RequestType<string, object, object, object>.Create("powerShell/showHelp");


Just to play Devil's advocate here:

Is the reason for the breaking change just that "showOnlineHelp" would be an inaccurate name? Or does possibly getting offline help make the change breaking too?

Just thinking that there are plenty of APIs where the message name doesn't quite mean what it used to because changing the message name is a bigger breaking change than changing what the message does.

It may be that we want to break the message compatibility anyway, but we do have other clients that might benefit from us not changing the name.

Anyway, just want to ask the question: "if we change the functionality but not the message name, is this still a breaking change?" with the followup "if the message name change is the breaking change, is it worth the breakage?"

given this is a command in the command pallet and not a settings related change or something along those lines... I think if someone complains we can consider a patch that will re-ADD the old functionality.

I think what @rjmholt is talking about is breaking on both sides with the change of the message as PSES isn't used exclusively by vscode-powershell. I'm not opposed to keeping the message name the old one (in fact I think it should be a matter of just backing out a single commit on both sides...)

That sounds reasonable!

I'm game 👍

oh sure, now I need to first remove my big foot from my mouth and figure out how to rewrite the git history...

There we go, apparently it's a simple interactive rebase off of master 😀

Depending on how brave you feel:

Brave way that deletes history

git reset --hard HEAD~1

There's a safer way to do it without git revert that I can't find a good answer for.

It makes no difference to the repo, since it will all be squashed.

the interactive rebase allowed me to delete the commits that changed the wording of the message (which is partly why I made them separate commits) while still preserving the super difficult minor bug fix commit at the end there.

corbob · 2018-09-06T04:49:53Z

Last minute commit that hopefully doesn't change the approvals 😜

I was looking at the overall changes and was reminded of a scenario where I found if nothing was sent across it was actually empty string and not null (discovered while working on the Command Explorer). I believe the only way for this to happen with the Show Help is if we have a completely empty file and try, but this at least prevents an error in that instance.

TylerLeonhardt

We're watching you @corbob 😎

rjmholt · 2018-09-06T05:26:42Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

@@ -249,7 +249,7 @@ protected Task Stop()
            string helpParams,
            RequestContext<object> requestContext)
        {
-            if (helpParams == null) { helpParams = "Get-Help"; }
+            if (helpParams == "") { helpParams = "Get-Help"; }


Might be worth just making this string.IsNullOrEmpty(helpParams). We lose nothing really if the string is never null

I don't know what you're talking about... it's definitely string.IsNullOrEmpty whistles innocently

helpParams doesn't actually come over null... It comes over as empty string. Fix this to prevent a likely rare bug condition because we can.

SeeminglyScience

LGTM 😀

TylerLeonhardt

LGTM 😊

rkeithhill · 2018-09-10T03:30:17Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+    [String]$CommandName
+)
+Try {
+    $null = Microsoft.PowerShell.Core\Get-Command $CommandName -ErrorAction Stop


👍 for module prefixed commands!

rkeithhill · 2018-09-10T03:31:32Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+param (
+    [String]$CommandName
+)
+Try {


Below you use a lower case try (and everywhere else uses lower case keywords). Probably should tweak that here to be consistent. :-)

I'll get this corrected after the below comment is replied to so I'm not doing a bunch of extra work :)

rkeithhill · 2018-09-10T03:39:04Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+    [String]$CommandName
+)
+Try {
+    $null = Microsoft.PowerShell.Core\Get-Command $CommandName -ErrorAction Stop


If this script is only used in the HandleShowOnlineHelpRequest method, then it should be moved inside that method as a const string.

This seems to make sense. As was said in slack: if it's a const string it won't be stack alloc'd each time the methos is invoked. https://stackoverflow.com/questions/7616448/const-string-inside-a-function

Unless @tylerl0706 @SeeminglyScience or @rjmholt have any objections I'll move the string back into the function as const...

Reasonable to me! Let's hear what 1 other has to say

Yep no issues with that 👍

Moved the string into the function as const

(also creeped the scope a bit by handling both commands that could come in (although not from vscode-powershell...) and marking one as deprecated.

If someone calls showOnlineHelp we will write a warning that it's deprecated and then call showHelp. This should allow us to more easily update to remove the references to Online. Also: Move CheckHelpScript into the function and make it constant...

rjmholt · 2018-09-16T02:56:54Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+        )
+        {
+            const string deprecatedWarning = @"
+            Write-Warning ""'powerShell/showOnlineHelp' has been deprecated. Use 'powerShell/showHelp' instead.""";


Oooh, does this show in the Integrated Console?

It does 😃 it's due to sendOutputToHost: true

rjmholt

LGTM3!

rjmholt · 2018-09-17T21:27:25Z

src/PowerShellEditorServices.Protocol/LanguageServer/ShowOnlineHelpRequest.cs

@@ -7,10 +7,18 @@

 namespace Microsoft.PowerShell.EditorServices.Protocol.LanguageServer
 {
+    // We don't expect ShowOnlineHelpRequest to come from vscode anymore, but it could come from another editor.
+    // TODO: Note that it's deprecated if it's called???


You might be able to mark this with the [Obsolete] attribute :)

TylerLeonhardt

LGTM 🎉

TylerLeonhardt · 2018-09-18T02:23:25Z

src/PowerShellEditorServices.Protocol/LanguageServer/ShowOnlineHelpRequest.cs

    public class ShowOnlineHelpRequest
    {
        public static readonly
            RequestType<string, object, object, object> Type =
            RequestType<string, object, object, object>.Create("powerShell/showOnlineHelp");
    }
+        public class ShowHelpRequest


nitpick: the indentation of this line is a bit off

TylerLeonhardt · 2018-09-18T02:25:13Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

@@ -230,22 +231,45 @@ protected Task Stop()
                    }
                });
        }
-
-        protected async Task HandleShowOnlineHelpRequest(
+        protected async Task HandleShowHelpRequest(


nitpick: newline between functions

rkeithhill · 2018-09-18T03:12:35Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+            Write-Warning ""'powerShell/showOnlineHelp' has been deprecated. Use 'powerShell/showHelp' instead.""";
+            PSCommand commandDeprecated = new PSCommand()
+                .AddScript(deprecatedWarning);
+            await editorSession.PowerShellContext.ExecuteCommand<PSObject>(commandDeprecated, sendOutputToHost: true);


Instead of (or in addition to) a PSIC warning, we could log a warning to the log file. I could go either way on this.

~~👍 on log file~~ Actually, users won't read the log file, and it won't be relevant when looking back through the logs. We should warn the user somehow.

The ideal would be to launch a popup. But that would be a lot for this PR. Perhaps instead we can leave it as a PSIC message and open an issue to make a popup?

rkeithhill

LGTM

SeeminglyScience · 2018-09-18T11:47:04Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+        )
+        {
+            const string deprecatedWarning = @"
+            Write-Warning ""'powerShell/showOnlineHelp' has been deprecated. Use 'powerShell/showHelp' instead.""";


Instead of AddScript you could use AddCommand here instead.

pwsh.AddCommand("Microsoft.PowerShell.Utility\\Write-Verbose") .AddParameter("Message", "'powerShell/showOnlineHelp' has been deprecated. Use 'powerShell/showHelp instead.");

Using command instead allows it to create a command processor for that command specifically, instead of having to parse a script.

SeeminglyScience · 2018-09-19T12:55:49Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

            PSCommand commandDeprecated = new PSCommand()
-                .AddScript(deprecatedWarning);
+                .AddCommand("Microsoft.PowerShell.Utility\\Write-Verbose")
+                .AddParameter("Message", ";powerShell/showOnlineHelp' has been deprecated. Use 'powerShell/showHelp' instead.");


Typo here I think (from ; to ')

rjmholt

Ok this looks good to me! If you can just make the whitespace changes, we can merge this in and then the vscode one :)

rjmholt · 2018-09-20T18:54:37Z

src/PowerShellEditorServices.Protocol/LanguageServer/ShowHelpRequest.cs


 namespace Microsoft.PowerShell.EditorServices.Protocol.LanguageServer
 {
+    [Obsolete("This class is deprecated. Use ShowHelpRequest instead.")]
    public class ShowOnlineHelpRequest
    {
        public static readonly
            RequestType<string, object, object, object> Type =
            RequestType<string, object, object, object>.Create("powerShell/showOnlineHelp");
    }


Can you add a newline here?

rjmholt · 2018-09-20T18:55:05Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

@@ -122,6 +122,7 @@ public void Start()
                this.HandleDocumentRangeFormattingRequest);

            this.messageHandlers.SetRequestHandler(ShowOnlineHelpRequest.Type, this.HandleShowOnlineHelpRequest);
+            this.messageHandlers.SetRequestHandler(ShowHelpRequest.Type, this.HandleShowHelpRequest);


Maybe add newlines around this line to keep the style consistent?

rjmholt · 2018-09-20T18:55:32Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+                } catch [System.Management.Automation.PSInvalidOperationException] {
+                    Microsoft.PowerShell.Core\Get-Help $CommandName -Full
+                }";
+            if (string.IsNullOrEmpty(helpParams)) { helpParams = "Get-Help"; }


Maybe add a newline above this if

rjmholt · 2018-09-20T18:55:55Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+            PSCommand checkHelpPSCommand = new PSCommand()
+                .AddScript(CheckHelpScript, useLocalScope: true)
+                .AddArgument(helpParams);
+            await editorSession.PowerShellContext.ExecuteCommand<PSObject>(checkHelpPSCommand, sendOutputToHost: true);


I would add a newline above the first await statement

rjmholt · 2018-09-20T18:56:20Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

            await requestContext.SendResult(null);
        }
+        protected async Task HandleShowOnlineHelpRequest(


Definitely add a newline before this method definition here

rjmholt · 2018-09-20T18:56:36Z

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+            PSCommand commandDeprecated = new PSCommand()
+                .AddCommand("Microsoft.PowerShell.Utility\\Write-Verbose")
+                .AddParameter("Message", ";powerShell/showOnlineHelp' has been deprecated. Use 'powerShell/showHelp' instead.");
+            await editorSession.PowerShellContext.ExecuteCommand<PSObject>(commandDeprecated, sendOutputToHost: true);


Maybe add a newline above this await

TylerLeonhardt

Ship it 🚢

rjmholt

I'm sold!

rjmholt · 2018-09-27T17:50:12Z

This doesn't seem to be working now that it's merged in. Going to investigate today.

rjmholt · 2018-09-27T19:32:52Z

Can't reproduce the behaviour I saw earlier.

But it looks like Get-Help -Online doesn't fail if there's no internet or the help site is down.

What scenarios would trigger the offline help?

I'm trying to think of a way to identify when online help is inaccessible and fall back appropriately.

rjmholt · 2018-09-27T20:47:28Z

Spoke to @adityapatwardhan offline.

Get-Help -Online won't give us any indication if getting online help failed -- it just passes a URL to the browser and forgets. It might give an error if there's no browser, but I can't imagine a common scenario where VSCode is installed but there's no browser.

There's also no simple way to get the help URL out of Get-Help to test it ourselves.

He suggested getting local help first and falling back to online instead.

The problem is the Get-Help is designed never to throw, so you get a generated help object back instead. But, that object won't have the Description property populated. So that could be a check.

Ideally what we turn this into would be:

Ctrl+F1 gets local help first.
If description is populated, we turn the whole thing into a string (with Out-String) and send it in a message to VSCode to put in a pane.
Otherwise, we run Get-Help -Online and return null (in which case VSCode knows to do nothing).

SeeminglyScience · 2018-09-27T21:27:37Z

@rjmholt

There's also no simple way to get the help URL out of Get-Help to test it ourselves.

What about Microsoft.PowerShell.Commands.GetHelpCodeMethods.GetHelpUri(PSObject)?

Note: The PSObject required is a CommandInfo object (like from Get-Command), and it needs to be invoked from the pipeline thread.

corbob requested review from rjmholt and TylerLeonhardt as code owners August 10, 2018 14:17

corbob changed the title ~~WIP: Change Get-Help behavior to return help when no online help available~~ WIP: Change Get-Help behavior to return local help when no online help available Aug 10, 2018

corbob changed the title ~~WIP: Change Get-Help behavior to return local help when no online help available~~ Change Get-Help behavior to return local help when no online help available Aug 18, 2018

TylerLeonhardt requested review from SeeminglyScience and rkeithhill August 27, 2018 17:27

TylerLeonhardt suggested changes Aug 27, 2018

View reviewed changes

corbob mentioned this pull request Sep 3, 2018

Change "Get Online Help" menu item label: remove Online. PowerShell/vscode-powershell#1516

Merged

3 tasks

SeeminglyScience requested changes Sep 4, 2018

View reviewed changes

corbob added 6 commits September 4, 2018 20:50

Change from Online to Full

c35d13e

Also output to Host. This is an expiriment on vscode-powershell issue 884.

Added checking for RelatedLinks

75f8afb

Check if command first

55936d1

If not a command, Get-Help takes 4-5 seconds to return, and we will crash PSES if we don't account for the return of a 0 count.

Cleanup types. Output when no commands found.

6ad4e91

Refactor to a PowerShell Function

51a2ae8

Remove extraneous function declaration.

0f65dc6

TylerLeonhardt reviewed Sep 5, 2018

View reviewed changes

Move Script to private static variable

74f04ff

SeeminglyScience requested changes Sep 5, 2018

View reviewed changes

Be explicit with bool parameters

c772aa0

Also: * make CheckHelpScript constant * assign output of Get-Command to $null * Move CheckHelpScript to top of class.

rjmholt reviewed Sep 6, 2018

View reviewed changes

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs Outdated Show resolved Hide resolved

Chain all the things (checkHelpPSCommand variable)

4090e8e

TylerLeonhardt approved these changes Sep 6, 2018

View reviewed changes

rjmholt approved these changes Sep 6, 2018

View reviewed changes

rjmholt reviewed Sep 6, 2018

View reviewed changes

TylerLeonhardt approved these changes Sep 6, 2018

View reviewed changes

rjmholt reviewed Sep 6, 2018

View reviewed changes

minor bug fix

c481d02

helpParams doesn't actually come over null... It comes over as empty string. Fix this to prevent a likely rare bug condition because we can.

SeeminglyScience approved these changes Sep 6, 2018

View reviewed changes

TylerLeonhardt approved these changes Sep 6, 2018

View reviewed changes

rkeithhill reviewed Sep 10, 2018

View reviewed changes

corbob added 2 commits September 15, 2018 19:42

Try -> try

f100131

rjmholt reviewed Sep 17, 2018

View reviewed changes

rjmholt approved these changes Sep 17, 2018

View reviewed changes

TylerLeonhardt approved these changes Sep 18, 2018

View reviewed changes

rkeithhill reviewed Sep 18, 2018

View reviewed changes

rkeithhill approved these changes Sep 18, 2018

View reviewed changes

SeeminglyScience reviewed Sep 18, 2018

View reviewed changes

corbob added 2 commits September 18, 2018 20:06

Remove Online from filename

c2ba5ad

Tidy up. Resolve comments from PR.

504a5a8

SeeminglyScience reviewed Sep 19, 2018

View reviewed changes

rjmholt approved these changes Sep 20, 2018

View reviewed changes

Fix Typo. Adjust whitespace.

004d53d

TylerLeonhardt approved these changes Sep 21, 2018

View reviewed changes

rjmholt approved these changes Sep 21, 2018

View reviewed changes

rjmholt merged commit 11f4e9b into PowerShell:master Sep 21, 2018

Change Get-Help behavior to return local help when no online help available #721

Change Get-Help behavior to return local help when no online help available #721

Conversation

corbob commented Aug 10, 2018 • edited Loading

TylerLeonhardt left a 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

corbob commented Sep 3, 2018

SeeminglyScience left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TylerLeonhardt left a 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

SeeminglyScience left a comment

Choose a reason for hiding this comment

TylerLeonhardt left a comment

Choose a reason for hiding this comment

rjmholt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rjmholt Sep 6, 2018 • 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

Choose a reason for hiding this comment

corbob commented Sep 6, 2018

TylerLeonhardt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SeeminglyScience left a comment

Choose a reason for hiding this comment

TylerLeonhardt left a 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

Choose a reason for hiding this comment

rjmholt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TylerLeonhardt left a 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

rjmholt Sep 18, 2018 • edited Loading

Choose a reason for hiding this comment

rkeithhill left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

corbob commented Aug 10, 2018 •

edited

Loading

rjmholt Sep 6, 2018 •

edited

Loading

rjmholt Sep 18, 2018 •

edited

Loading