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

Reload file when someone tries to `open` file #204

Closed nisargjhaveri closed 6 years ago

nisargjhaveri commented 6 years ago

If the "Auto reload on update" is disabled. The file never updates. Even if someone manually tries to open a file or calls atom.workspace.open.

With this PR, when "Auto reload on update" is disabled, and someone clicks on the file on sidebar, and if the file is updated, the file should reload.

The file should also reload when some other plugin, for example latex, calls atom.workspace.open after compiling the pdf.

This fixes #161. Hope it doesn't break anything else! :P

izuzak commented 6 years ago

Hey @nisargjhaveri 👋 Thanks for another PR!

I was playing with this today and it seemed to work well and I didn't find anything major braking, so finger crossed! 🤞

I did find one case where I didn't observe behavior I expected. Here's the scenario:

  1. start Atom
  2. make sure that the auto-reload-on-update configuration setting is disabled
  3. open a PDF
  4. update the PDF on the filesystem. The PDF shouldn't auto-reload since the configuration setting is disabled
  5. enable the auto-reload-on-update configuration setting
  6. the PDF will not auto-update and clicking on the PDF in the list of files in the tree-view didn't trigger an update

Do you see the same behavior? If so, would that be something we'd like to cover as well?

Thanks again! ❤️

nisargjhaveri commented 6 years ago

Sure. I'll update the PR to cover this case.

I think, the expected behaviour should be that, as soon as auto-reload-on-update is enabled, the file should update if changed, without needing to click on tree-view or anything else. Does this sound good?

izuzak commented 6 years ago

I think, the expected behaviour should be that, as soon as auto-reload-on-update is enabled, the file should update if changed, without needing to click on tree-view or anything else. Does this sound good?

Yep, sounds perfect! 👌 Thank you 🙏

nisargjhaveri commented 6 years ago

Fixed and update the PR. Have a look?

izuzak commented 6 years ago

Looks great to me -- thanks again! Gonna merge this and see if anyone notices any problems :v:

Love your contributions, @nisargjhaveri! 👏 ❤️