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

mark-feed-read when filter is applied should only mark visible items read #1364

Closed
tonycpsu opened this issue Dec 18, 2020 · 1 comment · Fixed by #1558
Closed

mark-feed-read when filter is applied should only mark visible items read #1364

tonycpsu opened this issue Dec 18, 2020 · 1 comment · Fixed by #1558
Labels
bug This issue reports a problem that ought to be fixed
Milestone

Comments

@tonycpsu
Copy link

Newsboat version (copy the output of newsboat -v or the first line of git show):

r2.21-362-g80f1

Steps to reproduce the issue:

  1. Apply a filter by pressing F and type in title =~ "(string that's only in a subset of articles)"

  2. Press A to mark all read.

Expected: only visible articles are read, similar to if one had used / (open-search) to do the filtering instead of a filter.

Actual: all articles in the feed are read.

My guess is this happens because / creates a temporary "feed" and marks it read, while filters work differently? Anyway, seems like they should work the same, or perhaps a new mark-visible-read command could be added?

@Minoru
Copy link
Member

Minoru commented Dec 18, 2020

Reproduced on 8880ba4, thanks for the report!

My guess is this happens because / creates a temporary "feed" and marks it read, while filters work differently?

Exactly. Filters are applied at "display" time, by filtering out the items that don't match:

for (const auto& item : items) {
item->set_index(i + 1);
if ((show_read || item->unread()) &&
(!apply_filter || matcher.matches(item.get()))) {
new_visible_items.push_back(ItemPtrPosPair(item, i));
}
i++;
}

seems like they should work the same

Yes, I think the expected behaviour you describe is the correct one.

@Minoru Minoru added the bug This issue reports a problem that ought to be fixed label Dec 18, 2020
@Minoru Minoru added this to the 2.24 milestone Jun 20, 2021
Minoru added a commit that referenced this issue Jun 27, 2021
5163927 (PR #1558) changed
mark-all-read to mark each item individually. That made mark-all-read
aware of the filters, but also made it slower. This was especially
noticeable with remote APIs, where mark-read latency is high.

We can't speed it up in the general case, but when there is no filter,
we can fall back onto the faster mark_all_read() method which marks the
entire feed in one go. I tested that this new code is indeed faster and
doesn't re-introduce #1364.

Reported by Oneiric on IRC.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue reports a problem that ought to be fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants