Add command to toggle display of binary data

[Alhadis]

This PR adds two enhancements:

• A toggle-binary-view command to temporarily display the raw binary data (fixes #184).

• A package setting to customise which file extensions are to be opened as PDFs. I've added ai to the default list, because Adobe Illustrator documents are essentially just PDF files with extra metadata thrown in.

If there's anything I've missed, please let me know.

[izuzak]

Hey, @Alhadis 👋 Wow, thanks so much for this pull request! 😍 There's quite a few changes here, some of which look like refactoring? 💭

Anyway -- I just tried to play with this, and it was working great up until I tried to zoom in when viewing a PDF. That triggered an exception:

Uncaught ReferenceError: hasFocus is not defined
/Users/izuzak/github/atom-pdf-view/lib/pdf-editor-view.js:126

ReferenceError: hasFocus is not defined
    at HTMLElement.pdfViewZoomIn (/Users/izuzak/github/atom-pdf-view/lib/pdf-editor-view.js:126:13)
    at CommandRegistry.handleCommandEvent (/Applications/Atom.app/Contents/Resources/app/src/command-registry.js:384:49)
    at KeymapManager.module.exports.KeymapManager.dispatchCommandEvent (/Applications/Atom.app/Contents/Resources/app/node_modules/atom-keymap/lib/keymap-manager.js:621:22)
    at KeymapManager.module.exports.KeymapManager.handleKeyboardEvent (/Applications/Atom.app/Contents/Resources/app/node_modules/atom-keymap/lib/keymap-manager.js:412:28)
    at WindowEventHandler.handleDocumentKeyEvent (/Applications/Atom.app/Contents/Resources/app/src/window-event-handler.js:110:40)

If you're interested in fixing that, I'd be happy to take another look 👍

[Alhadis]

Anyway -- I just tried to play with this, and it was working great up until I tried to zoom in when viewing a PDF. That triggered an exception:

Oops. Fixed. 😅

That was originally defined as const hasFocus = () => …, and made a class method late into the PR. Silly me forgot to update. 😓

There's quite a few changes here, some of which look like refactoring? 💭

Do you mean the hasFocus helper? If so, I'm sorry – seeing the same code repeated six times was screaming at me to make it a meaningful helper function, for obvious maintainability reasons.

Otherwise, I made every effort to change only what was needed.

[izuzak]

Thanks for the fix, @Alhadis! :bow: Works fine now

Do you mean the hasFocus helper? If so, I'm sorry – seeing the same code repeated six times was screaming at me to make it a meaningful helper function, for obvious maintainability reasons.

I noticed how the config part was moved to package.json as well. But not a problem 😄

Happy to merge this -- thanks again! ⚡️

[Alhadis]

My pleasure! :D Thank you for taking the time to review!

I noticed how the config part was moved to package.json as well. But not a problem

That's the canonical and recommended way of defining package-settings these days. package.json was the first place I went to add/edit settings, and I anticipate other users would do the same.

Early versions of Atom required authors to use JavaScript/CoffeeScript to define package settings (the same as you've been using), but a "formal" way to do this was then introduced. This has been the norm for quite some time: relocating the config was less about "refactoring" and more about "modernisation".

I probably should have mentioned that in the PR, but it appears to have slipped my memory. =) Sorry if this came across too pushy.

[izuzak]

Thanks for explaining, @Alhadis, and for making those updates! Definitely didn't came across as pushy -- I haven't been following Atom development closely so I appreciate these cleanups and the schooling 😄 🏫 I was just trying to understand which changes are which (refactoring vs feature work) to make sure I understand what's going on.

💙