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

feat: suggestion for clock:now #747

Merged
merged 6 commits into from
Nov 22, 2021
Merged

Conversation

tarnung
Copy link
Collaborator

@tarnung tarnung commented Nov 7, 2021

A quick and dirty implementation for parts of the feature set described in #737

=> clock:now searches for active clocks

I thought that other things than clocks can create "logBookEntries". But maybe i misremember. I could not find any in my demo files.

This pull request does not address the issue #745. Since it does not add to the problem we can go along and refactor later once @schoettl and I what if anything should be done.

@tarnung tarnung changed the title WIP Suggestion for clock:now suggestion for clock:now Nov 8, 2021
@tarnung
Copy link
Collaborator Author

tarnung commented Nov 8, 2021

I added a basic indicator for active clocks. It checks for them when parsing the org file and updates their number when clocking in/out.
It currently just turns a button red. This should probably be surplanted by a solution that is more theme aware.

@tarnung tarnung changed the title suggestion for clock:now feat: suggestion for clock:now Nov 8, 2021
@tarnung
Copy link
Collaborator Author

tarnung commented Nov 9, 2021

The indicator can get out of sync when headers with active clocks are deleted. Maybe there's a more robust implementation, maybe this is good enough as it corrects itself on the next file parse.

@munen
Copy link
Collaborator

munen commented Nov 9, 2021

Nice PR!

I thought that other things than clocks can create "logBookEntries". But maybe i misremember. I could not find any in my demo files.

A :LOGBOOK: drawer can also contain a state change for a task. For example updating this task:

* TODO Repeating task
  SCHEDULED: <2019-11-27 Wed +1d>

will become:

* TODO Repeating task
  SCHEDULED: <2019-11-28 Thu +1d>
  :PROPERTIES:
  :LAST_REPEAT: [2021-11-09 Tue 14:39]
  :END:
  :LOGBOOK:
  - State "DONE"       from "TODO"       [2021-11-09 Tue 14:39]
  :END:

Example taken from https://github.com/200ok-ch/organice/blob/master/test_helpers/fixtures/various_todos.org#repeating-task

Relevant Org mode doc: https://orgmode.org/manual/Drawers.html

Having said that, your hasActiveClock implementation looks solid enough that it will only trigger on actual clocks(;

@munen
Copy link
Collaborator

munen commented Nov 9, 2021

I have gone through the code and tested it. It works for me and I like the implementation.

I haven't merged, yet, for this reason: Since the magnifying glass already changes color, I'd say this PR also needs the icon that populates a search with clock:now. Otherwise the magnifying glass indicates something that is actionable, but the user doesn't know what. Alternatively, to get this PR quicker into master, we can remove the color change and add it at a later date.

@tarnung
Copy link
Collaborator Author

tarnung commented Nov 14, 2021

this pr is replaced by #751 if the changes over there are accepted

@munen munen merged commit 7258080 into 200ok-ch:master Nov 22, 2021
@chasecaleb
Copy link
Contributor

Nice job on this, thanks for implementing @tarnung!

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.

3 participants