Improved CountDown timer #300

dmgerman · 2023-06-05T16:56:11Z

Improved configuration of the notification area (size and transparency)
Added optional icons in the menubar to start, pause and end the time, and notify of remaining time
It is now able to deal with suspending the laptop during the timer i.e. time runs in "user" time, not "computer running" time
added hotkeys
- able to call timer again with last period
- or set new period

dmgerman · 2023-08-30T19:55:37Z

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

asmagill · 2023-08-30T20:24:49Z

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 · 2023-08-30T20:29:58Z

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 <#300 (comment)>, 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 · 2024-08-07T13:25:13Z

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

muescha

Added some minor nitpicks.

muescha · 2024-08-08T11:06:26Z

Source/CountDown.spoon/init.lua

+end
+
+function obj:query_time_and_start()
+   local button, time = hs.dialog.textPrompt("Enter time", "in minutes", string.format("%s", obj.defaultLenMin), "Ok", "Cancel")


Suggested change

local button, time = hs.dialog.textPrompt("Enter time", "in minutes", string.format("%s", obj.defaultLenMin), "Ok", "Cancel")

local button, time = hs.dialog.textPrompt("Enter time", "in minutes", string.format("%s", obj.defaultLenMin), "OK", "Cancel")

muescha · 2024-08-08T11:06:37Z

Source/CountDown.spoon/init.lua

+
+function obj:query_time_and_start()
+   local button, time = hs.dialog.textPrompt("Enter time", "in minutes", string.format("%s", obj.defaultLenMin), "Ok", "Cancel")
+   if button == "Ok" then


Suggested change

if button == "Ok" then

if button == "OK" then

muescha · 2024-08-08T11:07:24Z

Source/CountDown.spoon/init.lua

+-- default timer in minutes
+
+obj.defaultLenMin = 25


Suggested change

-- default timer in minutes

obj.defaultLenMin = 25

-- default timer in minutes

obj.defaultLenMin = 25

muescha · 2024-08-08T11:24:17Z

Source/CountDown.spoon/init.lua

+function obj:create_bar_timer(minutes)
+   local mainScreen = hs.screen.primaryScreen()
+   local mainRes = mainScreen:fullFrame()
+   obj.canvas:frame({x=mainRes.x, y=mainRes.h-obj.canvasHeight, w=mainRes.w, obj.canvasHeight})


is here h= missing?

Suggested change

obj.canvas:frame({x=mainRes.x, y=mainRes.h-obj.canvasHeight, w=mainRes.w, obj.canvasHeight})

obj.canvas:frame({x=mainRes.x, y=mainRes.h-obj.canvasHeight, w=mainRes.w, h=obj.canvasHeight})

muescha · 2024-08-08T11:32:30Z

Source/CountDown.spoon/init.lua

+   if obj.timer then
+      -- reset current timer and do nothing
+      obj:reset()
+   else


what if the minutes value is changed?

I think it is better to cleanup if there a timer exists and start over with setting the values to have a clean state

or check if minutes are change

Suggested change

if obj.timer then

-- reset current timer and do nothing

obj:reset()

else

if obj.timer then

-- reset current timer and do nothing

obj:reset()

end

muescha · 2024-08-08T11:36:50Z

Source/CountDown.spoon/init.lua

+         local pausedSeconds = obj:time_absolute_seconds() - obj.pausedAt
+         obj.startTime = obj.startTime + pausedSeconds


I would think this calculation is not valid if there are multiple pauses and resumes.

muescha · 2024-08-08T11:45:15Z

Source/CountDown.spoon/init.lua

+   local mainScreen = hs.screen.primaryScreen()
+   local mainRes = mainScreen:fullFrame()


wrong variable naming

Suggested change

local mainScreen = hs.screen.primaryScreen()

local mainRes = mainScreen:fullFrame()

local primaryScreen = hs.screen.primaryScreen()

local mainRes = primaryScreen:fullFrame()

muescha · 2024-08-08T11:47:10Z

Source/CountDown.spoon/init.lua

+        local mainScreen = hs.screen.primaryScreen()
        local mainRes = mainScreen:fullFrame()


wrong variable naming

Suggested change

local mainScreen = hs.screen.primaryScreen()

local mainRes = mainScreen:fullFrame()

local primaryScreen = hs.screen.primaryScreen()

local mainRes = primaryScreen:fullFrame()

muescha · 2024-08-08T11:50:46Z

Source/CountDown.spoon/init.lua

+      else
+         message = string.format("Illegal number [%s]", time)
+      end
+      hs.alert.show(message, nil, hs.screen.mainScreen())


The canvas is changed to primaryScreen — should the alert be as well?

muescha · 2024-08-08T11:50:59Z

Source/CountDown.spoon/init.lua

+         withdrawAfter = 100
+   }):soundName(obj.sound):send()
+
+   hs.alert.show(message, nil, hs.screen.mainScreen(), obj.alertLenMin)


The canvas is changed to primaryScreen — should the alert be as well?

dmgerman · 2024-08-08T18:37:47Z

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

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,

dmgerman · 2024-08-09T22:46:00Z

@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

some notes

muescha · 2024-08-09T22:34:03Z

Source/CountDown.spoon/init.lua

+---            the minutes that were requested.
+---            The callback is not called if timer is cancelled
+function obj:startFor(minutes, callback)
+   if not minutes  then


you could test minutes for type=function then minutes get the default minutes and place minutes into callback. so the minutes are optional

do you mean: if minutes is a function, call it and use return value as length of the timer?

currently, minutes is optional, if provided, if not provided it will use defaultLenMinutes

I mean that if minutes would be an optional parameter, but someone want a callback
function obj:startFor([minutes,] callback)

I have seen this on some places at hammerspoon:

if type(minutes)=='function' then callback = minutes minutes = obj.defaultLenMinutes end

example:
https://github.com/Hammerspoon/hammerspoon/blob/ec9a8ead1fc9074b8647c5ee4c09ba96f66a719a/extensions/host/host.lua#L89-L95

muescha · 2024-08-09T22:37:17Z

Source/CountDown.spoon/init.lua

+   if not minutes  then
+      minutes = obj.defaultLenMinutes
+   end
+   if obj.timerRunning then


maybe do here a timer:cancel() and start over with a new timer?

I think it is better to issue an error. The reasons is that one might accidentally try to set another timer without noticing the current one is running. I guess a better option would be to make this optional (via a variable). What do you think?

or alternatively, ask the user.

muescha · 2024-08-09T22:41:13Z

Source/CountDown.spoon/readme.org

+-- configure some bindings
+   countDown:bindHotkeys({
+      startFor = {{"cmd", "ctrl", "alt"}, "P"},
+      startInteractive = {{"cmd", "ctrl", "alt", "shift"}, "P"},


is here this is missing:
pauseOrResume = {{"cmd", "ctrl", "alt"}, "P"},

muescha · 2024-08-09T22:58:10Z

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()

I realized that I removed this function. To maintain full backwards compatibility I have added it back. Adds a new type of event (setProgress).

dmgerman · 2024-08-09T23:12:54Z

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 · 2024-08-09T23:32:48Z

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.

dmgerman mentioned this pull request Jun 5, 2023

Improved the countdown spoon #292

Closed

cmsj closed this Aug 7, 2024

cmsj reopened this Aug 7, 2024

muescha reviewed Aug 8, 2024

View reviewed changes

dmgerman force-pushed the improved-countdown-timer branch from 5b9a4ac to 72b703e Compare August 9, 2024 22:17

dmgerman force-pushed the improved-countdown-timer branch from 59380ca to b03f19a Compare August 9, 2024 22:45

muescha reviewed Aug 9, 2024

View reviewed changes

Add setProgress

e26c3e2

I realized that I removed this function. To maintain full backwards compatibility I have added it back. Adds a new type of event (setProgress).

cmsj merged commit c821e5a into Hammerspoon:master Aug 9, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved CountDown timer #300

Improved CountDown timer #300

dmgerman commented Jun 5, 2023

dmgerman commented Aug 30, 2023

asmagill commented Aug 30, 2023

dmgerman commented Aug 30, 2023 via email

cmsj commented Aug 7, 2024

muescha left a comment

muescha Aug 8, 2024

muescha Aug 8, 2024

muescha Aug 8, 2024

muescha Aug 8, 2024

muescha Aug 8, 2024 •

edited

Loading

muescha Aug 8, 2024

muescha Aug 8, 2024

muescha Aug 8, 2024

muescha Aug 8, 2024

muescha Aug 8, 2024

dmgerman commented Aug 8, 2024

dmgerman commented Aug 9, 2024 •

edited

Loading

muescha left a comment

muescha Aug 9, 2024

dmgerman Aug 9, 2024

muescha Aug 9, 2024

muescha Aug 9, 2024

dmgerman Aug 9, 2024

dmgerman Aug 9, 2024

muescha Aug 9, 2024

muescha commented Aug 9, 2024

dmgerman commented Aug 9, 2024

cmsj commented Aug 9, 2024

	local button, time = hs.dialog.textPrompt("Enter time", "in minutes", string.format("%s", obj.defaultLenMin), "Ok", "Cancel")
	local button, time = hs.dialog.textPrompt("Enter time", "in minutes", string.format("%s", obj.defaultLenMin), "OK", "Cancel")

	obj.canvas:frame({x=mainRes.x, y=mainRes.h-obj.canvasHeight, w=mainRes.w, obj.canvasHeight})
	obj.canvas:frame({x=mainRes.x, y=mainRes.h-obj.canvasHeight, w=mainRes.w, h=obj.canvasHeight})

		local pausedSeconds = obj:time_absolute_seconds() - obj.pausedAt
		obj.startTime = obj.startTime + pausedSeconds

		local mainScreen = hs.screen.primaryScreen()
		local mainRes = mainScreen:fullFrame()

Improved CountDown timer #300

Improved CountDown timer #300

Conversation

dmgerman commented Jun 5, 2023

dmgerman commented Aug 30, 2023

asmagill commented Aug 30, 2023

dmgerman commented Aug 30, 2023 via email

cmsj commented Aug 7, 2024

muescha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muescha Aug 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmgerman commented Aug 8, 2024

dmgerman commented Aug 9, 2024 • edited Loading

muescha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muescha commented Aug 9, 2024

dmgerman commented Aug 9, 2024

cmsj commented Aug 9, 2024

muescha Aug 8, 2024 •

edited

Loading

dmgerman commented Aug 9, 2024 •

edited

Loading