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

dep: update libvaxis #69

Merged
merged 4 commits into from
Nov 14, 2024
Merged

dep: update libvaxis #69

merged 4 commits into from
Nov 14, 2024

Conversation

furtidev
Copy link
Contributor

@furtidev furtidev commented Nov 13, 2024

We wanted to update libvaxis on Ghostty but because of this bug in the Zig build system - it didn't work out (both zf and Ghostty use libvaxis as a dependency, but there are some changes to dependencies in libvaxis itself, zigimg and libxev aren't lazy dependencies anymore. And latest libvaxis collides with zf's older one). As a workaround for now, updating libvaxis on zf is the first step. Not many changes were needed, and the TUI works as expected. So, if this isn't a bother, merging would be very helpful. Thank you! I've noticed that zf only used tagged libvaxis releases in the past - so it's fine if you don't want to update to a non-tagged version.

@natecraddock
Copy link
Owner

Hmm. Looks like the ubuntu tests failed

I've noticed that zf only used tagged libvaxis releases in the past - so it's fine if you don't want to update to a non-tagged version.

Thanks for being thorough! I think this is fine to use a non-tagged version

@furtidev
Copy link
Contributor Author

furtidev commented Nov 13, 2024

Hmm. Looks like the ubuntu tests failed

Yep. I'll have a look tomorrow.

@furtidev
Copy link
Contributor Author

furtidev commented Nov 14, 2024

@natecraddock The test fails mostly because in the latest version of libvaxis, a lot of the types using usize are now u16 - so the numbers used in tests can't fit in that limit. I'd like to know how you'd want the tests to be done. For example, some tests try to move the cursor to a position way way bigger than the unsigned 16bit limit. The eb.cursor value is used with vaxis, and thus it needs to be a u16.

@natecraddock
Copy link
Owner

Feel free to lower that number. I wrote those tests to confirm the cursor would be clamped to the bounds of the text. Not the best tests and I'm not super picky about how you change them

@furtidev
Copy link
Contributor Author

furtidev commented Nov 14, 2024

I had to make some changes in eb.moveCursor to deal with integer overflowing. Tests should pass now.

@natecraddock
Copy link
Owner

Thanks for this!

@natecraddock natecraddock merged commit ed99ca1 into natecraddock:main Nov 14, 2024
2 checks passed
@furtidev
Copy link
Contributor Author

Thank you!

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