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

Store attributes in TextKit 1 text storage #28

Merged
merged 6 commits into from
Nov 18, 2023

Conversation

DivineDominion
Copy link
Contributor

@DivineDominion DivineDominion commented Nov 18, 2023

NSLayoutManager's temp attributes can't be fonts. So no font weight or italic text for anyone. Instead, store the attributes in the text storage directly.

Since #26 that doesn't cause any extra work:

  1. User types
  2. Text storage updates and notifies the delegate about character (typed text) + attribute changes (text view typingAttributes)
  3. Highlighting is performed
  4. Attributes are applied
  5. Text storage updates and notifies the delegate about attribute changes
  6. Delegate ignores this 👍
Before (TK1 and TK2) After (TK1)
2023-11-18 12-11-14 NeonExample - Untitled@2x 2023-11-18 12-24-35 NeonExample - Untitled@2x

This applies to TextKit 1 only.

TextKit 2 still looks like 'before'. (Out of scope with the knowledge I have)

Also the weird 'value' font appears to be a "hole" in the grammar. Nothing's applied there. I'm investigating. Works fine with the JS grammar I'm using because everything's something in JavaScript (:

@mattmassicotte
Copy link
Contributor

It is absolutely possible to make a custom interface that does this. These classes are all here just for convenience.

But, I'm not opposed to changing them. Editing like this, and the resulting re-laying, will have performance implications. But I doubt anyone that is seriously concerned about that would be using these.

Do you know what's up with the tests?

@DivineDominion
Copy link
Contributor Author

DivineDominion commented Nov 18, 2023

Fixed the tests and how to run them locally.

But I doubt anyone that is seriously concerned about that would be using these.

I'm using that approach in The Archive and it works really well for huge notes :)

Am investigating the fallback font still

@mattmassicotte
Copy link
Contributor

I'm using that approach in The Archive and it works really well for huge notes :)

Using these specific Neon classes, I meant! The technique may work just fine!

Though out of curiosity, could you define “huge” for me?

@DivineDominion
Copy link
Contributor Author

Though out of curiosity, could you define “huge” for me?

Moby Dick (1.1MB file)

@mattmassicotte
Copy link
Contributor

When I do scrolling performance testing, I typically use a 1 million line document. For large file sizes, 100MB-1GB depending on the way it is generated. But it's great to know it can work in that case! I'm not terribly worried.

@mattmassicotte mattmassicotte merged commit a369b1c into ChimeHQ:main Nov 18, 2023
2 checks passed
@mattmassicotte
Copy link
Contributor

Thank you!

@DivineDominion
Copy link
Contributor Author

1GB 🫣 Ok I'll try that, too.

@mattmassicotte
Copy link
Contributor

While I am curious, but I won't worry about it too much. Again, it only matters when using these specific integrations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants