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

[Feature request] Skip position during review #104

Open
cashpw opened this issue Oct 10, 2022 · 2 comments
Open

[Feature request] Skip position during review #104

cashpw opened this issue Oct 10, 2022 · 2 comments

Comments

@cashpw
Copy link
Contributor

cashpw commented Oct 10, 2022

Problem

have several positions which are some variation on "Implement X algorithm in python". I'd prefer to only implement one algorithm per review session.

Potential solutions

  1. (current) Quit and create a new session to shuffle the positions around. Repeat until I get through all other reviews.
  2. (proposed) Add a function, and keybinding, to skip the current position during review. Depending on user settings this would either (1) remove the position from the review queue or (2) send it to the back of the queue.
  3. (alternative) Generalize org-fc-bury-siblings to enable limiting the number of positions shown from cards with a user-chosen tag per review. That is, to reference my use-case: only show one position from cards tagged with :algorithm:
@l3kn
Copy link
Owner

l3kn commented Nov 23, 2023

I just found org-fc-review-skip-card in the source code https://github.com/l3kn/org-fc/blob/main/org-fc-review.el#L261 which implements variant 2.1, it's just not bound in the keymap.

Variant 2.2 would be nice, the review queue needs some rework anyway so failed cards with a non-zero interval get a chance to be reviewed if the interval is over once the other cards have been reviewed.

3 sounds more complicated and could be part of an extension of the review-context system
to allow limiting cards and creating review sessions from multiple filters (e.g. 10 cards matching this tag, 20 matching some other tag)

@cashpw
Copy link
Contributor Author

cashpw commented Nov 23, 2023

I just found org-fc-review-skip-card in the source code https://github.com/l3kn/org-fc/blob/main/org-fc-review.el#L261 which implements variant 2.1, it's just not bound in the keymap.

That's exactly what I'd imagined! I hadn't found it in the code when I submitted the feature request. Thanks for adding it in the first place 😄!

3 sounds more complicated and could be part of an extension of the review-context system to allow limiting cards and creating review sessions from multiple filters (e.g. 10 cards matching this tag, 20 matching some other tag)

Agreed --- I ended up implementing that along with the class refactors I made a while back. I've been running this daily for a few months now but never got around to refactoring and creating a new pull request against this repo.

In short, I extended the review code to filter positions against a user-defined set of filter functions. I added the following to address my concern in the original comment:

(cl-defmethod cashpw/org-fc--filter-limit-implement ((positions list))
  "Return nil to remove the POSITIONS (`org-fc-position's) from the review list."
  (let ((implement-position-limit 1)
        (implement-position-count 0))
    (--filter
     (let ((tags (oref (oref it card) tags)))
       (if (member "implement" tags)
           (if (= implement-position-count implement-position-limit)
               nil
             (cl-incf implement-position-count)
             t)
         t))
     positions)))

Note this function won't plug-and-play with the current version of org-fc because it's built on top of the class refactoring I mentioned. I realize now on re-reading this function that it'd be better to (1) take a tag and limit as arguments rather than embed them and (2) refactor to make the function early-return if implement-position-count is >= the limit.

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

No branches or pull requests

2 participants