feat(language-core): improve support for range mapping#206
feat(language-core): improve support for range mapping#206piotrtomiak wants to merge 8 commits intovolarjs:masterfrom
Conversation
piotrtomiak
force-pushed
the
overlapping-ranges
branch
from
c7b4c3d to
82eafae
Compare
| break; | ||
| } | ||
|
|
||
| for (const [mapped] of map.getGeneratedOffsets(start, filter)) { |
There was a problem hiding this comment.
filter is added to map API, to optimize iterations and disregard mappings early on
| }); | ||
|
|
||
| it('overlapping ranges', () => { | ||
| const mappings: Mapping<CodeInformation>[] = [ |
There was a problem hiding this comment.
Overlapping ranges actually work, they just need to be in separate mappings
packages/source-map/lib/sourceMap.ts
Outdated
| const memo = this.getMemoBasedOnRange(fromRange); | ||
| if (memo.offsets.length === 0) { | ||
| return; | ||
| * findMatchingStartEnd(start: number, end: number, fromRange: CodeRangeKey, filter?: (data: Data) => boolean): Generator<[start: number, end: number, Mapping<Data>]> { |
There was a problem hiding this comment.
I've encapsulated all of the range mapping logic here. The main idea is to not take the first mapping possible, but look for the one which fits the range the best, i.e. is the shortest possible.
|
|
||
| } | ||
|
|
||
| class BinarySearchStorage<Data> { |
There was a problem hiding this comment.
I've originally had the interval storage here, which turned out to be inefficient and not really needed anyway, so I kept the logic in the separate class.
| const fromLength = fromLengths[mid]; | ||
|
|
||
| if (offset >= fromOffset && offset <= fromOffset + fromLength) { | ||
| yield [mapping, mid] |
There was a problem hiding this comment.
The main difference is that we don't return offset, but the mapping with range index. This allows us to find ranges which contain both start an end and prefer them.
| } | ||
| } | ||
|
|
||
| export function areRangesSortedAndNonOverlapping(offsets: number[], lenghts: number[]): boolean { |
There was a problem hiding this comment.
A little bit stricter check - no overlapping allowed within a particular mapping
| const generateEnd = toGeneratedOffset(language, serviceScript, sourceScript, end, isFormattingEnabled); | ||
| if (generateStart !== undefined && generateEnd !== undefined) { | ||
| const edits = getFormattingEditsForRange(targetScript.id, generateStart, generateEnd, options); | ||
| const generatedRange = toGeneratedRange(language, serviceScript, sourceScript, { pos: start, end }, isCodeActionsEnabled); |
| else { | ||
| for (const [generatedStart, generatedEnd] of toGeneratedRanges(language, serviceScript, sourceScript, positionOrRange.pos, positionOrRange.end, isCodeActionsEnabled)) { | ||
| return getApplicableRefactors(targetScript.id, { pos: generatedStart, end: generatedEnd }, preferences, triggerReason, kind, includeInteractiveActions); | ||
| for (const generatedRange of toGeneratedRanges(language, serviceScript, sourceScript, positionOrRange, isCodeActionsEnabled)) { |
There was a problem hiding this comment.
I've changed API of toGenerateRanges to return a range object
Co-Authored-By: Piotr Tomiak <piotr.tomiak@gmail.com>
Co-Authored-By: Piotr Tomiak <piotr.tomiak@gmail.com>
piotrtomiak
force-pushed
the
overlapping-ranges
branch
from
3a025d5 to
3dc48f0
Compare
…allback capability
|
Hi! I've added some refinements to the startEnd matching logic. It is more readable now and also it supports the fallback mode out-of-the-box. |
Co-Authored-By: Piotr Tomiak <piotr.tomiak@gmail.com>
|
Since overlapping mapping handling is Angular-specific behavior, I think it's best to keep this implementation in WebStorm-Angular and keep the default implementation simple. #207 Added a method to make mapping logic customizable, can you check if it meets your needs? |
|
@johnsoncodehk - sounds good! I've copy/paste my implementation of source map to angular plugin and it works fine! Thanks for the changes! |
2 participants
Fixes #203