Skip to content
This repository was archived by the owner on Aug 30, 2025. It is now read-only.

Position refactor#364

Merged
scohen merged 11 commits into
mainfrom
position-refactor
Sep 14, 2023
Merged

Position refactor#364
scohen merged 11 commits into
mainfrom
position-refactor

Conversation

@scohen

@scohen scohen commented Sep 12, 2023

Copy link
Copy Markdown
Collaborator

@zachallaun and I realized that if you track the line that a position occurs on, you no longer need the document to do conversions. Watch this space.

Comment thread apps/protocol/lib/lexical/protocol/convertibles/lexical.document.range.ex Outdated
Comment thread apps/server/test/lexical/convertibles/lexical.plugin.diagnostic.result_test.exs Outdated
scohen and others added 10 commits September 13, 2023 09:26
This will help with conversions
In order to get tests passing, I wrote an assert_normalized macro that
normalizes both sides of an assertion before calling assert. This
prevents spurious failures when using PositionSupport and comparing
different positions' line and valid? fields, since building these via
PositionSupport won't set those fields properly
@scohen scohen marked this pull request as ready for review September 13, 2023 16:43

@zachallaun zachallaun left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty confident in this. As you mentioned, there may be more we can do by looking further into errors.ex, but I think these changes are a good start and we're going to be building on them imminently, so there will be more opportunity to surface any weird behavior.

@scohen

scohen commented Sep 14, 2023

Copy link
Copy Markdown
Collaborator Author

agreed, @zachallaun. I built this branch and have been using it as my main LSP for a couple days. No issues so far.

Comment thread projects/lexical_shared/lib/lexical/document/position.ex
Comment thread projects/lexical_test/lib/lexical/test/position_support.ex
Comment thread apps/protocol/lib/lexical/protocol/conversions.ex
@scohen scohen merged commit 506c3f0 into main Sep 14, 2023
@scohen scohen deleted the position-refactor branch September 14, 2023 15:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants