izuzak / atom-pdf-view

Support for viewing PDF files in Atom.
https://atom.io/packages/pdf-view
MIT License
106 stars 30 forks source link

Add synctex option to choose pane #160

Closed sherwoodinc closed 7 years ago

sherwoodinc commented 7 years ago

This would implement what I proposed in #159.

I added the relevant option and the code using it. The "default" setting kees the current behavior.

izuzak commented 7 years ago

Thanks for opening a pull request for this, @sherwoodinc ⚡️

Unfortunately, I no longer have synctex installed on my machine so can't test this out. Could you perhaps record a GIF which demonstrates how this works and post it here? So, a GIF which shows how the behavior for opening the file changes as you modify the value of the paneToUseInSynctex config setting? 🙏 The code looks good to me, but just want to make sure it's doing what we expect it should. 😸 Thanks!

sherwoodinc commented 7 years ago

Of course! I added 4 pane options (left, right, up, bottom) which are the 4 possible options for the workspace.open(...) atom API method, and "default" which uses the current pane (and keeps the old behavior). I will record a GIF and post it.

EDIT: Recording the GIF I noticed that the panes meant by Atom as "up", "bottom", "left" and "right" are not intuitive, sometimes it opens a new pane, other times it reuses an existing one. For simple two left/right panes, it works predictably. I'm going to record that case.

I added the four options for completeness, but it could be restricted to left/right/default, or even more to a checkbox "Always use left pane"...

sherwoodinc commented 7 years ago

Screencast peek 2016-12-21 17-01

izuzak commented 7 years ago

Great! Thanks!