Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple Highlighter Support #273

Merged

Conversation

thecoolwinter
Copy link
Collaborator

@thecoolwinter thecoolwinter commented Nov 8, 2024

Description

Adds support for multiple highlight providers.

Note

For reviewers: You may notice that this uses an underscored module _RopeModule. This module is safe, as in it has tests and is used in production (it backs AttributedString and BigString Foundation types). It's underscored because the API may change in the future, and the swift-collections devs consider it to be the incorrect collection type to use for most applications. However, this application meets every requirement for using a Rope.

Related Issues

Checklist

  • I read and understood the contributing guide as well as the code of conduct
  • The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

Screenshots

N/A

@thecoolwinter thecoolwinter added the enhancement New feature or request label Nov 9, 2024
0xWDG
0xWDG previously approved these changes Nov 9, 2024
Copy link
Collaborator

@0xWDG 0xWDG left a comment

Choose a reason for hiding this comment

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

LGTM

@thecoolwinter
Copy link
Collaborator Author

thecoolwinter commented Nov 10, 2024

Small update, fixed a bug where when the visible set was updated it'd invalidate the whole visible range rather than just the new indices. Also fixes a bug where the entire document could have attributes set (and therefore layout invalidated for the entire document). Added a test to make sure this doesn't happen in the future.

tom-ludwig
tom-ludwig previously approved these changes Nov 11, 2024
Copy link
Member

@tom-ludwig tom-ludwig left a comment

Choose a reason for hiding this comment

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

Great changes! I really appreciate the good documentation and mostly clean code style!

Copy link
Member

@tom-ludwig tom-ludwig left a comment

Choose a reason for hiding this comment

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

Approved accidentally

Co-authored-by: Tom Ludwig <[email protected]>
Copy link
Contributor

@matthijseikelenboom matthijseikelenboom left a comment

Choose a reason for hiding this comment

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

LGTM, but I must say that a lot of this stuff is really above my head, using advanced Swift features and Apple API's etc

@thecoolwinter
Copy link
Collaborator Author

thecoolwinter commented Nov 14, 2024

Thanks y'all for the review work, but I'm going to have to draft this for a minute. I found a few cases of incorrect/different highlights that I think has to do with tree-sitter returning overlapping captures.

@thecoolwinter thecoolwinter marked this pull request as draft November 14, 2024 15:28
@thecoolwinter thecoolwinter marked this pull request as ready for review November 17, 2024 20:49
@thecoolwinter thecoolwinter mentioned this pull request Nov 17, 2024
6 tasks
@thecoolwinter
Copy link
Collaborator Author

Alright I've got a pull request open for the issue I mentioned, it's only a problem for this highlight system so I've made it based on this branch. Both are ready for review now

Copy link
Collaborator

@austincondiff austincondiff left a comment

Choose a reason for hiding this comment

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

Looks good!

@thecoolwinter thecoolwinter merged commit 098e648 into CodeEditApp:main Nov 18, 2024
2 checks passed
thecoolwinter added a commit that referenced this pull request Nov 21, 2024
### Description

In the highlight state, invalidates the correct range when performing an
edit.

Sometimes tree-sitter doesn't invalidate the ranges that were edited, so
we need to add that edited range in. This ensures that a non-empty range
is always invalidated.

### Related Issues

- Related pr: #273 

### Checklist

- [x] I read and understood the [contributing
guide](https://github.com/CodeEditApp/CodeEdit/blob/main/CONTRIBUTING.md)
as well as the [code of
conduct](https://github.com/CodeEditApp/CodeEdit/blob/main/CODE_OF_CONDUCT.md)
- [x] The issues this PR addresses are related to each other
- [x] My changes generate no new warnings
- [x] My code builds and runs on my machine
- [x] My changes are all related to the related issue above
- [x] I documented my code

### Screenshots


https://github.com/user-attachments/assets/9e4d9b35-1e2d-44b8-9c2e-ffbfde69c8e4

Previously:


https://github.com/user-attachments/assets/4b572c31-0320-4f90-ac52-6f911713bc75
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants