org parser ignores timestamps with max repeat period

[cpbotha]

Describe the bug

From the Orgmode habits manual, Orgmode supports timestamps like this:

SCHEDULED: <2009-10-17 Sat .+2d/4d>

which means that it should repeat every two days but at most every four days.

There's nothing organice can do to really apply these timestamps (you repeat them the same as always, using the .+2d, the /4d is an additional piece of information used by for example orgmode habits).

However, currently that /4d causes the whole timestamp to be missed and ignored completely by the rest of the organice logic.

See in this screenshot that the timestamp is not clickable:

When I remove the /4d, all is good:

To Reproduce

Create a heading like this:

* TODO H: Running
  SCHEDULED: <2020-04-05 Sun .+2d/4d>

Toggle it to DONE.

Expected behavior

As with other scheduled items with repeats, the item should return to the TODO state with the scheduled date moved 2 days into the future from the done date, because .+.

What happens instead, is that organice toggles it to done, and that's it.

Possible solution

The regexes and other logic should be tweaked to recognize these timestamps, and then simply to disregard the /4d bits. Then everything should work as expected.

A few minutes later: Ok, if I add an additional (\/\d+[hdwmy])? after the first hdwmy-group, it fixes this specific issue.

A few more minutes later: It has to be a non-capturing group, so nothing else breaks. With (?:\/\d+[hdwmy])? inserted right after the first ([hdwmy]) all tests still pass. However, because that group is not stored in any of the parser data structures, it will get lost.

@DanielDe and @munen I think I could perhaps fix timeStampFromRegexpMatch(), but parseMarkupAndCookies() along with its monster regex really needs a bit of documentation to help random contributors (like me) to figure out how they work.

[munen]

Hi @cpbotha

Thanks for opening this issue. A couple of things:

Please take a look at the contribution guide, particularly this section. You'll see that we don't consider your issue a bug, but a new feature. I'll change the label accordingly.

Sorry about the big Regexp. Of course, you are right that it's huge. It's not easy for anyone to use. For the time being, all current knowledge of it is documented with tests. Apart from that, please look at this section of the README which further explains the strategy regarding the parser.

If you're planning to work on this feature, I suggest to do it test-driven. This approach has proven most useful for working with the big Regexp. Alternatively, you could contribute to org-parser. The feature won't be in organice any time soon, then, but in the long run it'll be more sustainable. Best case scenario is that you do both(;

Does that help? I suspect it's not exactly what you were hoping for, but it's pretty much the best I can offer.

Thanks for thinking to improve the timestamp functionality! 🙏

[cpbotha]

Thanks for the pointers @munen -- I can't work on this one right now, so I'll let it simmer for a while.

Please do accept my suggestion to require that contributors document a little bit more. The extensive code handling that giant regexp is comment-less. If there had been a few comments explaining the general working, there was a much greater probability that I could have made progress with this fix.

[schoettl]

Please do accept my suggestion to require that contributors document a little bit more. The extensive code handling that giant regexp is comment-less.

@cpbotha if you refer specifically to the regex, I don't think, that it need much comments. There is a good link above the regex, a website which visualize and let you edit the regex. That's the best documentation.

Oh, and I should merge my changes to the org-parser project (if I haven't done it yet). Because I added the most part of orgmode timestamps already (with tests). I'll take a note on that.

Please, anyone, wait until you start adding timestamps to the org-parser, it's in the pipeline...

[munen]

@cpbotha

Please do accept my suggestion to require that contributors document a little bit more.

You're preaching to the choir here^^ Before I started contributing, the project had three obsolete and failing tests, now we have 241. Before, there were fewer than 100LOC of documentation, now we have ~1000LOC. One of the things, I introduced is the Definition of Done which includes tests and documentation, so I'm very much with you.

And this is how the general approach relates to the parser and the Regexp: In fact, it's the first project I tackled when I started to contribute. Back then, the parser was in a bad shape and was constantly messing up my Org files. There were no tests and nobody could easily work on it. I started writing the first tests, wrote docs with regards to the state in the README and now we have 440LOC of tests for the parser. Of course, this state can always be improved upon. Hence the work we started on the sister project org-parser. In the meantime, anyone is welcome to write more docs and tests(;

I can't work on this one right now, so I'll let it simmer for a while.

Then we'll leave this issue open for the time being. If it's getting too old and stale, we'll close it to keep the number of open issues manageble.

[munen]

@schoettl Superb, looking forward to your PR on org-parser (^_^)/

[schoettl]

I implemented the parsing part here in org-parser.

[munen]

Perfect, thank you, @schoettl 🙏 🙇

[munen]

Since the bug is fixed in org-parser (which at some point will land in organice) and nobody spoke up to implement it in organice, I'm closing this issue for the time being. If anybody wants to implement this in organice right away, please feel free to re-open this issue and open a PR at any time!