Skip to content

Commit

Permalink
Missing flashcards (bug introduced in 1.12.0) (#927)
Browse files Browse the repository at this point in the history 
* Fixed and ready for beta testing

* Fixed for case where topic tag is at the end of the question

* Updated comment

* Comments, change log etc

* lint and format
  • Loading branch information
@ronzulu
ronzulu authored Apr 18, 2024
1 parent 5f4f784 commit 2e76981
Show file tree
Hide file tree
Showing 8 changed files with 343 additions and 55 deletions.
6 changes: 6 additions & 0 deletions docs/changelog.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. Dates are d

Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).

#### [Unreleased]

- [BUG] Plugin not picking up certain flashcards https://github.com/st3v3nmw/obsidian-spaced-repetition/issues/915
- [BUG] Unable to recognize multi-line card that begins immediately after the frontmatter's closing line https://github.com/st3v3nmw/obsidian-spaced-repetition/issues/922
- [BUG] Most of my flashcards are now missing https://github.com/st3v3nmw/obsidian-spaced-repetition/issues/923

#### [1.12.3](https://github.com/st3v3nmw/obsidian-spaced-repetition/compare/1.12.2...1.12.3)

- Fixed slow load of flashcard modal (bug introduced in 1.12.0) [`#926`](https://github.com/st3v3nmw/obsidian-spaced-repetition/pull/926)
Expand Down
140 changes: 103 additions & 37 deletions src/NoteQuestionParser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { parseEx, ParsedQuestionInfo } from "./parser";
import { Question, QuestionText } from "./Question";
import { CardFrontBack, CardFrontBackUtil } from "./QuestionType";
import { SRSettings, SettingsUtil } from "./settings";
import { ISRFile } from "./SRFile";
import { ISRFile, frontmatterTagPseudoLineNum } from "./SRFile";
import { TopicPath, TopicPathList } from "./TopicPath";
import { extractFrontmatter, splitTextIntoLineArray } from "./util/utils";

Expand All @@ -14,10 +14,24 @@ export class NoteQuestionParser {
noteFile: ISRFile;
folderTopicPath: TopicPath;
noteText: string;
frontmatterText: string;

// This is the note text, but with the frontmatter blanked out (see extractFrontmatter for reasoning)
contentText: string;
noteLines: string[];

// Complete list of tags
tagCacheList: TagCache[];

// tagCacheList filtered to those specified in the user settings (e.g. "#flashcards")
flashcardTagList: TagCache[];

// flashcardTagList filtered to those within the frontmatter
frontmatterTopicPathList: TopicPathList;

// flashcardTagList filtered to those within the note's content and are note-level tags (i.e. not question specific)
contentTopicPathInfo: TopicPathList[];

questionList: Question[];

constructor(settings: SRSettings) {
Expand Down Expand Up @@ -47,6 +61,7 @@ export class NoteQuestionParser {

// The following analysis can require fair computation.
// There is no point doing it if there aren't any topic paths
[this.frontmatterText, this.contentText] = extractFrontmatter(noteText);

// Create the question list
this.questionList = this.doCreateQuestionList(
Expand Down Expand Up @@ -114,9 +129,10 @@ export class NoteQuestionParser {
}

private parseQuestions(): ParsedQuestionInfo[] {
// We pass contentText which has the frontmatter blanked out; see extractFrontmatter for reasoning
const settings: SRSettings = this.settings;
const result: ParsedQuestionInfo[] = parseEx(
this.noteText,
this.contentText,
settings.singleLineCardSeparator,
settings.singleLineReversedCardSeparator,
settings.multilineCardSeparator,
Expand Down Expand Up @@ -178,60 +194,104 @@ export class NoteQuestionParser {
// within frontmatter appear on separate lines)
//
private analyseTagCacheList(tagCacheList: TagCache[]): [TopicPathList, TopicPathList[]] {
let frontmatterTopicPathList: TopicPathList = null;
const contentTopicPathList: TopicPathList[] = [] as TopicPathList[];
// The tag (e.g. "#flashcards") must be a valid flashcard tag as per the user settings
this.flashcardTagList = tagCacheList.filter((item) =>
SettingsUtil.isFlashcardTag(this.settings, item.tag),
);
if (this.flashcardTagList.length > 0) {
// To simplify analysis, sort the flashcard list ordered by line number
this.flashcardTagList.sort((a, b) => a.position.start.line - b.position.start.line);
}

let frontmatterLineCount: number = 0;
if (this.frontmatterText) {
frontmatterLineCount = splitTextIntoLineArray(this.frontmatterText).length;
}

const frontmatterTopicPathList: TopicPathList = this.determineFrontmatterTopicPathList(
this.flashcardTagList,
frontmatterLineCount,
);
const contentTopicPathList: TopicPathList[] = this.determineContentTopicPathList(
this.flashcardTagList,
frontmatterLineCount,
);

return [frontmatterTopicPathList, contentTopicPathList];
}

private determineFrontmatterTopicPathList(
flashcardTagList: TagCache[],
frontmatterLineCount: number,
): TopicPathList {
let result: TopicPathList = null;

// Only keep tags that are:
// Filter for tags that are:
// 1. specified in the user settings as flashcardTags, and
// 2. is not question specific (determined by line number)
const filteredTagCacheList: TagCache[] = tagCacheList.filter(
// 2. is not question specific (determined by line number) - i.e. is "note level"
const noteLevelTagList: TagCache[] = flashcardTagList.filter(
(item) =>
SettingsUtil.isFlashcardTag(this.settings, item.tag) &&
this.questionList.every(
(q) => !q.parsedQuestionInfo.isQuestionLineNum(item.position.start.line),
),
item.position.start.line == frontmatterTagPseudoLineNum &&
this.isNoteLevelFlashcardTag(item),
);
let frontmatterLineCount: number = null;
if (filteredTagCacheList.length > 0) {
// To simplify analysis, ensure that the supplied list is ordered by line number
filteredTagCacheList.sort((a, b) => a.position.start.line - b.position.start.line);

if (noteLevelTagList.length > 0) {
// Treat the frontmatter slightly differently (all tags grouped together even if on separate lines)
const [frontmatter, _] = extractFrontmatter(this.noteText);
if (frontmatter) {
frontmatterLineCount = splitTextIntoLineArray(frontmatter).length;
const frontmatterTagCacheList = filteredTagCacheList.filter(
if (this.frontmatterText) {
const frontmatterTagCacheList = noteLevelTagList.filter(
(item) => item.position.start.line < frontmatterLineCount,
);

// Doesn't matter what line number we specify, as long as it's less than frontmatterLineCount
if (frontmatterTagCacheList.length > 0)
frontmatterTopicPathList = this.createTopicPathList(frontmatterTagCacheList, 0);
result = this.createTopicPathList(
frontmatterTagCacheList,
frontmatterTagPseudoLineNum,
);
}
}
//
const contentStartLineNum: number = frontmatterLineCount > 0 ? frontmatterLineCount + 1 : 0;
const contentTagCacheList: TagCache[] = filteredTagCacheList.filter(
(item) => item.position.start.line >= contentStartLineNum,
return result;
}

private determineContentTopicPathList(
flashcardTagList: TagCache[],
frontmatterLineCount: number,
): TopicPathList[] {
const result: TopicPathList[] = [] as TopicPathList[];

// NOTE: Line numbers are zero based, therefore don't add 1 to frontmatterLineCount to get contentStartLineNum
const contentStartLineNum: number = frontmatterLineCount;
const contentTagCacheList: TagCache[] = flashcardTagList.filter(
(item) =>
item.position.start.line >= contentStartLineNum &&
this.isNoteLevelFlashcardTag(item),
);

// We group together all tags that are on the same line, taking advantage of flashcardTagList being ordered by line number
let list: TagCache[] = [] as TagCache[];
for (const t of contentTagCacheList) {
for (const tag of contentTagCacheList) {
if (list.length != 0) {
const startLineNum: number = list[0].position.start.line;
if (startLineNum != t.position.start.line) {
contentTopicPathList.push(this.createTopicPathList(list, startLineNum));
if (startLineNum != tag.position.start.line) {
result.push(this.createTopicPathList(list, startLineNum));
list = [] as TagCache[];
}
}
list.push(t);
list.push(tag);
}
if (list.length > 0) {
const startLineNum: number = list[0].position.start.line;
contentTopicPathList.push(this.createTopicPathList(list, startLineNum));
result.push(this.createTopicPathList(list, startLineNum));
}
return result;
}

return [frontmatterTopicPathList, contentTopicPathList];
private isNoteLevelFlashcardTag(tagItem: TagCache): boolean {
const tagLineNum: number = tagItem.position.start.line;

// Check that the tag is not question specific (determined by line number)
const isQuestionSpecific: boolean = this.questionList.some((q) =>
q.parsedQuestionInfo.isQuestionLineNum(tagLineNum),
);
return !isQuestionSpecific;
}

private createTopicPathList(tagCacheList: TagCache[], lineNum: number): TopicPathList {
Expand All @@ -242,6 +302,11 @@ export class NoteQuestionParser {
return new TopicPathList(list, lineNum);
}

private createTopicPathList_FromSingleTag(tagCache: TagCache): TopicPathList {
const list: TopicPath[] = [TopicPath.getTopicPathFromTag(tagCache.tag)];
return new TopicPathList(list, tagCache.position.start.line);
}

//
// A question can be associated with multiple topics (hence returning TopicPathList and not just TopicPath).
//
Expand All @@ -268,17 +333,18 @@ export class NoteQuestionParser {

// Find the last TopicPathList prior to the question (in the order present in the file)
for (let i = this.contentTopicPathInfo.length - 1; i >= 0; i--) {
const info: TopicPathList = this.contentTopicPathInfo[i];
if (info.lineNum < question.parsedQuestionInfo.firstLineNum) {
result = info;
const topicPathList: TopicPathList = this.contentTopicPathInfo[i];
if (topicPathList.lineNum < question.parsedQuestionInfo.firstLineNum) {
result = topicPathList;
break;
}
}

// For backward compatibility with functionality pre https://github.com/st3v3nmw/obsidian-spaced-repetition/issues/495:
// if nothing matched, then use the first one
if (!result && this.contentTopicPathInfo.length > 0) {
result = this.contentTopicPathInfo[0];
// This could occur if the only topic tags present are question specific
if (!result && this.flashcardTagList.length > 0) {
result = this.createTopicPathList_FromSingleTag(this.flashcardTagList[0]);
}
}
}
Expand Down
17 changes: 10 additions & 7 deletions src/SRFile.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
} from "obsidian";
import { parseObsidianFrontmatterTag } from "./util/utils";

// NOTE: Line numbers are zero based
export interface ISRFile {
get path(): string;
get basename(): string;
Expand All @@ -19,6 +20,11 @@ export interface ISRFile {
write(content: string): Promise<void>;
}

// The Obsidian frontmatter cache doesn't include the line number for the specific tag.
// We define as -1 so that we can differentiate tags within the frontmatter and tags within the content
export const frontmatterTagPseudoLineNum: number = -1;

// NOTE: Line numbers are zero based
export class SrTFile implements ISRFile {
file: TFile;
vault: Vault;
Expand Down Expand Up @@ -48,6 +54,7 @@ export class SrTFile implements ISRFile {
const result: TagCache[] = [] as TagCache[];
const fileCachedData = this.metadataCache.getFileCache(this.file) || {};
if (fileCachedData.tags?.length > 0) {
// console.log(`getAllTagsFromText: tags: ${fileCachedData.tags.map((item) => `(${item.position.start.line}: ${item.tag})`).join("|")}`);
result.push(...fileCachedData.tags);
}

Expand All @@ -63,19 +70,14 @@ export class SrTFile implements ISRFile {
const result: TagCache[] = [] as TagCache[];
const frontmatterTags: string = frontmatter != null ? frontmatter["tags"] + "" : null;
if (frontmatterTags) {
// The frontmatter doesn't include the line number for the specific tag, defining as line 1 is good enough.
// (determineQuestionTopicPathList() only needs to know that these frontmatter tags come before all others
// in the file)
const line: number = 1;

// Parse the frontmatter tag string into a list, each entry including the leading "#"
const tagStrList: string[] = parseObsidianFrontmatterTag(frontmatterTags);
for (const str of tagStrList) {
const tag: TagCache = {
tag: str,
position: {
start: { line: line, col: null, offset: null },
end: { line: line, col: null, offset: null },
start: { line: frontmatterTagPseudoLineNum, col: null, offset: null },
end: { line: frontmatterTagPseudoLineNum, col: null, offset: null },
},
};
result.push(tag);
Expand All @@ -87,6 +89,7 @@ export class SrTFile implements ISRFile {
getQuestionContext(cardLine: number): string[] {
const fileCachedData = this.metadataCache.getFileCache(this.file) || {};
const headings: HeadingCache[] = fileCachedData.headings || [];
// console.log(`getQuestionContext: headings: ${headings.map((item) => `(${item.position.start.line}: ${item.heading})`).join("|")}`);
const stack: HeadingCache[] = [];
for (const heading of headings) {
if (heading.position.start.line > cardLine) {
Expand Down
7 changes: 7 additions & 0 deletions src/parser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ export class ParsedQuestionInfo {
/**
* Returns flashcards found in `text`
*
* It is best that the text does not contain frontmatter, see extractFrontmatter for reasoning
*
* Multi-line question with blank lines user workaround:
* As of 3/04/2024 there is no support for including blank lines within multi-line questions
* As a workaround, one user uses a zero width Unicode character - U+200B
* https://github.com/st3v3nmw/obsidian-spaced-repetition/issues/915#issuecomment-2031003092
*
* @param text - The text to extract flashcards from
* @param singlelineCardSeparator - Separator for inline basic cards
* @param singlelineReversedCardSeparator - Separator for inline reversed cards
Expand Down
23 changes: 17 additions & 6 deletions src/util/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,18 @@ export function stringTrimStart(str: string): [string, string] {
return [ws, trimmed];
}

//
// This returns [frontmatter, content]
//
// The returned content has the same number of lines as the supplied str string, but with the
// frontmatter lines (if present) blanked out.
//
// 1. We don't want the parser to see the frontmatter, as it would deem it to be part of a multi-line question
// if one started on the line immediately after the "---" closing marker.
//
// 2. The lines are blanked out rather than deleted so that line numbers are not affected
// e.g. for calls to getQuestionContext(cardLine: number)
//
export function extractFrontmatter(str: string): [string, string] {
let frontmatter: string = "";
let content: string = "";
Expand All @@ -113,12 +125,11 @@ export function extractFrontmatter(str: string): [string, string] {

if (frontmatterEndLineNum) {
const frontmatterStartLineNum: number = 0;
const frontmatterLineCount: number =
frontmatterEndLineNum - frontmatterStartLineNum + 1;
const frontmatterLines: string[] = lines.splice(
frontmatterStartLineNum,
frontmatterLineCount,
);
const frontmatterLines: string[] = [];
for (let i = frontmatterStartLineNum; i <= frontmatterEndLineNum; i++) {
frontmatterLines.push(lines[i]);
lines[i] = "";
}
frontmatter = frontmatterLines.join("\n");
content = lines.join("\n");
}
Expand Down
Loading

0 comments on commit 2e76981

Please sign in to comment.