search

ChimeHQ / Neon

A Swift library for efficient, flexible content-based text styling
BSD 3-Clause "New" or "Revised" License
320 stars 18 forks source link

TreeSitterClient target #9

Closed krzyzanowskim closed 2 years ago

krzyzanowskim commented 2 years ago

A take to separate TreeSitterClient as a separate target and make it a dependency to Neon. Ideally, the TreeSitterClient could be a separate package

mattmassicotte commented 2 years ago

Thank you for putting this together!

One of the challenges with something like this is deciding which library owns which types. Do you think it makes the most sense to have both Token and TextTarget be in TreeSitterClient?

Also the platform restriction removes a lot of compatibility. I know why you did it, but I don't understand why Xcode 14 is producing a warning for this. Is it really suddenly necessary for all packages to do something like this?

krzyzanowskim commented 2 years ago

Also the platform restriction removes a lot of compatibilities

right, it should list all apple platforms. also, Xcode 14 bumps the minimum target to 10.13 šŸ˜­ and I've made Package@swift-5.7.swift to address that

krzyzanowskim commented 2 years ago

Do you think it makes the most sense to have both Token and TextTarget be in TreeSitterClient?

mattmassicotte commented 2 years ago

Ok one more thing. It looks like TreeSitterClient only ever produces an IndexSet for invalidation. I think Iā€™d prefer, then, to define it that way and move TextTarget back into Neon. So for being difficult šŸ˜ž 

public var invalidationHandler: (TextTarget) -> Void

becomes

public var invalidationHandler: (IndexSet) -> Void
krzyzanowskim commented 2 years ago

makes sense. changed.

mattmassicotte commented 2 years ago

Thanks so much for doing this. Very good change!