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.

Sign up for GitHub

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

Jump to bottom

Issue 20 harcoded 12h time format #23

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Issue 20 harcoded 12h time format #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d8607bf
Fix #21: do not stop timer if notification is closed by timeout (not …
brunetton Mar 24, 2023
a269c36
Code refactoring: move Timer.on_close() outside Timer.start() and ren…
brunetton Mar 24, 2023
e04eb4b
Fix #20: time format should use locale settings, not 12-hour format e…
brunetton Mar 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 20 additions & 13 deletions timer/Timer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from datetime import datetime, timedelta
from itertools import count

import gi
from gi.repository import Notify

from .media import get_icon_path, play_sound
Expand All @@ -22,48 +23,54 @@ class Timer:
the timer notification is closed.
"""

def __init__(self, run_seconds, name, callback, persistent=False):
def __init__(self, run_seconds, name, callback, loop, persistent=False):
self.id = next(ids)
self.run_seconds = run_seconds
self.name = name
self.callback = callback
self.loop = loop
self.persistent = persistent
self.tag = None
self.end_time = datetime.now() + timedelta(seconds=run_seconds)
self.notification = None
self.intervals = [30, 30, 30, 10, 10, 10]

def start(self, loop):
def start(self):
def on_end(on_close=None):
self.notify(self.name, self.time_since_end, on_close=on_close)
if self.persistent:
interval = self.intervals.pop() if self.intervals else 60
self.tag = loop.call_after_delay(interval, on_end)
log.debug(f"Notification sleeping for {interval} seconds...")
self.tag = self.loop.call_after_delay(interval, on_end)
else:
self.callback(self)
self.tag = None

def on_close(arg):
log.debug("notification closed")
self.callback(self)
self.stop(loop)

self.tag = loop.call_after_delay(self.run_seconds, on_end, on_close)
self.tag = self.loop.call_after_delay(self.run_seconds, on_end, self.on_notification_close)
self.notify(self.description, sound=False)
log.debug('Timer set for %s', self.end_time)

def stop(self, loop, notify=False):
def on_notification_close(self, arg):
# https://gnome.pages.gitlab.gnome.org/libnotify/enum.ClosedReason.html
reason = self.notification.get_closed_reason() # 1: timeout, 2: dismissed by the user
log.debug("notification closed" + (" by user" if reason == 2 else ''))
if reason == 2:
# Closed by user, stopping this timer
self.stop()
self.callback(self)

def stop(self, notify=False):
if self.tag is not None:
self.persistent = False # prevent unstoppable on_end() calls
loop.cancel_callback(self.tag)
self.loop.cancel_callback(self.tag)
if notify:
self.notify("Timer stopped", self.description, sound=False)
loop.call_after_delay(5, self.notification.close)
self.loop.call_after_delay(5, self.notification.close)
self.tag = None

@property
def description(self):
return f"{self.name} at {self.end_time.strftime('%-I:%M %p')}"
return f"{self.name} at {self.end_time.strftime('%-H:%M %p')}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brunetton does this break the AM/PM format?
Some people may be using it, I assume

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good question and I searched a while to figure out. At the beginning I thought that using %H with %p would check in locale def for the locale preferences and use 12h or 24h according to it. But the doc isn't clear on this point.

I digged around and I'm not sure anymore: is the 12h format automatic when locale is en_US ? Apparently this is a free interpretation. It made quite a mess in Debian when they changed it (I didn't read the whole thread).

I didn't found any "official" reference on this and this comment seems to go in flavor of local preferences.

I tried using specialized libs like Pendulum and Arrow but they seems to follow the same formatting as standard DateTime: 24h format by default.

So I guess this is a matter of local choice and I suggest that we add an option to the user to use 12h format or not.

What do you think about ?


@property
def time_since_end(self):
Expand Down
6 changes: 3 additions & 3 deletions timer/TimerExtension.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@ def __init__(self):
def set_timer(self, delay, text):
log.debug("add timer %s %s", delay, text)
persistent = self.preferences["persistent"]
timer = Timer(delay, text, self.on_timer_end, persistent)
timer.start(self.loop)
timer = Timer(delay, text, self.on_timer_end, self.loop, persistent)
timer.start()
self.timers.add(timer)

def stop_timer(self, timer_id):
timer = self.get_timer(timer_id)
if timer is not None:
log.debug("stop timer %s", timer.name)
timer.stop(self.loop, notify=True)
timer.stop(notify=True)
self.timers.remove(timer)

def on_timer_end(self, timer):
Expand Down