influxdata / flux-lsp

Implementation of Language Server Protocol for the flux language
MIT License
27 stars 5 forks source link

Updates to yieldless composition, to get basic composition coordination working with UI. #615

Closed wiedld closed 1 year ago

wiedld commented 1 year ago

We have 3 bugs, and 1 feature update needed.

.

Feature: need executeCommand/setMeasurement

Need for this feature is well described here.

.

Bug: applyEdit -- is incorrect when file is empty.

When performing an executeCommand/initialize, we are getting a grossly incorrect range on the first initialize request (when nothing else exists).

Screen Shot 2022-11-21 at 2 47 00 PM

It looks like the max of a 32 bit integer, and we are asking for the default position which has u32's as nested types. The API for ast::Position indicates that it's not 0 indexed, as in the valid calls confirm that the u32s are >0.

.

Bug: applyEdit -- newline always added

Even once the previous^^ items were fixed, we still had 1 additional new line added per each applyEdit. See the vid: any change, be it adding or removing or staying the same number of lines in a composition block, resulted in 1 additional blank line being added afterwards:

https://user-images.githubusercontent.com/10232835/203417860-f01d31ec-3f9d-4078-9b1f-9b2b2c746e6d.mov

Looking at the applyEdit payload, we always have a newline character added to the end of the newText, even as the range includes the end of the previous line (assuming no newline).

Screen Shot 2022-11-22 at 4 00 39 PM

This newline makes sense on executeCommand/initialize (to have the previous code lines/composition be moved downwards below). It does not make sense on the other executeCommands with incremental changes.

.

Bug: didChange is not persisting changes made outside of composition

Once we fixed all of that^^, we still have the loss of additional transformations added outside of the composition block.

https://user-images.githubusercontent.com/10232835/203427457-39d42ca6-0ba1-41f6-90ec-51e3dd66740f.mov

Occurs with any change made in the composition block, be it calls or the range. The didChange should be updating the stateful LSP to be synced with the latest composition. Debugged by: