Add FileContext.GetText(IFileRange) overload#2306
Merged
andyleejordan merged 1 commit intoJun 18, 2026
Merged
Conversation
`EditorContext.SelectedRange` is typed as `IFileRange` (it's actually a `BufferFileRange`), but `FileContext.GetText` and `GetTextLines` only accepted the concrete `FileRange` class. That meant the obvious script `$Context.CurrentFile.GetText($Context.SelectedRange)` failed overload resolution with "Cannot find an overload for GetText and the argument count: 1", and the documented workaround of constructing a `FileRange` isn't even reachable when that type isn't loaded. Widen both parameters from `FileRange` to `IFileRange`. The bodies already route through the `ToBufferRange()` extension defined on `IFileRange`, so nothing else changes, and `FileRange` callers keep working since it implements the interface. Adds focused regression tests covering `GetText`/`GetTextLines` with a `SelectedRange` and with a hand-built `FileRange`. Fixes #1496. Drafted by Copilot (Claude Opus 4.8). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
Software development really has changed. When this was first opened I said to the effect of "too busy to debug this, sorry" in #1496 (comment) but today it just got automatically fixed when I threw GitHub Copilot across the repo to solve issues it thinks it could handle. And yes, this was an easy fix that was a typing problem (using a concrete type instead of the interface) and it's confirmed with a correct regression test. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #1496 where $Context.CurrentFile.GetText($Context.SelectedRange) failed in PowerShell because GetText/GetTextLines only accepted the concrete FileRange class, while EditorContext.SelectedRange is typed as IFileRange (backed by the internal BufferFileRange struct). The fix widens both method parameters from FileRange to IFileRange, which is fully compatible since the method bodies already call ToBufferRange() — an extension method defined on IFileRange.
Changes:
- Widen the parameter type of
FileContext.GetTextandFileContext.GetTextLinesfromFileRangetoIFileRange. - Add a new test class
FileContextTestswith three focused regression tests coveringGetText/GetTextLineswith bothSelectedRange(the interface path) and a hand-constructedFileRange(the concrete class path).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/PowerShellEditorServices/Extensions/FileContext.cs |
Widen GetText and GetTextLines parameter types from FileRange to IFileRange |
test/PowerShellEditorServices.Test/Extensions/FileContextTests.cs |
New test class with 3 tests verifying GetText/GetTextLines work with IFileRange |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
Author
|
@SeeminglyScience @JustinGrote this is definitely worth landing, it was a dumb type mismatch, easily fixed. |
JustinGrote
approved these changes
Jun 18, 2026
JustinGrote
left a comment
JustinGrote
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, tested as working on my local instance.
andyleejordan
deleted the
andyleejordan/add-filecontext-gettext-ifilerange
branch
andyleejordan
added a commit
that referenced
this pull request
Jun 22, 2026
#2306 widened the parameters of `FileContext.GetText` and `GetTextLines` from the concrete `FileRange` class to the `IFileRange` interface so that `$Context.CurrentFile.GetText($Context.SelectedRange)` would resolve (since `SelectedRange` is typed as `IFileRange`). That fixed the script-side call, but it dropped the old concrete-typed method slots from the assembly. Downstream modules that compile against PSES, most notably SeeminglyScience/EditorServicesCommandSuite, bind to the published metadata at build time. A precompiled caller of the old `GetText(FileRange)` signature would throw `MissingMethodException` at runtime even though it recompiles cleanly, so this was a binary-breaking change. Restore `GetText(FileRange)` and `GetTextLines(FileRange)` alongside the `IFileRange` overloads. Each casts to `IFileRange` and delegates to the widened implementation, so old binaries keep resolving and behavior is unchanged. The added tests declare their argument as the concrete `FileRange` so overload resolution actually exercises the compatibility shims. Drafted by Copilot (Claude Opus 4.8). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
andyleejordan
added a commit
that referenced
this pull request
Jun 22, 2026
I missed that #2306 was binary-breaking until after it merged, so add a "Public API & Binary Compatibility" section to the Copilot instructions to catch the next one at review time. The key context the agent was missing: the public types under `Microsoft.PowerShell.EditorServices` aren't only reached through PowerShell script. Downstream modules compile against the shipped assemblies, and SeeminglyScience/EditorServicesCommandSuite in particular references the extension API surface (`FileContext`, `IFileRange`, `ILspCurrentFileContext`, `IEditorScriptFile`, ...) directly from C#. So widening or renaming an existing `public` member is source-compatible at most but binary-breaking. The section spells out the failure mode (`MissingMethodException`), the fix (add an overload that delegates rather than editing the signature in place), and the expectation to bind a regression test to the old concrete signature. Drafted by Copilot (Claude Opus 4.8). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
andyleejordan
added a commit
that referenced
this pull request
Jun 22, 2026
…mpat (#2319) * Restore concrete `FileRange` overloads on `FileContext` #2306 widened the parameters of `FileContext.GetText` and `GetTextLines` from the concrete `FileRange` class to the `IFileRange` interface so that `$Context.CurrentFile.GetText($Context.SelectedRange)` would resolve (since `SelectedRange` is typed as `IFileRange`). That fixed the script-side call, but it dropped the old concrete-typed method slots from the assembly. Downstream modules that compile against PSES, most notably SeeminglyScience/EditorServicesCommandSuite, bind to the published metadata at build time. A precompiled caller of the old `GetText(FileRange)` signature would throw `MissingMethodException` at runtime even though it recompiles cleanly, so this was a binary-breaking change. Restore `GetText(FileRange)` and `GetTextLines(FileRange)` alongside the `IFileRange` overloads. Each casts to `IFileRange` and delegates to the widened implementation, so old binaries keep resolving and behavior is unchanged. The added tests declare their argument as the concrete `FileRange` so overload resolution actually exercises the compatibility shims. Drafted by Copilot (Claude Opus 4.8). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Document public API binary-compatibility rules for Copilot I missed that #2306 was binary-breaking until after it merged, so add a "Public API & Binary Compatibility" section to the Copilot instructions to catch the next one at review time. The key context the agent was missing: the public types under `Microsoft.PowerShell.EditorServices` aren't only reached through PowerShell script. Downstream modules compile against the shipped assemblies, and SeeminglyScience/EditorServicesCommandSuite in particular references the extension API surface (`FileContext`, `IFileRange`, `ILspCurrentFileContext`, `IEditorScriptFile`, ...) directly from C#. So widening or renaming an existing `public` member is source-compatible at most but binary-breaking. The section spells out the failure mode (`MissingMethodException`), the fix (add an overload that delegates rather than editing the signature in place), and the expectation to bind a regression test to the old concrete signature. Drafted by Copilot (Claude Opus 4.8). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #1496.
EditorContext.SelectedRangeis typed asIFileRange, butFileContext.GetText/GetTextLinesonly accepted the concreteFileRangeclass. So the natural$Context.CurrentFile.GetText($Context.SelectedRange)failed overload resolution ("Cannot find an overload for GetText and the argument count: 1"), and the documented workaround of constructing aFileRangeisn't reachable when that type isn't loaded.This widens both parameters from
FileRangetoIFileRange. The bodies already route through theToBufferRange()extension onIFileRange, so nothing else changes and existingFileRangecallers keep working.Adds focused regression tests covering
GetText/GetTextLineswith aSelectedRangeand with a hand-builtFileRange.