Range mappings: spec text for proposal #233
Conversation
Adds a flag to Decoded Mapping Records which indicate if a mapping is a range mapping. Populate this field based on the decoded range mapping offsets.
| 1. If the result of performing ComparePositions(_last_.[[GeneratedPosition]], _mapping_.[[GeneratedPosition]]) is ~equal~, then | ||
| 1. Append _mapping_.[[OriginalPosition]] to _originalPositions_. | ||
| 1. <ins>If _mapping_.[[IsRangeMapping]] is true, then</ins> |
There was a problem hiding this comment.
(copy-pasting my message from the other PR, to make it more visible)
If we are looking at the mappings at e here:
abcdefg
and we have none but we have two mappings right before at c, out of which one is the range mappings, maybe we should exclude the non-range one? Since the range one actually ends up being an exact match.
Not exactly sure though.
spec.emu
Outdated
| <td>a non-negative integer</td> | ||
| </tr> | ||
| <tr> | ||
| <td><ins>[[MappingIndex]]</ins></td> |
There was a problem hiding this comment.
Nit: can we call this something with "range mapping"?
| <emu-alg> | ||
| 1. Let _relativeIndex_ be the VLQUnsignedValue of |Vlq|. | ||
| 1. If _relativeIndex_ is 0, then | ||
| 1. Optionally report an error. |
There was a problem hiding this comment.
Does this mean the first mapping can never be a range mapping?
"mappings": "AAAA",
"rangeMappings": "A"There was a problem hiding this comment.
It can be, because the indices are 1-based and the index counter starts at 0. So specifying B gives you the first mapping in this encoding.
| 1. If _relativeIndex_ is 0, then | ||
| 1. Optionally report an error. | ||
| 1. Set _state_.[[MappingIndex]] to _state_.[[MappingIndex]] + _relativeIndex_. | ||
| 1. Let _rangeMappingOffset_ be a new Decoded Range Mapping Offset Record { [[GeneratedLine]]: _state_.[[GeneratedLine]], [[MappingIndex]]: _state_.[[MappingIndex]] - 1 }. |
There was a problem hiding this comment.
Why are we dealing with 1-based indexes?
There was a problem hiding this comment.
WIth 1-based indices the error check is pretty simpler, and it's just to reject 0. In the future we could re-use the 0 value for something if there's a need.
If it were 0-based, we would need to error or no-op if a 0 shows up but it's not the first value in the line.
* Address some quick fixes from review * Move null check into decode operation * Return an empty List on null range mappings
* Refactor range mapping decoding based on feedback * Range mapping decoding happens separate of and after mapping decoding now * Mappings are updated in-place to add range mapping info * Continue if mapping is NOT-FOUND in range mapping update
This is the actual PR for the draft range mappings proposal for detailed review. Comments and feedback welcome.
On the other PR #232, Nic raised some points about what we should do in case there are multiple mappings in one location and only one (or a subset) are range mappings.