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

Missing flashcards (bug introduced in 1.12.0) #927

Merged
merged 7 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/changelog.md
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
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
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
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
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
Loading