Enable the Code Editor, with new apply-text-edits algo. #9055
Conversation
kazcw
requested review from
PabloBuchu,
indiv0,
somebody1234,
Frizi,
farmaazon,
MichaelMauderer and
vitvakatu
as code owners
kazcw
added
the
CI: No changelog needed
app/gui2/shared/util/data/text.ts
Outdated
| while (starts[0] !== undefined) { | ||
| if (starts[0] < range[0]) { | ||
| startMap.set(starts[0], starts[0] + offset) | ||
| } else if (starts[0] <= range[1]) { | ||
| startMap.set(starts[0], range[0] + offset + insert.length) | ||
| } else { | ||
| break | ||
| } | ||
| starts.shift() | ||
| } | ||
| while (ends[0] !== undefined) { | ||
| if (ends[0] <= range[0]) { | ||
| endMap.set(ends[0], ends[0] + offset) | ||
| } else if (ends[0] <= range[1]) { | ||
| endMap.set(ends[0], range[0] + offset) | ||
| } else { | ||
| break | ||
| } | ||
| ends.shift() | ||
| } |
There was a problem hiding this comment.
Shifting arrays in a loop is unnecessarily expensive. Better to replace those with index accesses, and maintain two incrementing "read head" indices throuhgout the function.
There was a problem hiding this comment.
Hm. The typechecker doesn't seem to know that while (ends[endsIndex] !== undefined) implies that ends[endsIndex] !== undefined at the start of the loop body, so I'll use iterators.
| (ready) => { | ||
| if (ready) graphStore.moduleSource.observe(observeSourceChange) | ||
| }, | ||
| { immediate: true }, |
There was a problem hiding this comment.
If viewInitialized is always set asynchronously, then this immediate has no real effect, since ready will always be false on the first execution.
There was a problem hiding this comment.
Actually in practice projectStore.module is already non-null when the top-level watchEffect above runs, so this needs immediate to work.
| function* fieldDataEntries<Fields>(map: FixedMapView<Fields>) { | ||
| for (const entry of map.entries()) { | ||
| // All fields that are not from `AstFields` are `FieldData`. | ||
| if (!astFieldKeys.includes(entry[0] as any)) yield entry as [string, FieldData] |
There was a problem hiding this comment.
Since the key type is checked here, the return type can be narrowed.
| if (!astFieldKeys.includes(entry[0] as any)) yield entry as [string, FieldData] | |
| if (!astFieldKeys.includes(entry[0] as any)) yield entry as [keyof RawAstFields, FieldData] |
This should help further reduce casts down the line.
Also to contain those remaining (trivial in-context) type casts, a separate isAstFieldKey type asserting function could be created. Same for FieldData validation, though this one may be a bit harder to balance safety and runtime verification overhead. But given that this effectively deals with arbitrary network received data, we shouldn't just blindly trust it.
There was a problem hiding this comment.
This return the fields that are not from AstFields, so I don't see a good way to narrow it.
Runtime AST field validation would need to occur when applying updates to the module, and seems orthogonal to this new functionality--on develop we already trust the received AST fields not to cause TypeErrors.
This reverts commit c811a5a.
Add a test for some functionality introduced in #9055.
Pull Request Description
Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build.