Skip to content

Get help for cmdlet #47

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
+13 −5
Draft

Get help for cmdlet #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1080ae4
Add Start of Get-Help command.
corbob Jul 9, 2019
0222a5b
Send the proper message, and it works...
corbob Jul 10, 2019
979bfd0
Get Help for the line...
corbob Jul 11, 2019
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
17 changes: 12 additions & 5 deletions src/client/extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ import { fileURLToPath, sleep } from './utils'
import { getDefaultPowerShellPath, getPlatformDetails } from './platform';
import settings = require("./settings");
import * as process from './process';
import { EvaluateRequestMessage, IEvaluateRequestArguments } from "./messages";
import { EvaluateRequestMessage, IEvaluateRequestArguments, GetHelpRequestMessage } from "./messages";

async function getCurrentSelection(mode: string) {
let doc = await workspace.document

// This doesn't actually get the selection... It gets the lines... As though the mode was V-LINE and not VISUAL...
Copy link
Member Author

Choose a reason for hiding this comment

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

@TylerLeonhardt or @yatli Was it intentional to get the lines instead of the selection? I was hoping to use getCurrentSelection for the Get-Help wrapper in that we could get the selected command, but we can't if we get the whole line.

Copy link
Member

Choose a reason for hiding this comment

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

would you like to get the word under the cursor?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well yes, but also in my mind getCurrentSelection would be to get the currently selected text when in VISUAL mode.

Given the line:

Write-Host this is a test

a selection of Write-Host would ideally return just Write-Host. As it is currently, it returns the entire line.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah unfortunately, @yatli had to hack the getCurrentSelection method to work (it works pretty well!). coc.nvim supplies a “get current selection” equivalent but I couldn’t get it to work and requires some work to get an idea for what’s going wrong:
neoclide/coc.nvim#933

Copy link
Member

Choose a reason for hiding this comment

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

maybe, we should distinguish mode "v" from "V"

Copy link
Member Author

Choose a reason for hiding this comment

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

From what I can tell there are effectively 3 Visual modes: V-LINE ("V"), V-BLOCK ("Ctrl+v"), and VISUAL ("v"). Should we distinguish between V-Block and normal Visual? I would think in V-Block mode, we'd want to get from start to end and possibly ignore that it's a block (for evaluation I can't think of a scenario where we would want to evaluate just the block).

Copy link
Member

@yatli yatli Jul 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Had a second thought and realized this:

Executing a part of something would be dangerous.
I wouldn't want someone to execute:

[[rm -rf /]]foo/bar

Line-based eval is good because it will never be a part of something (because of the line escape symbol `

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes, so for evaluation then line based makes sense. Perhaps I'll either create a get-help specific one, or add a mode specific to Get-Help. As using Get-Help you'll not want the entire line, but just the selected command.

if (mode === "v" || mode === "V") {
let [from, _ ] = await doc.buffer.mark("<")
let [to, __ ] = await doc.buffer.mark(">")
Expand Down Expand Up @@ -92,10 +93,16 @@ function startREPLProc(context: ExtensionContext, config: settings.ISettings, pw

await proc.showTerminalIfVisible();
}

let getHelp = async function(mode: string) {
const getHelpArgs: string = (await getCurrentSelection(mode)).join("");
client.sendRequest(GetHelpRequestMessage, getHelpArgs)
await proc.showTerminalIfVisible()
}

let cmdEvalLine = commands.registerCommand("powershell.evaluateLine", async () => doEval('n'));
// This isn't actually evaluating the selection.... it's evaluating the entire line....
let cmdEvalSelection = commands.registerCommand("powershell.evaluateSelection", async () => doEval('v'));
let cmdGetHelp = commands.registerCommand("powershell.getHelp", async () => getHelp('v'));
let cmdExecFile = commands.registerCommand("powershell.execute", async (...args: any[]) => {
let document = await workspace.document
if (!document || document.filetype !== 'ps1') {
Expand All @@ -119,9 +126,9 @@ function startREPLProc(context: ExtensionContext, config: settings.ISettings, pw
term.show(false)
})

// Push the disposable to the context's subscriptions so that the
// Push the disposable to the context's subscriptions so that the
// client can be deactivated on extension deactivation
context.subscriptions.push(disposable, cmdExecFile, cmdEvalLine, cmdEvalSelection);
context.subscriptions.push(disposable, cmdExecFile, cmdEvalLine, cmdEvalSelection, cmdGetHelp);

return proc.onExited
}
Expand All @@ -133,7 +140,7 @@ export async function activate(context: ExtensionContext) {

let config = settings.load()
let pwshPath = config.powerShellExePath
? this.config.powerShellExePath
? this.config.powerShellExePath
: getDefaultPowerShellPath(getPlatformDetails())

// Status bar entry showing PS version
Expand Down
1 change: 1 addition & 0 deletions src/client/messages.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ export const EvaluateRequestMessage = "evaluate";
export interface IEvaluateRequestArguments {
expression: string;
}
export const GetHelpRequestMessage = "powerShell/showHelp";