obsidian-spaced-repetition icon indicating copy to clipboard operation
obsidian-spaced-repetition copied to clipboard

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

Open MostlyArmless opened this issue 3 years ago • 8 comments

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.

MostlyArmless avatar Oct 08 '22 02:10 MostlyArmless

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 avatar Jan 08 '23 15:01 st3v3nmw

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

pikatwinky avatar Dec 18 '23 20:12 pikatwinky

Sure I'll take a look tonight.

MostlyArmless avatar Dec 19 '23 00:12 MostlyArmless

Is this still in progress?

4Source avatar Jan 11 '24 09:01 4Source

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?

ronzulu avatar Jan 11 '24 09:01 ronzulu

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

4Source avatar Jan 11 '24 21:01 4Source

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: https://github.com/st3v3nmw/obsidian-spaced-repetition/pull/834

Cheers Ronny

ronzulu avatar Jan 12 '24 00:01 ronzulu

Hi @MostlyArmless have you had a chance to look at https://github.com/st3v3nmw/obsidian-spaced-repetition/pull/834?

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

Cheers Ronny

ronzulu avatar Jan 18 '24 10:01 ronzulu

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

ronzulu avatar Mar 10 '24 12:03 ronzulu