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.

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

added linux notification #2

Merged
merged 2 commits into from Jul 14, 2022
Merged

added linux notification #2

merged 2 commits into from Jul 14, 2022

Conversation

basaran
Copy link
Contributor

@basaran basaran commented Jul 14, 2022

added "m" to string as well. Could you see if the os_check works for you on mac and merge if all is well?

added "m" to string
@olimorris
Copy link
Owner

Amazing!! Thank you so much. I shall test out later today.

@basaran
Copy link
Contributor Author

basaran commented Jul 14, 2022

Hello, it turns out tmux has a nice command prompt. I added additional method to set the timer manually. Should probably add some validation there perhaps. Also, I wasn't sure if it should restart an existing timer. Please create an issue if you would like to discuss.

@olimorris
Copy link
Owner

Can confirm that it works on Mac perfectly.

Regarding the custom pomodoro timer. I ❤️ the idea. Fully onboard. Only request...once you enter a value in the command prompt, can you trigger the countdown? I love that it stores the input for future pomodoros though.

@basaran
Copy link
Contributor Author

basaran commented Jul 14, 2022

great, there are a few ways to go at this manual timer. It could stop any existing timer and start a new one. That's what I had done initially.

But then I realized it might be more consistent to just use it for setting the timer and not interfere with anything else, and let the user start / stop it. At the moment, if a timer was already running, manually setting would make the existing timer look like it started from that timer. For instance:

a. Start pomodoro with the hard coded timer, say 20 minutes.
b. 5 minutes go by, now the timer 15 minutes.
c. You manually set the timer to 30 minutes
d. Timer shows 25 minutes to go.

I think this is more consistent then resetting and restarting. But since we don't show the initial value, it could be also be confusing What do you think? Perhaps show the current value next to the count down? like 20m/20m 19/20m, 18/20m..

Another idea, to make the newly set timer value persist after a shutdown, we could read "try" to read @pomodoro_mins from a file during the first launch, it would be that file which the manual method would write to it once it's set.

And finally, we could probably also prompt a task name, but then we would have to dive into multiline status bars, which might not look very uniform.

@basaran
Copy link
Contributor Author

basaran commented Jul 14, 2022

I looked more into this, I think it's best to avoid the task name. A pomodoro timer is not a task manager, for that we have orgmode :)

@olimorris
Copy link
Owner

All really good points. My thoughts are that we run the risk of making this really complicated. Your initial idea of customising the timer is great and the API can be called out in the README to make it clear to all users.

I'm keen to merge this into main, as-is. What are your thoughts?

@basaran
Copy link
Contributor Author

basaran commented Jul 14, 2022

I think we should merge it as it is for now. I briefly looked into writing the new timer value to a file to make it permanent etc, but that turned out to be hackier than I thought.

When you set the tmux variable through the command-prompt, and try to get it back right away doesn't appear to be working. It retrieves the previous value. Probably a tick condition. I will try to work on this sometime next week and find an efficient way to do this.

@olimorris olimorris merged commit 672c8cd into olimorris:main Jul 14, 2022
@mskelton
Copy link

This seems to have broken sound on macOS due to these lines.

https://github.com/olimorris/tmux-pomodoro-plus/pull/2/files#diff-49376947ccc1be4101aa3ff278c29ca8d4e0809d5cc6e1e9102daf15b8e8f4bdR80-R85

It checks if the sound is 'on' but then uses that as the sounds name but on is not a sound name.

@mskelton mskelton mentioned this pull request Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants