Hammerspoon: Version 0.9.81

Created on 26 Sep 2020  路  17Comments  路  Source: Hammerspoon/hammerspoon

@cmsj, I know you're busy and I greatly appreciate the time you have had to share with us lately! But since releases require you, I wanted to kind of force the issue and put this out for comment...

I think we need to get 0.9.81 out sooner rather than later because it does contain some important fixes to hs.window.filter and hs.window. I think incoming user issues have slowed down because our most active users have manually applied our fixes at our requests to verify that they work, but if someone else stumbles upon them, it might discourage them if they aren't already somewhat invested...

Here are my thoughts re the currently outstanding pull requests:

  • 1706 - Add extension for Do Not Disturb Mode (and underlay User Preference extension)

    • I've already stated my oppinion that this really belongs in a more generic module that is a combination of hs.settings, CFPreferences, and some PList management tools... I have started, but my version is no where near ready and should be held till 0.9.82+.
  • 2493 - Add method to hs.eventtap.event to capture touch data for events of type NSEventTypeGesture (29)

    • This is still very much a work in progress and I think needs to be held for 0.9.82
  • ~#2511 - Updated hs.fs to use LuaFileSystem 1.8.0~

  • 2526 - Updated hs.fs to use LuaFileSystem 1.8.0 take 2

    • Since we've moved to Lua 5.4, I think this would be nice to include ~, but I won't be able to do much with it until tonight/tomorrow (see my latest comment in the pull)~. I'd like this in 0.9.81, but I don't think it's a deal breaker.
  • 2512 - Added hs.eventtap.event.newGesture()

    • As much as @latenitefilms thinks this is ready, I really have not had the chance to test it at all and probably won't before Monday. And it folds so nicely in with #2493. I'm inclined to hold this till 0.9.82 unless there are any objections.
  • 2516 [PTAL]Add localization and localize Chinese

    • I think this is a great idea and I would love to see it added, but it's a large enough change, at least in terms of files touched that I think it should be held over until 0.9.82 so we can really test it and possibly solicit some additional localizations
  • 2522 make retain and release of elementRef in HSuielement explicit

    • Appears to have helped with #2521, so unless @cmsj thinks I'm missing something and there was a reason for the previous lack of CFRetain/CFRelease, I believe this should land as well.

Thoughts? Disagreements? Skewering of the messenger?

@cmsj, since you have to do the actualy build, when is a good time to say "Hey, I'm building this on X, so decide by then"?

edited to reflect updated LFS 1.8.0 pull and addition of HSuielement issue

Most helpful comment

Released! Thanks @asmagill , @latenitefilms and everyone else who helped!

All 17 comments

All sounds good to me! Although I'd probably just push out 0.9.81 whenever @cmsj next gets a free moment, as I think the lfs stuff can wait, given hs.fs is working fine and passing all the tests.

To be honest, I'm not sure how important it is to stay in-sync with lfs anyway, as hs.fs seems to use a lot of macOS API calls whereas lfs is more generic, so I feel like hs.fs may have already drifted away from lfs anyway? Will leave this for you to test and ponder though @asmagill.

But yes, as far as 0.9.81 - I think it's probably good to go whenever you're ready. Absolutely no rush on the gestures stuff - I'd definitely like @asmagill to review and test before merging.

Thanks team!

I have used the more basic aspect of lfs before... not often, but occasionally... we should either update it (though I agree it's not a deal breaker for 0.9.81 -- it hasn't caused issues that anyone has reported yet) or replicate the functionality for anything that isn't already... how much overlap between the original and our additions is there?

I've just done a quick comparison between the code that @cmsj originally based hs.fs off and the latest release of lfs here:

https://github.com/latenitefilms/hammerspoon/commit/ca14a3b538cabca7ecd272e8200843adfb1d7288

It looks like the only new addition was lfs.link, but hs.fs already has an equivalent hs.fs.link() function.

(Actually, hs.fs was created on 25 Oct 2014, so I think this tree is probably a better comparison if he used the code from the master branch or v1.6.2 if he used the shipping release, which is probably much more likely).

Between @heptal and @cmsj - it looks like a lot of the original lfs code has been substantially changed, and Hammerspoon-ified. Back in 2016 it looks like @heptal removed all the Windows code and tidied things up.

There's lots of macOS specific additions such as:

  • path_to_nsurl
  • path_to_cstring
  • path_at_index
  • tags_from_file
  • tags_to_file

It also looks like over the years, as Lua itself has been updated, fixes have been manually applied to hs.fs.

Given this, I think it might be hard to rewrite hs.fs to be a wrapper of lfs without breaking things or changing how things behave. I think probably letting hs.fs seperate from lfs, like it's pretty much already done, makes the most sense in this case.

This comparison is probably more useful - the changes between lfs 1.6.2 and 1.8.0:

https://github.com/latenitefilms/hammerspoon/commit/2f62a6f6ec1653a94e23655d6ee8f0c4cb11581b

Meh, I'm easy... should I leave it to you then, since you've already added the __close stuff for the directory iterator or do you want me to take it over?

I do think the lfs functions should be compared against the master to make sure there haven't been any real fixes we've missed, and static FILE *check_file (lua_State *L, int idx, const char *funcname) ; should probably be completely replaced... the only reason we've gotten away with it is because the first element of luaL_Stream is the file handle... if that ever changes we'd have an issue with not complying with the proper lua structure for streams. Are there other helpers that should be reviewed as well?

I'm more than happy to manually compare, see if I can spot any obvious changes that should be addressed and add them to the existing pull request - but someone (most likely you) is going to have to idiot check anyway, so if it's easier for you to just tackle from scratch, go for it! No stress either way - but if there's ANYTHING I can do to make your life easier, just let me know.

I'd like @cmsj to have a chance to review #2522 to make sure I haven't missed anything before I merge it, but given the positive feedback (so far) from #2521, it seems likely that it should land as well before building 0.9.81.

I'm going to take a crack at reviewing and updating hs.fs tonight, given the direction discussed above and see if maybe it can't be ready as well.

@asmagill re your question of when to call the release, I agree that sooner would be better than later, so I'd suggest we hold off on things like LFS unless absolutely necessary, and get 0.9.81 out whenever we can. It really doesn't take me very long to do a release, so whenever you think you have all the fixes we need, let me know and I'll do it.

The only real questions I have before landing 0.9.81:

  • re lfs, do we change the example in the docs for hs.fs.dir? And I'd like someone other than me to review the changes to make sure I didn't miss anything. Otherwise I'm pretty sure it's ready to go. I got the clarification I wanted on why the iterator returns 4 args now.
  • #2528 applies to the latest Safari, so it probably needs to be in 0.9.81 as well... it should also be reviewed by someone else to make sure I'm releasing things properly (I'm going to bed about 4 hours late after I finish this post, so I won't trust it until someone else looks at it or I look at it again fresh in about 12 hours or so).

Otherwise, I think we're basically ready, unless @latenitefilms has any last minute thoughts or additions.

Oh, and I guess the question about whether or not the generator should throw an error instead of return nil, msg when an error occurs -- look at #2526 for details.

FWIW - I'll defer to @cmsj in regards to hs.fs.dir. I'm happy with whatever @asmagill and @cmsj recommend.

I'll have a look and test #2528 shortly, and let you know if it passes the tests locally using Safari 14.

FWIW - #2528 looks good to me.

2529 can also probably be included in 0.9.81 as well, as it shouldn't take long to review.

If there are no objections, I'm going to change hs.fs.dir so that it throws an error -- it's what I would expect when using an iterator in a for loop which is really the right way to use this function in Lua 5.4.

Fine by me!

@asmagill so we're good to go now for release, right?

Yep, as discussed here, good to go!

Released! Thanks @asmagill , @latenitefilms and everyone else who helped!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

piskov picture piskov  路  4Comments

zhenwenc picture zhenwenc  路  4Comments

dasmurphy picture dasmurphy  路  4Comments

aaronjensen picture aaronjensen  路  3Comments

asmagill picture asmagill  路  4Comments