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

Parsing of logged time in :LOGBOOK: drawers with comments fails #111

Closed
munen opened this issue Nov 21, 2019 · 7 comments
Closed

Parsing of logged time in :LOGBOOK: drawers with comments fails #111

munen opened this issue Nov 21, 2019 · 7 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@munen
Copy link
Collaborator

munen commented Nov 21, 2019

Clocking time within organice and saving it in a :LOGBOOK: drawer is a new feature (See #103).

It works fine, unless the clocked time is intervoven with comments in which case the parser crashes here.

According to the Emacs documentation, "can contain anything but a headline and another drawer", though.

A first test case (added to logbook.org) for this could be:

* Logbook with comments within the clocked times
  :LOGBOOK:
  CLOCK: [2019-11-21 Thu 21:12]--[2019-11-21 Thu 21:12] =>  0:00
  Here's another comment
  CLOCK: [2019-11-21 Thu 21:12]--[2019-11-21 Thu 21:12] =>  0:00
  CLOCK: [2019-11-12 Tue 14:15]--[2019-11-12 Tue 13:20] => -0:55
  There's a comment
  CLOCK: [2019-11-12 Tue 14:15]--[2019-11-12 Tue 14:25] =>  0:10
  :END:
@munen munen added bug Something isn't working enhancement New feature or request labels Nov 21, 2019
@munen
Copy link
Collaborator Author

munen commented Nov 21, 2019

The issue was first found by @Linuus in this ticket: #108

@munen
Copy link
Collaborator Author

munen commented Nov 21, 2019

@jamesnvc Since you've build the initial version of clocking time and hence might be interested, I'm pinging you here. Of course, there's neither obligation to work on this or even answer to this message. If you're not interested, please just ignore the notification and be sure that I'm sorry about cluttering your inbox(;

@jamesnvc
Copy link
Contributor

Oh yes, I ran in to that on some of my own data; I'm happy to work on a fix

@jamesnvc
Copy link
Contributor

Would removing the "invalid" data be acceptable?

@Linuus
Copy link

Linuus commented Nov 21, 2019

Would removing the "invalid" data be acceptable?

There’s nothing invalid about the data so please do not remove anything? :)

@munen
Copy link
Collaborator Author

munen commented Nov 21, 2019

@jamesnvc

Would removing the "invalid" data be acceptable?

Generally speaking, I'd say that parsing without crashing is good. However, for this specific case, I'd have the following two arguments:

  • One goal of organice is not to alter existing Org files so that there's no diff when using Emacs and organice. That's a big differentiator to me compared with alternative tools.
  • More importantly for this specific issue: This is clocked time. If people use Emacs to track their time, this could for example be for billing customers (I assume, because I do generate my invoices from it^^). If people use comments in between the clock entries and we remove them, they might not catch it and lose the ability to answer the rudimentary question on what they worked at a specific point in time. So in this case, I'd actually rather that organice crashes instead of losing data(;

Imo the best approach here would be a very aggressive regexp to check every line on whether or not it's a clock entry. If it's not, it can be saved and re-exported verbatim.

As a bonus, this other clocking related parser issue (#110) would also be solved(;

I'm happy to work on a fix

This is great news. Thanks for jumping into the discussion less than half an hour after a ping. There's an amazing community forming here. I'm stoked. Thank you everyone! 🙏 🙇

@munen
Copy link
Collaborator Author

munen commented Nov 22, 2019

Fixed by @jamesnvc in PR #112. 🙏

@munen munen closed this as completed Nov 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants