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

chore(core): rename keyboardprocessor.h to keyman_core_api.h #9723

Conversation

mcdurdin
Copy link
Member

@mcdurdin mcdurdin commented Oct 9, 2023

Fixes #9721.

Also tweaks some documentation references to keyboardprocessor, but does not touch those relating to debian/control.

@keymanapp-test-bot skip

Fixes #9721.

Also tweaks some documentation references to keyboardprocessor, but does
not touch those relating to debian/control.
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Oct 9, 2023

@mcdurdin mcdurdin marked this pull request as ready for review October 9, 2023 11:58
@rc-swag
Copy link
Contributor

rc-swag commented Oct 10, 2023

Looks fine. I don't quite understand what is required but do we do need to make changes to the api_version.in or the name of the built library to match this comment fcitx/fcitx5-keyman#3 (comment).

@mcdurdin
Copy link
Member Author

but do we do need to make changes to the api_version.in or the name of the built library to match this comment fcitx/fcitx5-keyman#3 (comment).

@ermshiperete thoughts?

@mcdurdin mcdurdin merged commit a3bcc8b into master Oct 10, 2023
17 checks passed
@mcdurdin mcdurdin deleted the chore/core/9721-rename-keyboardprocessor.h-to-keyman_core_api.h branch October 10, 2023 00:57
@ermshiperete
Copy link
Contributor

ermshiperete commented Oct 10, 2023

The names in linux/debian/libkmnkbp0-0.symbols should not have been renamed - that's the safety net that should trigger when we modify the API in ways that require a version change. Also, that file shows that we missed one: kbp_state_get_intermediate_context.

We should rename the library to libkmncore (or libkeymancore?) and probably also change the version.

#9733

@mcdurdin
Copy link
Member Author

oops okay! Do you want to revert that specific change and put in place what should be done instead?

Also, kbp_state_get_intermediate_context was missed because it had the wrong name -- it was missing km_ prefix! So that's a double-oops.

@ermshiperete
Copy link
Contributor

Ok, I'll take care of the .symbols file when I do #9733.

@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 17.0.187-alpha

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

Successfully merging this pull request may close these issues.

chore(core): rename keyboardprocessor.h to keyman_core_api.h
5 participants