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

Bug 495 card should be added to all tagged decks not just the first one #506

Conversation

MostlyArmless
Copy link
Contributor

I have a change which fixes #495, allowing a card to be added simultaneously to multiple decks if the note is tagged with multiple valid deck tags.

This does lead to some questionable behavior, where if a user tags the same note with #parentTag and #parentTag/childTag, then the cards in that note will be single-counted in the #parentTag/childTag deck (which is fine), but double-counted in the #parentTag deck (which seems weird, but hey it's what they've literally asked for by doing that).

Example of what I mean

Here's the contents of the note:
image

and here's the resulting UI from this plugin when you use the 'cram flashcards in this note' command:
image

I'm not sure if this is how we want the plugin to behave, or if we would perhaps like to make it configurable by having a setting that determines what happens in this situation. I'm open to suggestions, as I haven't been using the obsidian-spaced-repetition plugin personally for very long so I don't have a good feel for what makes sense here.

The side effect of this is that cards are double-counted in the parentTag deck if they are tagged with both #parentTag and #parentTag/childTag. This seems odd but if that's how the user is choosing to do it, I think we should respect that. Will look into making a setting to control this behavior next.
@st3v3nmw
Copy link
Owner

st3v3nmw commented Jan 8, 2023

This does lead to some questionable behavior, where if a user tags the same note with #parentTag and #parentTag/childTag, then the cards in that note will be single-counted in the #parentTag/childTag deck (which is fine), but double-counted in the #parentTag deck (which seems weird, but hey it's what they've literally asked for by doing that).

Yeah, this approach would break a few things like the counting as you've mentioned and we'd have the card in multiple decks, so if we review it the first time in deck A, the same card will show up later in the same review session (in deck A/B).

We'll probably need to rewrite parts of this plugin to use hash(cardText) as IDs to make it easier to remove/pick/update the same card in multiple decks

@st3v3nmw st3v3nmw self-requested a review as a code owner February 6, 2023 10:37
Copy link
Owner

@st3v3nmw st3v3nmw left a comment

Choose a reason for hiding this comment

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

Hi, thanks for the contribution 😄!

Sorry the underlying code has gone through a major refactor recently. I'm not sure if you're still available to help resolve the merge conflicts.

Stephen,

@pikatwinky
Copy link

Any chances to check this merge conflicts @MostlyArmless ? This PR looks really really promising...

@MostlyArmless
Copy link
Contributor Author

Sure I'll take a look tonight.

@4Source
Copy link
Contributor

4Source commented Jan 11, 2024

Is this still in progress?

@ronzulu
Copy link
Collaborator

ronzulu commented Jan 11, 2024

Hi @4Source

Yes, it's actually been quite involved, but I'm at the tail end. I'm hoping to release a beta version in the next couple of days. Will you be able to test and give me feedback?

@4Source
Copy link
Contributor

4Source commented Jan 11, 2024

I think that should be possible. I'll wait for the release. Will you mark this here when the time comes?

@ronzulu
Copy link
Collaborator

ronzulu commented Jan 12, 2024

Hi @4Source @MostlyArmless @pikatwinky (and anyone else)

I have completed the update but would appreciate beta testing and feedback before I finalise the PR.

The draft PR together with the updated code is available at:
#834

Cheers
Ronny

@ronzulu
Copy link
Collaborator

ronzulu commented Jan 18, 2024

Hi @MostlyArmless have you had a chance to look at #834?

After all your good work with this PR, I want to make sure you are happy with the final result.

Cheers
Ronny

@ronzulu
Copy link
Collaborator

ronzulu commented Mar 10, 2024

This has been superseded by #834, which was based on the refactored code base.

@ronzulu ronzulu closed this Mar 10, 2024
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.

[BUG] Obsidian SRS ignores more than one tag
5 participants