search

AnKing-VIP / advanced-browser

Anki add-on with card browser enhancements.
GNU General Public License v3.0
58 stars 13 forks source link

Fix sorting by overdue interval #144

Closed abdnh closed 2 years ago

abdnh commented 2 years ago

Fixes #112

The issue was caused by missing parentheses, from cards and end clauses. Apparently introduced in #73.

ijgnd commented 2 years ago

Thanks for fixing this long-standing bug.

I wonder if your code can be improved in one small regard: With your code cards that are not overdue are sorted next to the most overdue cards and not next to the least overdue cards with an overdue ivl ov 1. But I think the latter would make more sense.

Would it help to change line 156 from your branch from

 when {mw.col.sched.today} - due <= 0 then null

to 

when {mw.col.sched.today} - due <= 0 then 0

Keep in mind that I know next to nothing about sql.

abdnh commented 2 years ago

This can make it hard to find overdue cards at a glance as they will be positioned between cards with order=0 (non-overdue) and cards with order=null (new and learning cards).

Compare the following examples from a large collection (~ 240K cards):

(Notice the scrollbar position)

The user potentially has to scroll down a lot until they see the first overdue card, which makes sorting less useful.

ijgnd commented 2 years ago

I didn't think of this. If this were one of my add-ons I'd probably just add an option. But over the last three years no one else has asked about this. My idea is an unpopular one.

ijgnd commented 2 years ago

@AnKingMed: Can you merge this and make a new upload to ankiweb? Besides issue #112 this should also fix #143 so that you can also close these two issues. Could you also test this - this should take only a few minutes. I can't merge this myself since in May my access to this repo was removed (which I don't mind).

AnKingMed commented 2 years ago

Will do! Sorry your access got removed. I just added you back!

AnKingMed commented 2 years ago

should this only update the last 2 branches?

image

abdnh commented 2 years ago

should this only update the last 2 branches?

The issue also exists in the .35 branch according to my tests. I also tested with the .24 branch but got another (unrelated) "invalid search" error, which totally breaks the browser (no cards are shown and no search works at all).

ijgnd commented 2 years ago

@AnKingMed: It seems that all Advanced Browser versions since 2.1.24 were affected by this issue, see #148 and #149.

If you want to fix this for the branches for anki <.41: I would do a minimal modification of the AB-Versions that are on ankiweb: just backport the patch from abdo: download them, modify three lines that abdo changed, and re-upload them to ankiweb to the same branch (for details see the last paragraph in #148 and #149).

ijgnd commented 2 years ago

@AnKingMed : While looking into this I also found another unrelated bug for the anki version 2.1.22, see #147 that you could would "fix"/work around by only adjusting the compatibility info in the listing on ankiweb.

AnKingMed commented 2 years ago

I'm on service this whole week and on vacation next week. Are you able to merge these? I can quickly upload to Ankiweb but I won't have time for testing and such before then. You should have permissions. Let me know if not!

ijgnd commented 2 years ago

@AnKingMed : I'll post the files in the next two hours.

Important:

It was wrong to upload the latest AB version to the add-on branch for Anki .41-.44. Luckily abdnh tested it and just mentioned this in #151. I should have been clearer when asking you for uploading the new version.

Do you have an archive of uploaded old versions? I'll start looking for an old version or rebuild it from the last commit before rumo made the changes for 2.1.45, i.e. the AB version 3.9 from this commit. I'll also post what I have in two hours for this.

ijgnd commented 2 years ago

@AnKingMed : correction to last post from 15minutes ago: For 2.1.41-44 we also need a new file that fixes the overdue ivl problem so there's no point in reuploading the prior version.

ijgnd commented 2 years ago

@AnKingMed: here are the relevant files

upload these files to ankiweb:

on ankiweb change the compatibility info for the remaining branches (without modiying these files):

AnKingMed commented 2 years ago

Thank you for doing that. I have updated all of those on AnkiWeb. Please let me know if there's anything else I need to do

ijgnd commented 2 years ago

@AnkingMed: I also have an updated description for ankiweb: #153 - just copy the text from my top post into the ankiweb page. At the top right of my post there's a little box that you can click to copy the whole content of the source code box. I mostly changed the formatting (many empty lines in the changelog and I replaced the word "changes for" with one heading) and I made some small corrections.

AnKingMed commented 2 years ago

Done!