org-ql icon indicating copy to clipboard operation
org-ql copied to clipboard

Add effort properties for agenda elements

Open anpandey opened this issue 1 year ago • 5 comments

Related to #370

anpandey avatar Feb 25 '24 02:02 anpandey

Thanks, but please hold off on any further PRs until you've done the copyright assignment: https://github.com/alphapapa/org-ql/pull/325#issuecomment-1958871852 And until then, please convert this to a draft PR.

Regarding the code:

  1. I think this might call for a test to be included, because this involves the kind of code that can sometimes change between Org versions, and it would be good to catch such breakage automatically.
  2. That kind of test would be more easily facilitated if the txt property's value were returned from a function (so that function itself could be tested).

What do you think?

alphapapa avatar Feb 25 '24 04:02 alphapapa

Sure, I'll convert this to a draft. I'm currently waiting for a reply regarding the copyright assignment forms.

I agree with (1). We could use org agenda functions like org-agenda-compare-effort to see if the property can be properly read by org. I don't think I know enough about how the existing tests to have an opinion on (2) yet. I would prefer to go from end-to-end from org element to testing and agenda entry (simply because it has more coverage), but I can understand if this ends up being too hard to implement.

anpandey avatar Feb 25 '24 08:02 anpandey

Thanks.

We have two choices with testing:

  1. Compare our string and text properties with that from an Org Agenda buffer showing the same item. That would be the most thorough, but I don't think it's worth the trouble.
  2. Use code as close to what Org Agenda uses to get the string and text properties, and compare that with the expected value. It won't protect against the Org Agenda code or format changing, but it would alert us to changes in the underlying functions, which would still be very useful. (Beyond that, if the result changes in an incompatible way, a user will let us know.)

alphapapa avatar Feb 25 '24 08:02 alphapapa

@alphapapa FYI I don't think this needs the author-needs-FSF-CA tag anymore.

anpandey avatar Aug 21 '24 02:08 anpandey

@alphapapa FYI I don't think this needs the author-needs-FSF-CA tag anymore.

Thanks for the reminder, but please see this comment I just posted: https://github.com/alphapapa/org-ql/pull/325#issuecomment-2304235822

alphapapa avatar Aug 22 '24 09:08 alphapapa