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.

Sign up for GitHub

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

Jump to bottom

Add effort properties for agenda elements #415

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add effort properties for agenda elements #415

wants to merge 1 commit into from

Conversation

anpandey
Copy link
Contributor

Related to #370

@alphapapa
Copy link
Owner

Thanks, but please hold off on any further PRs until you've done the copyright assignment: #325 (comment) 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 alphapapa added this to the Future milestone Feb 25, 2024
@anpandey
Copy link
Contributor Author

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 anpandey marked this pull request as draft February 25, 2024 08:07
@alphapapa
Copy link
Owner

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.)

@anpandey anpandey force-pushed the format-element-effort branch 3 times, most recently from c677e65 to e17055a Compare April 14, 2024 04:58
@anpandey anpandey marked this pull request as ready for review April 14, 2024 05:07
@anpandey
Add: (org-ql-view--format-element) effort properties
ae20235 
Org uses the nested properties within the 'txt' property for effort-related
computations. The contents of 'txt' itself are not used for anything (and is
usually set to a string of the headline), so we do the same here.
@anpandey
Copy link
Contributor Author

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

@alphapapa
Copy link
Owner

@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: #325 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
author-needs-FSF-CA enhancement waiting
Projects
None yet
Milestone
Future
Development

Successfully merging this pull request may close these issues.
2 participants
@anpandey @alphapapa