Add support for Evince with SyncTex #225
Conversation
yitzchak
changed the title
| import dbus from 'dbus-native' | ||
| import url from 'url' | ||
|
|
||
| function SyncSource (uri, point) { |
There was a problem hiding this comment.
Unless I'm missing something, this should be syncSource
There was a problem hiding this comment.
You are not. I'll fix it.
| if (callback) callback(error) | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
We really need to hide all the callback stuff behind promises. Let's move to the async future ⚡
There was a problem hiding this comment.
Fixed in latest commit.
|
|
||
| getDaemon (callback) { | ||
| if (this.daemon) return callback(null, this.daemon) | ||
| this.bus.getInterface('org.gnome.evince.Daemon', '/org/gnome/evince/Daemon', 'org.gnome.evince.Daemon', |
There was a problem hiding this comment.
I don't understand what these strings mean. Can we extract them as constants with meaningful names?
There was a problem hiding this comment.
They are the object and interface names. I'll attach constants to them.
| windowInterface.on('Closed', () => delete this.windows[texPath]) | ||
| // Hack: a small wait seems to help when the document is opened for the | ||
| // first time. | ||
| setTimeout(() => callback(null, windowInterface), 200) |
There was a problem hiding this comment.
I really don't like this. Is there some way we can avoid this? Using some kind of polling maybe? This is almost guaranteed to cause problems under heavy workloads, on a slow computer, or what not.
There was a problem hiding this comment.
I agree. I am going to remove it and see what happens.
This PR addresses #162. It is currently a bit of callback hell, but SyncTeX forward and backward are working.
Remaining issues
dbus.sessionBus()needs to be disposed of. It does not, although if we allow the user to change the opener we may need to callremoveListenerSyncSourceeven if a build has not been done yet by listening to add_signal_receiver, but I have not been able figure out how do this using dbus-native. The current functionality matches that of okular, etc. It's done withaddMatchbut it seems useless, since Evince does not start sendingSyncSourceuntil it receivesSyncView.timestampparameter toSyncViewin version 2.91.0. If we do not pass this parameter (zero works) then current versions break. Passing this parameter may cause old versions to break, so we may need to inspect the method signature. Evince 2.91.0 was released in 2011, so I think we are okay assuming thes(ii)usignature.