Doesn't seem to really work for me.

I mean, yes it works in the sense that the timer doesn't go back to 20 seconds but at the end of my "you should rest" period i always get the "oh you've been bad my not resting" screen if i have used this feature of ignoring a few seconds.

Are you using the popup? (on the settings third panel on the list of the left, option of the bottom)

In D26498#591716, @aacid wrote:

Doesn't seem to really work for me.

I mean, yes it works in the sense that the timer doesn't go back to 20 seconds but at the end of my "you should rest" period i always get the "oh you've been bad my not resting" screen if i have used this feature of ignoring a few seconds.

I'm not sure what doesn't work for you. It seems to work as I intended. Here is how I test it (RSI settings are as at the bottom of this post):

1. work for 1 minute,
2. get a popup informing about a break,
3. popup counts down from 60 s,
4. I move a mouse for 1 s at 50 s,
5. popup counts down without a reset,
6. I move a mouse for 1 s at 40 s,
7. popup counts down without a reset,
8. At 30 s popup dims the screen and resting state is entered,
9. popup counts down without a reset.

If I would move my mouse for 3 s instead of 1 s at 4) or 6) point, then the countdown would be reset to 60 s which is undesirable.

Are you using the popup? (on the settings third panel on the list of the left, option of the bottom)

Here are my settings

ignore me i was complaining about a different behaviour that we already have in master and it's not introduced by this patch ^_^

Thanks a lot for your contribution :)

In D26498#592315, @aacid wrote:

Thanks a lot for your contribution :)

You're welcome, but what about the other revision, that I've sent you for a review?
Add length columns to CatalogModelColumns
Is it on your to-do list?

Is it on your to-do list?

Kind of, but my "review things for lokalize" priority is much smaller than "review things for rsibreak", i'm mostly hoping someone else picks it up, but if not i'll pick it up eventually

Hi @aacid & @wojnilowicz:

unfortunately this commit breaks the rsibreak_tests:

• in some states, m_shortInputCounter is a nullptr, so RSITimer::timeout() tries to ->tick() a null pointer
• even doing a nullptr check for the code above, RSITimerTest::triggerSimpleBigBreak() fails:

FAIL!  : RSITimerTest::triggerSimpleBigBreak() Compared values are not the same
   Actual   (spy1Relax.count()): 100
   Expected (relaxCountExp)    : 97
   Loc: [SRCDIR/test/rsitimer_test.cpp(186)]

Can you please look into both the issues? Thanks!

In D26498#637506, @pino wrote:

Hi @aacid & @wojnilowicz:

unfortunately this commit breaks the rsibreak_tests:

• in some states, m_shortInputCounter is a nullptr, so RSITimer::timeout() tries to ->tick() a null pointer
• even doing a nullptr check for the code above, RSITimerTest::triggerSimpleBigBreak() fails: ` FAIL! : RSITimerTest::triggerSimpleBigBreak() Compared values are not the same Actual (spy1Relax.count()): 100 Expected (relaxCountExp) : 97 Loc: [SRCDIR/test/rsitimer_test.cpp(186)] `
  
  Can you please look into both the issues? Thanks!

Following patch fixes the issue

In D26498#637978, @wojnilowicz wrote:

Following patch fixes the issue

0001-Fix-RSITimerTest-noPopupBreak.patch1 KBDownload

Is this solution good for you?

This patch fixes the rsibreak_tests crash, however the test still fails in RSITimerTest::triggerSimpleBigBreak(), in the way I already mentioned.

In D26498#638035, @pino wrote:

In D26498#637978, @wojnilowicz wrote:

Following patch fixes the issue

0001-Fix-RSITimerTest-noPopupBreak.patch1 KBDownload

Is this solution good for you?

This patch fixes the rsibreak_tests crash, however the test still fails in RSITimerTest::triggerSimpleBigBreak(), in the way I already mentioned.

Thank you for testing. Somehow I didn't notice the second failure. I've decided to post a solution in the another review .