search

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

Use the first SyncTeX record on forwardSync #123

Closed ig0774 closed 8 years ago

ig0774 commented 8 years ago

I've been playing with forwardSync function in atom-pdf-view which works pretty well except that it doesn't always sync to the right page. The problem is that synctex view... produces a list of records where the first is likely to be the most accurate. Unfortunately, in the current version, each subsequent record clobbers the previous one, so sometimes forwardSync will jump to the wrong page. This PR fixes the issue by stopping parsing once we've hit the second record, ensuring that the jump is made to the first page.

Sorry for the wall of text.

Thanks for adding the forwardSync feature. It's a real boon to those of us using LaTeX in Atom.

izuzak commented 8 years ago

Thanks for opening a pull request, @ig0774 :zap:

I have to say I'm not very familiar with synctex and forward/reverse sync -- the code in this package related to that was mostly added by other contributors. Soooo, I'd like to ask @nsaje for help in reviewing this -- he added the forwardSync code a few days ago in https://github.com/izuzak/atom-pdf-view/pull/111.

@nsaje Does this look okay to you? :thought_balloon:

nsaje commented 8 years ago

Yep, this looks good. I didn't notice that synctex can return multiple records. Good job @ig0774 ! :shipit:

izuzak commented 8 years ago

Great -- thanks for taking a look, @nsaje, :bow: and thanks again for opening a pull request, @ig0774 :zap: