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

Make push-to-talk work at every X11 platform. #1328

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

Make push-to-talk work at every X11 platform. #1328

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Feb 21, 2019

DragonFly BSD changed location for evdev input.h file, FreeBSD has different location from the beginning. Note that DragonFly BSD is compiled without evdev support by default, but it still has that header.


This change is Reviewable

static bool linux_check_ptt(void) {
#ifdef HAVE_EVDEV
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like inline ifdefs

Plus, it's not linux_check_ptt if it's running on bsd

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, I forgot about renaming the function

src/xlib/main.c Outdated
/* First, we try for direct access to the keyboard. */
if (ppt_keyboard_handle())
return true;
else if (!ppt_keyboard_handle_unknown)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1TBS

Needs { }

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, done

Copy link
Member

@jhert0 jhert0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 2 files at r1, 1 of 1 files at r3.
Reviewable status: 2 files need 1 more reviewer, 2 open discussions remaining, missing Admin LGTM at current revision.

@ghost
Copy link
Author

ghost commented May 21, 2019

@GrayHatter please remind me the link to branch, I can't find it.

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.

None yet

3 participants