-
Notifications
You must be signed in to change notification settings - Fork 234
Make Set-ScriptExtent
not slow
#1981
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.
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
Make Set-ScriptExtent
not slow
#1981
Conversation
When I first wrote this many moons ago I didn't know most things. Like that you can just edit in reverse and side step all positioning issues... This gets rid of the awful "wait for document update" logic and just does it properly.
) | ||
begin { | ||
$fileContext = $psEditor.GetEditorContext().CurrentFile | ||
$extentList = [System.Collections.Generic.List[[Microsoft.PowerShell.EditorServices.Extensions.FileScriptExtent, Microsoft.PowerShell.EditorServices]]]::new() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before this point is all style fixes
end { | ||
$needsIndentFix = $false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Defensively assigning to avoid potentially getting a variable with the same name from a parent scope
# Currently this kills the pipeline because we need to keep track and edit all extents for position tracking. | ||
# TODO: Consider queueing changes in a static property and adding a PassThru switch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't kill the pipeline, it just stops streaming behavior. Doesn't really need a comment.
Also the TODO
doesn't make any sense
$indentOffset = ' ' * ($aExtent.StartColumnNumber - 1) | ||
$aText = $aText -split '\r?\n' ` | ||
-join ([Environment]::NewLine + $indentOffset) | ||
$aText = $aText -split '\r?\n' -join ([Environment]::NewLine + $indentOffset) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just formatting. The rest of this should probably be revisited at some point as well, but not actively bad afaik
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work.
|
||
[Parameter(Mandatory, ParameterSetName='AsString')] | ||
[Parameter(Mandatory, ParameterSetName = 'AsString')] | ||
[switch] | ||
$AsString, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it intentional that $AsString,
is still kept on its own line here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah not intentional, just missed it
This was some of the first code I ever contributed...and it shows. It's actually really easy to solve the problem of "when editing many parts of the same document, offsets no longer match up". You just go in reverse, and then you don't have that problem anymore.
So this gets rid of the "waiting for the document" to update which takes forever.