@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:
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+.~#2511 - Updated hs.fs to use LuaFileSystem 1.8.0~
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
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_nsurlpath_to_cstringpath_at_indextags_from_filetags_to_fileIt 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:
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.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.
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!
Most helpful comment
Released! Thanks @asmagill , @latenitefilms and everyone else who helped!