Make Set-ScriptExtent not slow
#1981
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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() |
Collaborator
Author
There was a problem hiding this comment.
Before this point is all style fixes
| # 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. | ||
| end { | ||
| $needsIndentFix = $false |
Collaborator
Author
There was a problem hiding this comment.
Defensively assigning to avoid potentially getting a variable with the same name from a parent scope
Comment on lines
-39
to
-40
| # 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. |
Collaborator
Author
There was a problem hiding this comment.
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) |
Collaborator
Author
There was a problem hiding this comment.
This is just formatting. The rest of this should probably be revisited at some point as well, but not actively bad afaik
Hrxn
reviewed
Jan 19, 2023
| [Parameter(Mandatory, ParameterSetName='AsString')] | ||
| [Parameter(Mandatory, ParameterSetName = 'AsString')] | ||
| [switch] | ||
| $AsString, |
There was a problem hiding this comment.
Is it intentional that $AsString, is still kept on its own line here?
Collaborator
Author
There was a problem hiding this comment.
Nah not intentional, just missed it
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.