Skip to content

Exwm replace support #19

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.

Sign up for GitHub

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

Jump to bottom
Merged
ch11ng merged 1 commit into from
Mar 7, 2018
Merged

Exwm replace support #19

ch11ng merged 1 commit into from
Mar 7, 2018

Conversation

@medranocalvo
Copy link
Contributor

@medranocalvo medranocalvo commented Mar 1, 2018

XELB changes needed for replace-support EXWM branch.

Names are not great, please suggest better if possible. It might be useful to rename or alias xcb:+event to xcb:+add-event for similarity with add-hook and remove-hook.

@medranocalvo medranocalvo mentioned this pull request Mar 1, 2018
Merged
@ch11ng
Copy link
Owner

ch11ng commented Mar 3, 2018

It might be useful to rename or alias xcb:+event to xcb:+add-event.

The + in xcb:+event actually means 'add'. xcb:add-event would be better.

ch11ng
ch11ng requested changes Mar 3, 2018
View reviewed changes
xcb.el Outdated
;; - `xcb:+request-unchecked+reply'
;; + Event handling
;; - `xcb:+event'
;; - `xcb:+remove-event'
Copy link
Owner

@ch11ng ch11ng Mar 3, 2018

Choose a reason for hiding this comment

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

Use xcb:remove-event.

xcb.el Outdated
(cl-defmethod xcb:+remove-event ((obj xcb:connection) event listener)
"remove EVENT LISTENER
Note that event listeners attached this way are shared with the super- and sub-
Copy link
Owner

@ch11ng ch11ng Mar 3, 2018

Choose a reason for hiding this comment

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

This docstring is no longer relevant.

xcb.el Outdated
(xcb:flush obj))))

(cl-defmethod xcb:got-extension-data-p ((obj xcb:connection) namespace)
"Return t when the connection OBJ has extension data for NAMESPACE."
Copy link
Owner

@ch11ng ch11ng Mar 3, 2018

Choose a reason for hiding this comment

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

No need to convert the result to t.

@medranocalvo
Support WM_Sn atoms
2c50ed0 
* xcb-icccm.el (xcb:icccm:-atom, xcb:icccm:init): Register WM_Sn
atoms for each screen.
@medranocalvo medranocalvo force-pushed the exwm-replace-support branch from 33d529e to 6189e6b Compare March 5, 2018 16:03
@medranocalvo
Copy link
Contributor Author

medranocalvo commented Mar 5, 2018

Thanks for the review. Addressed the issues you noted.

Additionally, renamed xcb:+event to xcb:add-event and added the former as obsolete alias; let me know whether that's all right.

@ch11ng
Copy link
Owner

ch11ng commented Mar 5, 2018

Sorry but now that there is no use for xcb:remove-event (see the review on the other PR) perhaps we can just drop the changes on xcb:+event (the naming is admittedly not good but it's get used too much) and xcb:remove-event? Everything else looks fine to me.

PS. I have no idea but some of my original reviews seem get lost. This implementation of xcb:remove-event can't handle XGE and XKB events.

@medranocalvo medranocalvo force-pushed the exwm-replace-support branch from 6189e6b to 2c50ed0 Compare March 5, 2018 17:06
@medranocalvo
Copy link
Contributor Author

medranocalvo commented Mar 5, 2018

I didn't receive your comments w.r.t. XGE and XKB; you were right about it.
I think that being able to stop listening to events would be useful for XELB applications other than EXWM. Note that the old changes preserved the xcb:+event function as an alias.
I have removed those changes, along with xcb:got-extension-data-p which is not needed for replace support either.

@ch11ng ch11ng merged commit 185d266 into ch11ng:master Mar 7, 2018
tarsius pushed a commit to emacsmirror/xelb that referenced this pull request Feb 12, 2024
@Stebalien
Fix lint error from make-network-process call (ch11ng#19)
de71f3c 
The byte-compiler doesn't like the lack of a :service parameter. It
works just fine, but we might as well make it happy. It looks like
upstream Emacs source code uses `(... :family 'local :service socket)`
as well.

* xcb.el (xcb:connect-to-socket): Use :service & :family instead of
  :remote to avoid the byte-compiler warning.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ch11ng ch11ng ch11ng requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@medranocalvo @ch11ng