Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mountain Lion support #34

Merged
merged 15 commits into from
Aug 16, 2012
Merged

Mountain Lion support #34

merged 15 commits into from
Aug 16, 2012

Conversation

abarisain
Copy link
Contributor

Hello !
I noticed that, since DP1, Scrup auto uploading is broken on Mountain Lion.

I tracked this down to NSDistributedNotification center + a carbon notification being deprecated (Apple deprecated Carbon in 10.8)

Changes were required so Scrup watches the screenshot folder using FSEvents. Since FSEvents is a C api (thanks for the downgrade apple !) I used a small library called SCEvents, wrapping the C apis into nice Obj-C.

The events are fired every second, so it might look like it lags a little, but when you copy a lot of files in a directory with screenshots (usually Desktop !) it saves quite a lot of cpu compared to 0,5 seconds.
Also, the screenshot directory is not scanned anymore if the FS event occured in a sub directory (I have not found any way to tell FSEvent that I don't want it to fire events from subdirectories).

I did quite a lot of commits, and accidentally committed my xcode user files (I removed them afterwards), so you might have quite a lot of work reviewing this pull request ...

I also hope that I did not make your code dirty or buggy, as I'm still a quite noob in "real" Obj-C dev, coming from the easy "hey let's just parse this json" iOS world.

Please note that since I am working on Mountain Lion, I could not compile against the 10.6 SDK, since it's now gone (yay apple). However, I set the target to 10.6, so it should run on it anyway. (I said should, because I couldn't test it on 10.6/10.7, since I only have one single boot mac)

@WDC
Copy link
Contributor

WDC commented Jul 18, 2012

abarisain's build works perfectly on Mountain Lion GM.

PLEASE merge that code into the repo!

Thanks abarisain!

@mfahy
Copy link

mfahy commented Aug 10, 2012

Agreed! Thanks!

@jimgreer
Copy link

+1!

rsms added a commit that referenced this pull request Aug 16, 2012
@rsms rsms merged commit a0145e6 into rsms:master Aug 16, 2012
@rsms
Copy link
Owner

rsms commented Aug 16, 2012

@abarisain Seems this patch is actually broken. In pathWatcher:eventOccurred: the SCEvent type of object does not have a eventFlags method nor does it have a eventPath method. This is due to the lack of importing the SCEvent interface. I'm patching up the code.

@abarisain
Copy link
Contributor Author

Great to see that you're working on this :)

I thought that the patch was ok, since it's working here, sorry.

Also, I mentionned a bug with it (the autodection does not always work. It breaks rarely, but it does !) here : https://github.com/rsms/scrup/issues/32#issuecomment-7654581

I did not have any issues with my code, but then I merged this : abarisain@f676cda and I think that it might be the culprit.

I haven't had enough time to check if this was the cause, because I could not find a way to reproduce the bug. It just happens sometimes :/

Anyway, thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants