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

Add SyncTeX forward sync support #111

Closed nsaje closed 8 years ago

nsaje commented 8 years ago

Adds a method forwardSync to the PdfEditorView, which allows latex editors to sync the position in their tex file to the PDF view.

I added this along with an update in atom-latex package , which together make Atom a nice LaTeX editor with two-way PDF sync support.

I don't use JS often, so let me know if there's anything I could've done better. :smile:

also, closes #107

izuzak commented 8 years ago

Hey @nsaje -- thanks for the pull request. I'm a bit confused about how all of this is supposed to work. It seems that half of the implementation is here and half is in your fork of the atom-latex package (not even in the atom-latex package itself).

So, if someone installs just pdf-view, they can't really use this, even if half of the functionality is here. That doesn't sound right to me and it wouldn't really close #107 in my opinion. You shouldn't need to install other packages to use this in pdf-view. And again, it seems that the other half is only in your fork for atom-latex, not in the package itself.

Do you think it would be possible to make forward sync within this package alone?

nsaje commented 8 years ago

Sorry about it being in my fork only - I didn't want to push a PR to atom-latex that uses atom-pdf-view's functionality that doesn't exist yet.

As to why the functionality is split in two; it's because this is how synctex support is usually handled. The PDF viewer exposes the syncing functionality, but it is the editor's job to initiate the sync. This is how both atom-latex and atom-latextools handle forward syncing with other PDF viewers; they provide a command/keybinding that then syncs the editor's position to the PDF.

The way I see it atom-pdf-view should be responsible for what happens when a user has the PDF focused. What happens when the user is editing latex should be handled by latex plugins.

But this is just my opinion and if you disagree, I'll be happy to modify the PR and include for example a keybinding that, when editing a latex file, will trigger a forward sync to PDF (which might clash with latex packages' functionality), or whichever way you want it done. :smile:

izuzak commented 8 years ago

Thanks for explaining, @nsaje -- I appreciate that and I see your point.

I think I'd still like to see this implemented completely in pdf-view. I guess it comes down to this question: would it be possible and would it be useful to have this functionality even if you don't have an additional latex package installed? I think the answer to both of those questions in "yes". I shouldn't need to install a package for compiling latex documents (which is what atom-latex is for) in order to get a sync behavior between the source file and the PDF (that would be useful even if I'm not re-compiling the PDF). I'd expect that sync to work even without such a package or with any other latex package that comes along in the future.

That said, I think there's no downside in having this merged so that you could perhaps open a pull request to update atom-latex as well and move forward with this. We can always revisit the have-everything-in-pdfview idea later.

Thanks again for explaining and for your work :bow: :zap: