Hammerspoon / Spoons

The official repository of Spoon plugins
https://www.hammerspoon.org/Spoons/
453 stars 141 forks source link

Improved CountDown timer #300

Closed dmgerman closed 3 months ago

dmgerman commented 1 year ago
dmgerman commented 1 year ago

Is there anything else I can do to have this pull request merged? thank you.

asmagill commented 1 year ago

I apologize, I told @cmsj I'd take over for him re spoons, but had some unexpected interruptions occur. I'm hoping I can get back to this in the next day or so and run through a dry run to make sure I don't screw anything up and then start testing/merging this weekend.

dmgerman commented 1 year ago

hi,

I appreciate your response.

there is no rush at all. I am happy to know somebody is thinking about it.

I have other improvements that I can share, once things get rolling.

--daniel

On Wed, Aug 30, 2023 at 1:25 PM A-Ron @.***> wrote:

I apologize, I told @cmsj https://github.com/cmsj I'd take over for him re spoons, but had some unexpected interruptions occur. I'm hoping I can get back to this in the next day or so and run through a dry run to make sure I don't screw anything up and then start testing/merging this weekend.

— Reply to this email directly, view it on GitHub https://github.com/Hammerspoon/Spoons/pull/300#issuecomment-1699789159, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABIZ4BRGVJYNSYJPMUCQ2DXX6ORZANCNFSM6AAAAAAY3HWQ7U . You are receiving this because you authored the thread.Message ID: @.***>

-- --dmg


D M German http://turingmachine.org

cmsj commented 3 months ago

@dmgerman looks like you're missing * characters on some of the Parameters: lists in your docstrings. Even the none should be * None.

dmgerman commented 3 months ago

Thanks for the comment. There are some new features I have implemented. I'll address your comments and submit an update.

dmgerman commented 3 months ago

@cmsj @muescha

Hi everybody, I have uploaded my current improvements. I have added several features since I first uploaded these changes. I also fixed the documentation errors.

This is the complete summary:

 Improved CountDown timer

Lots of improvements over original timer. They can be divided into several
categories:

- Improve timer handler. The original timer counted seconds via a callback.
  If the computer was suspended, the timer would have been suspended too.
  The new timer continues to run even when the computer is suspended.

- Improved configurability of countdown timer
  Many of its properties can now be configured via object attributes

- Added menu bar:
  A menu bar item allows to start/pause/resume/cancel a timer

- Optional progress messages:
  It can optionally display messages to the screen as the timer is advancing

- Improved time-up messages.
  I found that the end of the timer notifications were too subtle to be noticed.
  It now allows several ways to configure the notifications

- A callback.
  User can specify a callback to the evaluated as the timer is
  started/paused/resumed/cancelled.

- In addition to minutes, a timer can now be set using a time of day,
muescha commented 3 months ago

your alerts are not on the same place always - is this by design?

/* 227 */ hs.alert.show(msg, obj.messageAttributes, nil, duration) -- nil
/* 291 */ hs.alert.show(message, obj.alertAttributes, screen, obj.alertLen) -- .mainScreen() / currentWin:screen()
/* 600 */ hs.alert.show(message, nil, hs.screen.mainScreen()) -- .mainScreen()
/* 630 */ hs.alert.show(message, nil, hs.screen.mainScreen()) -- .mainScreen()
dmgerman commented 3 months ago

your alerts are not on the same place always - is this by design?

/* 227 */ hs.alert.show(msg, obj.messageAttributes, nil, duration) -- nil
/* 291 */ hs.alert.show(message, obj.alertAttributes, screen, obj.alertLen) -- .mainScreen() / currentWin:screen()
/* 600 */ hs.alert.show(message, nil, hs.screen.mainScreen()) -- .mainScreen()
/* 630 */ hs.alert.show(message, nil, hs.screen.mainScreen()) -- .mainScreen()

Yes. We have three types of messsage: warnings (optional), messages while setting/modifying the timer, and final alert.

cmsj commented 3 months ago

Thanks @dmgerman. I'm going to merge this now, if you want to follow up on any of @muescha 's comments, feel free to open a new PR.