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

Fixing missing cards #701

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

richardrl
Copy link
Contributor

@richardrl richardrl commented Jul 3, 2023

Addresses #690 and #685

Fixing #690 means the parser now accepts white space before and after the separator.

Fixing #685 required breaking changes.

I was missing hundreds of cards as described in #685 - it turns out because these cards were not perfectly formatted. It is impossible to manually search through 1000s of cards to find the few that did not have the separators applied properly.

So, I have created a new note type CardType.Note which does not require a backside. It just has an empty string. Any note which does not have a separator, will still have a card generated under this new default card type.

Note: My change for #685 is breaking because we can no longer have multiple multiline cards in one Obsidian note. I am arguing that this change is an important improvement in the design of OSR. Previously, we used white space to generate multiple cards within one note. This leads to extremely poorly formatted cards when you have a lot of text - imagine trying to read paragraphs of text with no line breaks!

This new design decision says that each obsidian note must correspond to one card. I believe this is the correct choice - the only other alternative I see is to require some custom "note-break" symbol like "END_CARD" - but this is tedious and harms readability of cards. Forcing each note to be one card allows us to have expressive notes, with line breaks, and does not require us to insert nasty note-break symbols throughout our notes.

I believe these changes, particularly #685 are extremely important to the usability of the app. Previously, I was seriously hindered in my studies as hundreds of notes were invisible!

Under my new system in this PR, if you mis-format a note, you can easily find it later as you are studying your queue of cards. Then, if you wish, you can re-format your note into a nice front-back Q&A format.

Here is a screenshot showing the number of cards before these changes:

image

Old version of OSR:
image

Welcome your thoughts!
@AB1908 @st3v3nmw

@ronzulu
Copy link
Collaborator

ronzulu commented Sep 30, 2023

Hi Richard

Great that you have fixed this, but are you sure that all existing users would agree with the breaking change.

The plug-in has been downloaded over 120,000 times (amazing!). I would imagine that there would be some portion of the user base that expects the current functionality to continue.

Personally, I typically have multiple questions within the same note, albeit usually single line. Still, I don't want to have to go through all my notes to find the ones that have multiple multi-line questions, and extract into separate files.

I understand that in your workflow, you temporarily have a note with a flashcard tag, as well as the front side.

If there was a way to find all notes that have the required tag but no questions (both front/back) would that solve the issue?

Regards
Ronny

@ronzulu
Copy link
Collaborator

ronzulu commented Oct 1, 2023

Hello again

Another idea is if we essentially implement #729.

This would mean that any note that is suitably tagged, but doesn't have any questions, would appear as:
Front - Filename
Back - Contents of the note.

Cheers
Ronny

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.

I also agree with @ronzulu, this would break the UX for many users.

Stephen,

Copy link
Owner

Choose a reason for hiding this comment

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

Please use pnpm instead

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