Alternative attempt to support rangeLength field in LSP #283
Conversation
|
I think this approach looks better. The rls-vfs change looks fine, there is no change to rls-analysis that I can see from the diffs, and the change here looks good. I guess we need the rls-vfs change to land, then I'll publish to crates.io. Then this PR will need a rebase and we can land it. For rls-span and rls-data, we should revert the len field changes and issue new versions (0.3 and 0.4) which are identical to 0.1 and 0.2. |
|
And thanks for digging in to this problem! |
|
rls-vfs v0.2.0 is now on crates.io |
Note, that rls-data has another breaking change besides len field changes, namely addition of repr(C) on Analysis. rust-dev-tools/rls-data@2f18261 If this is to remain in 0.4, dependencies in the Rust compiler will have to be updated too, and this will break compat with older compilers, though admittedly it will probably mean less subtle breakage in the future if Rust changes field ordering and such in repr(Rust). If it's not to remain, I'm not quite sure why new versions of rls-span and rls-data are even needed. Everything should just keep depending on 0.1 versions, IMO. At least there seems to be no need to publish new rls-span/rls-data to crates.io, until some actual change warranting a version bump appears. |
|
So, I plan to update the compiler + RLS with that change soon. I'd rather remove the In the short term, we should be on 0.1 of rls-data and 0.3 of rls-span, and I will upgrade use to 0.4 of rls-data later. |
albel727
force-pushed
the
rangelength-take-two
branch
from
97c4c90 to
e61a3d0
Compare
|
Rebased on top of |
|
Thank you! |
2 participants
This is another attempt to fix the issue #280.
Since the pull request #281 ran into problems due to the fact that
rls-spanandrls-datacrates are actually used as API for interaction with Rust compiler, and have an embedded version of themselves inside it, changing them doesn't seem a good idea anymore.Hence this alternative less invasive approach, which touches only
rlsandrls-vfscrates. Since it keeps usingrls-span 0.1andrls-data 0.1, it works and passes all tests locally, including fresh compilers, and so I expect Travis to confirm.I've reverted all changes in
rls-analysisandrls-vfsthat were made in preparation of #281, as they're conflicting with this attempt, and further editedrls-vfsto support this new way, which basically consists of addinglenfield to ReplaceText event ofrls<->rls-vfsinterface.I will send PRs with those changes to the nrc repos, once Travis checks this build. Meanwhile, I've pointed
Cargo.tomlto my fixed repos, and you can see what's in them here.rust-dev-tools/rls-vfs@master...albel727:master
rust-dev-tools/rls-analysis@master...albel727:master
Luckily, changes to
rls-spanandrls-dataneed not be reverted immediately, as they're depended upon not as git repos, but through crates.io. Let's leave the decision of what to do with them to nrc.