Skip to content

Add GetSubTextFromRange to ISourceText - #15979

Merged
T-Gro merged 11 commits into
dotnet:mainfrom
nojaf:text-by-range
Oct 9, 2023
Merged

Add GetSubTextFromRange to ISourceText#15979
T-Gro merged 11 commits into
dotnet:mainfrom
nojaf:text-by-range

Conversation

@nojaf

Copy link
Copy Markdown
Contributor

In Fantomas, we have an extension for ISourceText to grab the text based on a range.

I find myself copying this over to other projects that consume FCS quite often, and I would like to have this in FCS.

@nojaf
nojaf requested a review from a team as a code ownerSeptember 14, 2023 14:33
Comment threadsrc/Compiler/Facilities/prim-lexing.fs
Comment threadsrc/Compiler/Facilities/prim-lexing.fs Outdated
Comment threadsrc/Compiler/Facilities/prim-lexing.fs
Comment threadtests/benchmarks/FCSBenchmarks/CompilerServiceBenchmarks/SourceText.fs Outdated
Comment threadvsintegration/src/FSharp.Editor/Common/Extensions.fs Outdated
Comment threadsrc/Compiler/Facilities/prim-lexing.fs Outdated
Comment threadsrc/Compiler/Facilities/prim-lexing.fs Outdated
@psfinaki

Copy link
Copy Markdown
Contributor

I'd say this can go in - but just trying to understand, can this be redesigned to actually not have duplicate implementations?

@nojaf

Copy link
Copy Markdown
ContributorAuthor

I guess, any suggestions on where to extract the code?

@psfinakipsfinaki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess at least this part

else(letsourceText= this :> ISourceText)letstartLine= range.StartLine -1letline= sourceText.GetLineString startLine
...

can be moved to a method in SourceText and then reused elsewhere.

@nojaf

Copy link
Copy Markdown
ContributorAuthor

The module SourceText is defined underneath the type.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants

@nojaf@psfinaki@vzarytovskii@kerams@T-Gro