Merge lp:~ksamak/compiz/ezoom_focus_tracking into lp:compiz
| Status: | Needs review |
|---|---|
| Proposed branch: | lp:~ksamak/compiz/ezoom_focus_tracking |
| Merge into: | lp:compiz |
| Diff against target: |
1617 lines (+1252/-39) 17 files modified
debian/compiz-dev.install (+1/-0) debian/compiz-plugins-default.install (+2/-0) debian/control (+1/-0) plugins/ezoom/CMakeLists.txt (+1/-1) plugins/ezoom/ezoom.xml.in (+35/-8) plugins/ezoom/src/ezoom.cpp (+123/-28) plugins/ezoom/src/ezoom.h (+22/-2) plugins/focuspoll/CMakeLists.txt (+9/-0) plugins/focuspoll/compiz-focuspoll.pc.in (+12/-0) plugins/focuspoll/focuspoll.xml.in (+34/-0) plugins/focuspoll/include/accessibilitywatcher/accessibilitywatcher.h (+82/-0) plugins/focuspoll/include/accessibilitywatcher/focusinfo.h (+76/-0) plugins/focuspoll/include/focuspoll/focuspoll.h (+50/-0) plugins/focuspoll/src/accessibilitywatcher.cpp (+506/-0) plugins/focuspoll/src/focuspoll.cpp (+220/-0) plugins/focuspoll/src/private.h (+75/-0) plugins/showmouse/showmouse.xml.in (+3/-0) |
| To merge this branch: | bzr merge lp:~ksamak/compiz/ezoom_focus_tracking |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| ksamak (community) | Resubmit on 2017-04-28 | ||
| Sam Spilsbury | code, style | 2017-04-02 | Pending |
| Marco Trevisan (TreviƱo) | 2017-04-02 | Pending | |
| Andrea Azzarone | 2017-04-02 | Pending | |
|
Review via email:
|
|||
This proposal supersedes a proposal from 2017-02-24.
Description of the Change
ezoom: added focus tracking based on at-spi a11y events.
| Andrea Azzarone (azzar1) wrote : | # |
| Andrea Azzarone (azzar1) wrote : | # |
Ok this is the first part of the review. I'll continue later.
| Sam Spilsbury (smspillaz) wrote : | # |
Thanks for all the hard work in implementing this. There's a fair bit of work to be done before this can be integrated. In particular, this cannot be merged whilst it opens a separate display connection - that is a recipe for deadlocks.
| Sam Spilsbury (smspillaz) wrote : | # |
Relevant prior art, if interested: https:/
| ksamak (ksamak) wrote : | # |
well, this plugin would indeed have been helpful.
Sam Spilsbury (smspillaz) wrote 18 hours ago:
I like the fact that this is a separate library, but this absolutely doesn't belong in the mousepoll plugin. The mouse poll plugin is supposed to do one thing and one thing only - update the position of the mouse every N milliseconds. Move all this into either a separate plugin or library.
I originally made a copy of the mousePoll plugin (called focusPoll), but for some unknown reason it... deadlocked, or froze at runtime. I couldn't find why, and no one had time to look at it, so I merged both logics in mousepoll, cause then, with sensibly the same methods, it didn't freeze.
I agree an additional plugin would serve better, so I'd appreciate some input on that problem.
there's a readyish branch on http://
question: how would you circumvent the problem of the call to XOpenDisplay, if the whole a11ywatcher was a library? Is there a better way to get the screen info? that is without causing all those problems.
I will meditate on these ideas, and weight the pros and cons of the different solutions, and i'll then continue the work.
thank you for all the advice and corrections, including those pending below. I am available to discuss these matters further, or if you have more insight.
| Sam Spilsbury (smspillaz) wrote : | # |
> Maybe you would have some insights as to what is blocking here. It
> might be, as you pointed, the call to open the display.
> question: how would you circumvent the problem of the call to XOpenDisplay, if
> the whole a11ywatcher was a library? Is there a better way to get the screen
> info? that is without causing all those problems.
Yes, there's a global variable called 'screen' and you can access the current connection from there (screen->dpy()). If your library needs a reference to it, I'd suggest having the caller pass it in.
| ksamak (ksamak) wrote : | # |
I've answered some of the following, and will superseed the merge request with the new version from the branch.
i thank you for your time, advice, and corrections.
| ksamak (ksamak) wrote : | # |
call for help on the matter of binding C callback to c++ member function.
thx
| ksamak (ksamak) wrote : | # |
just a ping, i changed coding style again, on request from 3v1n0.
| Andrea Azzarone (azzar1) wrote : | # |
I reviews part of the code. Fix it before I going on.
| ksamak (ksamak) wrote : | # |
I fixed the code according to your specifications, i changed the whole coding style of my editor, thanks to the few lines i (finally) found here:
http://
Sorry again for my misunderstanding tabs and coding style, i'll try my best to respect that from initial coding on, event though i find it less readable.
I thank you for your patience.
| ksamak (ksamak) wrote : | # |
created lp:~ksamak/compiz/correct_mousetheme_targetting_for_ezoom, although i think it wasn't necessary. I'll delete it.
I add the update for the rebranding of icedove into thunderbird.
thank you.
| Samuel thibault (samuel-thibault) wrote : | # |
Hello,
I have gone through the previous review from azzar1, and commented further.
Samuel
| Samuel thibault (samuel-thibault) wrote : | # |
I have commented on the callback issue, to avoid the singleton.
Unmerged revisions
- 4124. By ksamak <ksamak@ksalaptop> on 2017-04-28
-
added rebranding of icedove to thunderbird support
- 4123. By ksamak <ksamak@ksalaptop> on 2017-04-20
-
coding style
- 4122. By ksamak <ksamak@ksalaptop> on 2017-04-10
-
added precedence relation to start showmouse before ezoom
- 4121. By ksamak <ksamak@ksalaptop> on 2017-04-02
-
modifications of code according to azzar's request
- 4120. By ksamak <ksamak@ksalaptop> on 2017-03-14
-
coding style modifs. changed if format on 3v1n0's request
- 4119. By ksamak <ksamak@ksalaptop> on 2017-02-25
-
coding style modifs
- 4118. By ksamak <ksamak@ksalaptop> on 2017-02-25
-
coding style modifs
- 4117. By ksamak <ksamak@ksalaptop> on 2017-02-24
-
minors code modifs
- 4116. By ksamak <ksamak@ksalaptop> on 2017-02-24
-
minors code modifs
- 4115. By ksamak <ksamak@ksalaptop> on 2017-02-24
-
corrected segfault, minors code modifs


Can you please link a bug to the MP?