Proposal : TextViewSystemInterface to class

[kaunteya]

https://github.com/ChimeHQ/Neon/blob/4bc0fe43c46e2c08cf522d5db86666086b6e641e/Sources/Neon/TextViewSystemInterface.swift#L15

Hi. Will it make sense to convert TextViewSystemInterface to class and mark textView as a weak property?

[kaunteya]

public class TextViewSystemInterface {
    public typealias AttributeProvider = (Token) -> [NSAttributedString.Key: Any]?

    weak var textView: TextView!

After above changes, the code compiles, but not sure what the side effects would be

[mattmassicotte]

(Hello!!)

This type really just exists to connect a the pieces together. I don't think it makes sense to have this type outlive its TextView. Can you help me understand how you are using it?

[kaunteya]

I was segregating my existing code into View Controller and View model. For that, I moved my references of NSTextStorage, TreeSitterClient, Query and Highlighter to View model. Now I think as the highlighter's job is mainly associated with the view, I think I should keep it in the View controller instead of the view model.

Accessing NSTextView in the viewmodel didnt look right, hence the suggestion

[mattmassicotte]

Does your controller own the textview? If so, that's probably the same thing that should own the TextViewSystemInterface.

[kaunteya]

Got it.

I had initially kept highlighter in the ViewModel as it calls highlighterContentDidChange from ViewModel, but then moved it to Viewcontroller and made adjustments like this

class TextViewController: NSViewController {
    let textView: NSTextView
    let highlighter: Highlighter

    init(viewModel: TextViewModel) {
        textView = NSTextView(textStorage: viewModel.textStorage)
        highlighter = Highlighter(
            textInterface: TextViewSystemInterface(textView: textView, attributeProvider: viewModel.attributeProvider),
            tokenProvider: viewModel.tokenProvider
        )
        viewModel.highlighterContentDidChange = highlighter.didChangeContent
    }
}

and ViewModel

class TextViewModel: NSObject {
    let textStorage = NSTextStorage()
    let treeSitterClient: TreeSitterClient
    let query: Query

    var highlighterContentDidChange: ((NSRange, Int) -> Void)!

    override init() {
        super.init()
        textStorage.delegate = self
    }
}

extension ViewModel: NSTextStorageDelegate {
    func textStorage(_ textStorage: NSTextStorage, didProcessEditing editedMask: NSTextStorageEditActions, range editedRange: NSRange, changeInLength delta: Int) {
        highlighterContentDidChange(adjustedRange, delta)
    }
}

[mattmassicotte]

Does this arrangement work for you?

[kaunteya]

Yes. I have stripped other parts of code for brevity.

The code above demonstrates

1. Who owns what
2. How highlighter coordinates between VC and VM

[kaunteya]

Please let me know if you see any flaws there

[mattmassicotte]

It does make sense these can be difficult to manage, because this process has to sit between the view and model. But, it's hard for me to comment further. This is getting into how you want to structure your app.

But, I think I feel ok keeping TextViewSystemInterface as it is based on this! 😃