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

Add card position column and always show position in card info #3471

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

taylorobyen
Copy link
Contributor

Implements #3336

  • New position column that displays the original position of the card, supported with cards and notes sortable
    image
  • Original position now always show in card info
    image

@@ -205,6 +206,36 @@ fn add_regexp_tags_function(db: &Connection) -> rusqlite::Result<()> {
)
}

/// eg. get_original_position(c.due, c.data) -> i64
/// Check if c.data contains the position, if not it's still in c.due
fn add_get_original_position_function(db: &Connection) -> rusqlite::Result<()> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar routines above all use extract_ in the name; using it here too would be consistent.

I also I think this routine would be better off focusing on returning position | null. The caller can coalesce the returned null value to card.due if they want to fall back on it. That said, do we really want to be doing that? Can't we just leave cards without an original position unsorted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The caller can coalesce the returned null value to card.due if they want to fall back on it. That said, do we really want to be doing that? Can't we just leave cards without an original position unsorted?

Unless I'm misunderstanding something, every card will always have an original position. However, the column that stores it will change.

  1. User makes card, original position is in the due column
  2. User reviews card, due is replaced with a timestamp of next review and original position is pushed to the data column

It makes senses to fallback to the due column since the original position has to be there if it isn't in data column yet.

rslib/src/search/note_original_position_order.sql Outdated Show resolved Hide resolved
@dae
Copy link
Member

dae commented Oct 4, 2024

Original position now always show in card info

How does the user know if the card has an original position or not? Instead of falling back on due if position is unavailable, wouldn't it be clearer to show them separately when they differ?

@taylorobyen
Copy link
Contributor Author

How does the user know if the card has an original position or not? Instead of falling back on due if position is unavailable, wouldn't it be clearer to show them separately when they differ?

Based off my response to your other comment it makes the most sense to display it the same, regardless of the source. I'm assuming our end users don't care what column the original position is being sourced from as long as it is the actual original position. Now if you're aware of edge cases of the original position not existing in the card's data even after first review then I'll have to handle that, but in my testing I didn't see that occurring.

@dae
Copy link
Member

dae commented Oct 6, 2024

  • "Position" is the order new cards will appear in (the due #). It's only relevant for new cards.
  • "Original position" is assigned when new cards are answered for the first time. It exists solely to restore the due # if the cards are reset to new.
  • Anki didn't used to assign an original position, so non-new cards that were answered with older Anki versions will not have an original position either.
  • When an original position is missing, and the card is new, it's arguable whether the current position is also the original position. But if the card is not new, then falling back to card.due is not really valid.

The use case I'm imagining: the user may have a mix of new and non-new cards, and want to sort them in the order they originally were in, which means sorting based on coalesce(original, current).

@taylorobyen
Copy link
Contributor Author

taylorobyen commented Oct 6, 2024

The use case I'm imagining: the user may have a mix of new and non-new cards, and want to sort them in the order they originally were in, which means sorting based on coalesce(original, current).

Isn't this what my current implementation does?

image
image

@taylorobyen
Copy link
Contributor Author

@brishtibheja Do you have any input on the implementation of this? I saw you were on the original thread. I have an idea of what Damien wants but any clarifying details would be helpful.

@taylorobyen
Copy link
Contributor Author

taylorobyen commented Oct 6, 2024

Added handling of cards without position data, cards that aren't new won't attempt to show the due value as the position.
image
image

@brishtibheja
Copy link
Contributor

@taylorobyen I think going forward, we should make a distinction between position and due. In your screenshots, I see due column showing the current position (due #) of the card (and position is showing original position). I have seen users getting confused over why due is not showing them a date. @dae thoughts?

Also, to remind everyone of the use case brought up in the forums, it was mainly focused on creating shared decks while studying the decks. In such a situation, position helps us reposition the cards in order while creating them.

Lets say I reset all my cards without original position info, what will position show?

@taylorobyen
Copy link
Contributor Author

So, making the distinction would mean an Original Position and a Current Position column with them also being displayed separately in the card info window?

  • Original Position: Value only shown if able to be parsed from data
  • Current Position: Value only shown if it's a new card and this is derived from due

Lets say I reset all my cards without original position info, what will position show?

A new position is assigned to the card and that position will be shown.

@brishtibheja
Copy link
Contributor

making the distinction would mean an Original Position and a Current Position column

I simply meant not showing current due # in due column and instead show this under position. I think it is confusing for many reasons, one of them being we already use the word due to mean several different things. It makes sense to use due to only mean due date in the UI/documentation.

One thing I'll point out is that the reposition feature should always change the value in position and not due #. If it doesn't users can be left confused. Same with the two position based sort orders.

As for original due vs current due I think they only differ if you reset all of your cards without Restore original position. I think it makes sense to just show the current position here but I'll defer to dae's wisdom.

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

Successfully merging this pull request may close these issues.

3 participants