Skip to content

Fix Import-EditorCommand bug #1114

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

Merged
merged 2 commits into from
Nov 26, 2019
Merged

Fix Import-EditorCommand bug #1114

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
47a359a
Fix extension service field name
sk82jack Nov 24, 2019
c1678e3
Get editor commands using API instead of reflection
sk82jack Nov 25, 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
2 changes: 1 addition & 1 deletion module/PowerShellEditorServices/Commands/Public/Import-EditorCommand.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ function Import-EditorCommand {
}
$flags = [Reflection.BindingFlags]'Instance, NonPublic'
$extensionService = $psEditor.GetType().
GetField('extensionService', $flags).
GetField('_extensionService', $flags).
GetValue($psEditor)

$editorCommands = $extensionService.GetType().
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually you can replace all of these with $editorCommands = $psEditor.GetCommands(). That API didn't exist at the time, but now that it does that should definitely be used instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah that looks a lot nicer. I can change it to that but I'm not sure about the correct procedure in terms of not cluttering up the commit history - should I do an interactive rebase to clean up or leave it up to the maintainer to squash & merge?

Copy link
Collaborator

@SeeminglyScience SeeminglyScience Nov 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

leave it up to the maintainer to squash & merge?

That one :)

Also taking a closer look you'll probably want to do something like:

$editorCommands = [System.Collections.Generic.Dictionary[string, Microsoft.PowerShell.EditorServices.Services.PowerShellContext.EditorCommand]]::new(
    [System.StringComparer]::OrdinalIgnoreCase)

foreach ($existingCommand in $psEditor.GetCommands()) {
    $editorCommands.Add($existingCommand.Name, $existingCommand)
}

Optionally add some using namespace statements to clean it up a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the pointers - will get that implemented and confirm it all still works

Out of interest why do you suggest a strongly typed dictionary over a regular hashtable? Performance or just clearer intent/more control or something?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually I was just matching the type of the field, hashtable will probably work just fine and be a bit more clean. Good call :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome. Just pushed new version 🙂

Expand Down