Major library refactoring

[zoul]

See the commit logs for a changelog and rationale. I hope that the changes improve code readability and maintainability and can be merged. It would be a shame for MASShortcut to evolve into the hairy ball that became of Shortcut Recorder.

To be done:

• [x] Introduce a separate dispatcher class to bind shortcuts to actions?
• [x] Introduce a separate class to persist bindings to user defaults?
• [x] Add a convenience API to register default shortcuts.
• [x] Write code to enable backward compatibility with Shortcut Recorder?
• [x] Add a simple wrapper API atop the new classes to simplify usage?
• [x] Update documentation
• [x] Normalize whitespace

If there are any code style issues, I propose to resolve them later, after merging, preferably using an automated tool.

[zoul]

The work is more or less done. I have changed more than I expected and I realize this is a big chunk of work to review, but I hope the changes can be merged. I’m happy to assist with the review if needed.

I have just integrated the new version into our Mac App Store app that used to use Shortcut Recorder and everything seems to work fine, so that’s at least one real-world positive feedback.

[shpakovski]

@zoul Thanks a lot Tomáš, I did not have a chance to review these changes yet, but I will as soon as I can. Thanks again :)

[zoul]

Glad to help, thank you for the code in the first place. I don’t mind waiting for the code to be merged. If I can be of any further assistance, just ping me. Happy hacking!

[shpakovski]

Hi again, sorry for the late reply!

I reviewed the changes and must say that this is really solid work. However, it would break many projects that already use MASShortcut. You literally refactored all code for modern Cocoa. This is great, but I cannot merge this request for compatibility reasons.

What I think though, it makes sense to publish your project as a separate Framework option for new projects. Then we would refer each other so that people choose what suits better. What do you think?

Thanks in advance, Vadim

[zoul]

To give you a little context: We have a Mac App Store app that used to use Shortcut Recorder until recently. Last year we were doing an update and realized that the old version of Shortcut Recorder is mostly unmaintained and slowly rotting to pieces. I have found a good looking updated fork and our company sponsored a few days of my work into further refactoring, so that we could reasonably depend on Shortcut Recorder in the long run. Unfortunately my pull request was never merged, so the situation is exactly what it was before: There’s maybe a dozen of Shortcut Recorder forks, none of them having a long-term future. There’s even an official GitHub repo of Shortcut Recorder now, practically as dead as the rest of them.

Now back to MASShortcut. I don’t have the time to maintain a fork and I’d love the proposed changes to become a part of the official project, because merging our efforts improves the probability that in a year or two, MASShortcut will still exist and receive updates. (Especially given that most of my changes focus on long-term maintainability.)

Is there anything I can do to get the code merged? I fully understand your concerns about backwards compatibility, especially as someone who maintains several apps with third-party dependencies. Can you be more specific about what backwards compatibility you’d like to keep? Is there a minimum OS X target you need to support, a minimum Xcode version? Or is it API changes? If yes, it would be trivial to support the old +registerGlobalShortcutWithUserDefaultsKey API, for example. Or we could create a 2.0 release with breaking changes (but a better long-term perspective), and people who just need things to keep working could stick to a “legacy” branch that would just receive bug fixes.

Thank you, and sorry for the long post.

[shpakovski]

Thanks for sharing all the details.

Can you be more specific about what backwards compatibility you’d like to keep?

I find CocoaPods integration the most demanded feature. If we prefix all headers with Framework/, it may stop working.

I believe that 2.0 is the best option.

P. S. Can you share which apps do you use MASShortcut in? Thanks in advance!

[zoul]

CocoaPods integration should be a breeze. I have not (yet?) found a way to distribute the library as a framework using CocoaPods, but I think we can easily stick to the previous method of simply picking the right source files. I have updated the podspec (see 74f73b8) and tested on a sample project, seems to work. (I have tested on a slightly different podspec – the current podspec to be merged would not work, since it points to your repository and the 2.0.0 version that wasn’t released yet.)

Otherwise, I think that releasing the current version as 2.0.0 should work just fine. People who depend on the older API can easily pin the previous release in CocoaPods or Git Submodules. Semantic versioning should make it obvious that there were non-compatible API changes.

And, as a bonus, releasing the new version through CocoaPods should get us a nice documentation over at CocoaDocs.

[zoul]

Is there any chance of this getting merged, possibly making a 2.0 release as discussed above? The proposed changes no longer apply, but right now I could put some more work to update them for your current master. If we don’t merge the code now, the codebases will soon diverge for good. I think that would be a pity, for both sides.

[shpakovski]

Hey Tomáš, 2.0 sounds like the best option for me, but I still have no time for this. Mostly because I do not work on any Mac project at the moment to play with MASShortcut in production. However, I really like your code and all improvements.

Is there any way to invite you as a collaborator to my repository so that you can make changes without my approve?

[shpakovski]

Got it, done. This is my first collaboration experience on GitHub, so good luck to us. I'm not sure how to start the second version for CocoaPods with your proposed changes. Any advise? Thanks in advance!

[zoul]

Great, thank you! I’ll try and prepare a 2.0 branch based both on this pull request and your current master and will ping you when it’s ready.

[zoul]

I have rebased the changes on your current master, see #53.