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

remove unnecessary link with libxkbswitch #79

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thechampagne
Copy link

No description provided.

@sergei-mironov
Copy link
Owner

Hi @thechampagne . Thank you for the suggestion. Without your change, we keep binary code in the library and call it form the main application. You are suggesting to remove the linkage complexity at the price of duplicating the binary. Could you please describe the motivation behind this change? Do you have any problems with the linkage?

@thechampagne
Copy link
Author

we keep binary code in the library and call it form the main application.

Hi, I noticed that the executable is linked with libxkbswitch. I read the source code but couldn't find where it is actually called, so I thought it might be a mistaken link. Could you clarify where it is being called?

@sergei-mironov
Copy link
Owner

we keep binary code in the library and call it form the main application.

Hi, I noticed that the executable is linked with libxkbswitch. I read the source code but couldn't find where it is actually called, so I thought it might be a mistaken link. Could you clarify where it is being called?

The original version treats src/XKeyboard.cpp as a part of the library rather than of the main executable. Thus, every call to an entity defined in this file is actually a call to the library.

@thechampagne
Copy link
Author

I see. I prefer duplicating small code to minimize moving parts as much as possible. However, I understand that the library could be useful for other applications, like Vim. I can always specify BUILD_XKBSWITCH_LIB=OFF. 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