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

Timestamp ranges not matched the same way org-agenda does #237

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

yantar92
Copy link
Contributor

@yantar92 yantar92 commented Aug 28, 2021

I have a headline like the following:

***** NEXT [#A] Manuel Macías [Email] (2021) "Continuous Integration and TeX with Org-Mode", by Rohit Goswani (TUG 2021) :BOOKMARK:misc:email:
SCHEDULED: <2021-08-07 Sat 16:30-17:30>

I expect this headline to be matched by (scheduled 0) sexp, but it is not.

@alphapapa
Copy link
Owner

I may be mistaken, but I don't think it's valid to use a range timestamp as an entry's SCHEDULED timestamp.

@yantar92
Copy link
Contributor Author

yantar92 commented Aug 7, 2021 via email

@alphapapa
Copy link
Owner

I may be mistaken, but I don't think it's valid to use a range timestamp as an entry's SCHEDULED timestamp.

Well, I guess we'll have to do it too, then. :)

It is indeed discouraged in the manual. However, org-agenda does show scheduled entries with scheduled timestamp being a time interval. Also, ts predicate will not match timestamps with interval (see `org-ql-regexp-part-ts-time').

Hm, I thought I had that covered, but maybe I overlooked it. Thanks.

@alphapapa alphapapa self-assigned this Aug 10, 2021
@alphapapa alphapapa added the bug label Aug 10, 2021
@alphapapa alphapapa added this to the 0.6 milestone Aug 10, 2021
@alphapapa alphapapa modified the milestones: 0.6, 0.7 Sep 22, 2021
@alphapapa
Copy link
Owner

Retargeting this for 0.7. 0.6 has been delayed for too long.

@stfl
Copy link

stfl commented Dec 8, 2021

It's also relevant for ts-active timestamps. In particular this shows up for me when syncing my calendar with org-gcal which will create active ranged timestamps.

@alphapapa alphapapa changed the title scheduled predicate ignores headings scheduled to a time range Timestamp ranges not matched the same way org-agenda does Mar 1, 2022
@alphapapa alphapapa force-pushed the master branch 2 times, most recently from 4f5fbc4 to d0acc8c Compare May 30, 2022 16:27
@enko
Copy link

enko commented Aug 9, 2022

Anything hindering this pr to be merged?

balaramadurai added a commit to balaramadurai/org-ql that referenced this pull request Aug 14, 2022
scheduled time-range now covered

this is the code from alphapapa#237
@alphapapa
Copy link
Owner

@enko Well, yes: since this has been a bug, the fix should be accompanied by explicit tests, i.e. there should be a new test added for timestamp ranges, rather than just changing one of the test data entries, as the PR currently does. Maybe even multiple tests for active/inactive timestamps, in planning/deadline/scheduled lines, and in headings/content.

It also needs a changelog entry--which I typically take care of myself when merging, but if a contributor provides it in the patch, it saves me the time and makes it easier for me to merge (assuming it's done correctly; even when I ask someone to provide one, and they do, it usually contains typographical errors that I have to fix myself, which defeats the purpose).

Other than that, the limitation is time, which I don't have as much of as I used to.

In the meantime, you're free to use the PR's code and report back as to how well it works. Any bugs you could discover before merging would be helpful.

@alphapapa
Copy link
Owner

As well, fixing this properly probably requires more work, because a ranged timestamp essentially represents two timestamps, one at each end of the range, and searching for timestamps within specific ranges must account for both of them.

@dmitrym0
Copy link

Do you think it's possible to split this is up into two tasks:

  1. getting ranged timestamp to match scheduled and planning
  2. searching for a timestamp in a range.

I came here look for 1, looks like I'm not the only one. Would be happy to contribute tests for it as well and improve the original request.

@magogware
Copy link

I tried out your most recent commit @dmitrym0 and had no luck with either the scheduled predicate or the ts-active predicate.

@dmitrym0
Copy link

@magogware

Here's a part of my test data:

* TODO This heading should be there because it contains scheduling info with time
SCHEDULED: <2026-01-23 Sun 14:00-15:00>
:PROPERTIES:
:ID:       FAKE_ID1
:END:

and the associated org-ql query that returns it:

'(or
  (ts-active)
  (clocked)
  (and
   (todo "TODO" "DONE")
   (or
    (scheduled) (planning))))

@magogware
Copy link

Ack, sorry. A (Doom) Emacs restart ensured I was actually using the package built from your commit, and now everything works. Cheers! It all works a charm.

@ParetoOptimalDev
Copy link

ParetoOptimalDev commented Jan 31, 2024

I'm getting more friction from this issue lately as I now use a visual timeblock library that I find very helpful for me that depends on time ranges to work:

https://github.com/ichernyshovvv/org-timeblock

So... I'm interested in contributing here but I'm unclear on exactly the target is for it being done and being reasonably sure there are no regressions.

I see:

the fix should be accompanied by explicit tests, i.e. there should be a new test added for timestamp ranges, rather than just changing one of the test data entries, as the PR currently does. Maybe even multiple tests for active/inactive timestamps, in planning/deadline/scheduled lines, and in headings/content.

It also needs a changelog entry

testing confirms that fixing this issue will require more than just adjusting the regexp in org-ql-regexp-part-ts-time, because timestamps with time ranges like this must be handled correctly by other code.

Anyway, if anyone would like to dig into Buttercup's recent changes and figure out why it's broken, that would certainly be appreciated.

So from that the checklist in my mind would be:

* explicit tests
** add new test(s) for timestamp ranges
** 3-5 or more tests for active/inactive timestamps for across in planning
** 3-5 or more tests for active/inactive timestamps for across in scheduled
** 3-5 or more tests for active/inactive timestamps for across in deadlines
** 3-5 or more tests for active/inactive timestamps for across in headings
** 3-5 or more tests for active/inactive timestamps for across in content
* handle time rangers correctly in other code
** determine other code that needs to handle timestamps
* fix buttercup tests looping
** verify that buttercup tests still loop

I think if we could figure out a checklist that is likely to get merged or not get stuck because of a lack of confidence we can make progress on this. My biggest question and perhaps also the biggest unanswered question for you is where the other code is that needs to handle timestamps for a change like this.

Perhaps just adding the tests above or perhaps the above and any additions you may add will answer the question of what those places are?

@ivanperez-keera
Copy link
Contributor

Echoing @ParetoOptimalDev , what are the exact things that would need to be done for this to be complete?

It's 100% understandable if you cannot make this a priority, @alphapapa . This project is big and we are lucky that it works already as well as it does 😃

If you are able to enumerate the actions required, maybe others can complete them.

@alphapapa
Copy link
Owner

alphapapa commented Mar 11, 2024

@ivanperez-keera I'll try to summarize what comes to mind:

  1. First, since we're aiming to upstream at least part of org-ql into Org-mode, any contributions with regard to this issue would have to be made by someone who's done the FSF copyright assignment for Emacs.

  2. Given that, as I mentioned in Timestamp ranges not matched the same way org-agenda does #237 (comment), what's needed is to ensure that all the code that works with timestamps also works with timestamp ranges.

    That's probably not a trivial change, because it likely means passing one or two timestamps (e.g. a cons of beginning and end timestamps) everywhere (or almost everywhere) one timestamp is currently handled. Probably some wrapper macro and/or function would be written to help facilitate that where appropriate.

    Alternatively, maybe Org element objects which are Org timestamp ranges could be passed around. But that would probably mean 1) parsing them into ts objects in more places, which might reduce performance; and 2) changing code in a lot of places, including possibly turning ts objects into Org element timestamp objects sometimes.

  3. Finally, tests need to be added that test functionality with Org files containing timestamp ranges in various places (active/inactive, "standalone" ones, ones in planning (scheduled/deadline/closed) lines, logbook drawers, etc.

Looking back, I suppose this was a somewhat significant oversight on my part, to not realize that timestamps can have time ranges (something I never use in my own Org files). Thankfully it seems that not many other users use them, or else Org QL would probably be much less useful than it is.

Anyway, that's what comes to mind. Actually digging in to the code might reveal more or fewer problems to deal with.

If someone who has done the FSF CA wants to work on this, I'll be glad to provide feedback (but with the caveat that I aim for very high quality code on this project, and I'm very particular (realizing of course that there are probably people out there who think my code is atrocious, haha)).

@ivanperez-keera
Copy link
Contributor

org-ql together with timestamp ranges enables some pretty advanced forms of planning. I manage everything I do with org-mode, both at work and in my personal life, and scheduling tasks with ranges allowed me to plan full weeks of tasks with minute precision -- and stick to the plan.

I hope someone who knows lisp can help by sending a PR that meets Adam's requirements. I can contribute by trying this on large files.

Not sure if that will help, but I'll happily buy whoever works on this a few rounds of coffee, or a drink of your choice -- whatever keeps you going while you code :)

@alphapapa
Copy link
Owner

org-ql together with timestamp ranges enables some pretty advanced forms of planning. I manage everything I do with org-mode, both at work and in my personal life, and scheduling tasks with ranges allowed me to plan full weeks of tasks with minute precision -- and stick to the plan.

I'd like to read more about that, because it would be helpful to me too.

Not sure if that will help, but I'll happily buy whoever works on this a few rounds of coffee, or a drink of your choice -- whatever keeps you going while you code :)

FWIW, I do intend to see that this is solved, whether the code is written by me or someone else. It's just a case of limited time and professional priorities right now, for myself to do it. If someone were interested, I'd be open to the idea of the work on this project feature being sponsored, either by an individual or through a bounty program; that would allow me to prioritize doing the work myself.

@ivanperez-keera
Copy link
Contributor

ivanperez-keera commented Apr 11, 2024

I'd like to read more about that, because it would be helpful to me too.

I've been meaning to write about this, but --not a joke or some messed up passive aggression-- lately finding time has become hard because of this missing feature 😅

It's just a case of limited time and professional priorities right now, for myself to do it.

That's absolutely understandable. I'm on the same boat with the projects I maintain.

If someone were interested, I'd be open to the idea of the work on this project feature being sponsored, either by an individual or through a bounty program; that would allow me to prioritize doing the work myself.

I'd be open to this. I say let's try it.

I don't know how big a bounty we are talking; I doubt I'd be able to contribute a big enough amount to change your priorities, but if several of us are willing to chip in, then maybe we'll get there.

Also, it may make sense to let others who submit a PR take the bounty even if we don't get to an amount that would allow you to do it. They may have a bit more time and be willing to do it even if you can't.

@alphapapa
Copy link
Owner

alphapapa commented Apr 11, 2024

I've been meaning to write about this, but --not a joke or some messed up passive aggression-- lately finding time has become hard because of this missing feature 😅

LOL :)

Regarding a bounty: I'd probably only be willing to be involved in something like that if it were explicitly intended, from the beginning, to sponsor my working on the feature, not anyone else. The reason is not to deny anyone else the opportunity to work on it and benefit from a bounty. The reason is that, if someone else did the work and wanted to claim the bounty, I would still be involved, as it would be my responsibility to evaluate the code and decide whether to merge it; and if I rejected it, or if I required changes that the other developer did not want to make, I would not want to deal with conflict over whether the bounty should be claimed, whether it should be divided among various participants, whether my requirements were reasonable, etc. Whereas, if I were doing the work myself, I would naturally do it in a way that I would approve of and merge.

So I think it would be better considered as an opportunity to sponsor the author to implement a specific feature with priority over other features.

Of course, I can't control what anyone else does; if someone were to start a bounty somewhere, and that resulted in code being submitted (with proper copyright assignment), I wouldn't necessarily reject it. But I would not give any consideration to the bounty being involved. My objective is only to have high-quality code implementing useful features in a well-integrated, maintainable way. This is why it's important to coordinate such things with project maintainers in advance.

I hope you understand what I mean. When money gets involved, everything changes, and one has to be very careful. It's best to be as clear as possible from the beginning.

@dmitrym0
Copy link

I rely on scheduled time ranges heavily, though org manual seems to discourage this.

I wrote a small package to plan my days: org-hyperscheduler. I use org-ql to surface tasks and task-like things though SCHEDULED entries with a range require a patched version of org-ql.

@alphapapa

This comment was marked as off-topic.

@yantar92

This comment was marked as off-topic.

@alphapapa

This comment was marked as off-topic.

@yantar92

This comment was marked as off-topic.

@ivanperez-keera

This comment was marked as off-topic.

@dmitrym0

This comment was marked as off-topic.

@alphapapa

This comment was marked as off-topic.

@ivanperez-keera

This comment was marked as off-topic.

@yantar92

This comment was marked as off-topic.

@alphapapa

This comment was marked as off-topic.

@alphapapa

This comment was marked as off-topic.

@ivanperez-keera

This comment was marked as off-topic.

@ivanperez-keera

This comment was marked as off-topic.

@yantar92

This comment was marked as off-topic.

@ivanperez-keera

This comment was marked as off-topic.

@yantar92

This comment was marked as off-topic.

@hpfr

This comment was marked as off-topic.

@ivanperez-keera

This comment was marked as off-topic.

@alphapapa
Copy link
Owner

I still think this conversation is relevant to this issue, since its related to the use of ranges, but I'm ok taking this offline if @yantar92 prefers. I don't know if the mailing list is the right place.

This sub-thread seems to be about the semantics of SCHEDULED timestamps as interpreted by users and as implemented by Org Agenda. But this issue is supposed to be about Org QL's support for timestamps with internal time ranges. So, yes, please continue it on the mailing list, or on a thread on this repo's Discussions forum. If you do, feel free to post a comment here with a link.

In the meantime, I'm going to mark the relevant comments here as off-topic so the issue can remain focused on the technical issues regarding Org QL. No hard feelings; conversations work this way. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for timestamps with internal time ranges (events)
9 participants