diff --git a/FOLLOWUP.md b/FOLLOWUP.md index e80f690..e4b62d1 100644 --- a/FOLLOWUP.md +++ b/FOLLOWUP.md @@ -101,13 +101,39 @@ the verse never appears, so it can't linger. --- -## 5. Smaller cleanups noticed in passing +## 5. Smaller cleanups noticed in passing — ✅ RESOLVED -- `src/services/BibleContentService.ts` and `src/services/ESVApiService.ts` both - contain `// TODO` notes about duplicated ESV-response → `BiblePassage` - conversion logic that could be unified. -- `src/services/BibleChapterFiles.ts` has a `// TODO - logic in this class needs - to move to BibleFiles` note; `BibleFiles.ts` has TODOs for `openChapterNote` / - `openPassageNote` helpers. -- `openChapterNote` takes a `string` and re-parses it; several call sites already - hold a `BibleReference` and could pass it directly (existing TODO). +**Files:** `src/services/BibleContentService.ts`, `src/services/ESVApiService.ts`, +`src/services/BibleFiles.ts`, `src/services/BibleChapterFiles.ts` (removed), +`src/core/DisciplesJournalPlugin.ts`, `src/components/OpenBibleModal.ts`, +`src/components/BibleNavigation.ts`, `src/components/BibleReferenceRenderer.ts` + +**What was wrong:** +- `BibleContentService` and `ESVApiService` each had their own copy of the + ESV-response → `BiblePassage` conversion (parse `canonical`, take `passages[0]`), + flagged by matching `// TODO` notes. +- `BibleChapterFiles` carried a `// TODO - logic in this class needs to move to + BibleFiles` note while `BibleFiles` had placeholder TODOs for `openChapterNote` / + `openPassageNote` — the two classes were the same responsibility split in half. +- `openChapterNote` took a `string` and re-parsed it via `BibleReference.parse`, + even though every call site already had (or trivially built) a `BibleReference`. +- `BibleChapterFiles` also had a dead `getFullContentPath()` duplicating the one in + `BibleFiles`, and `ESVApiService.downloadFromESVApi` had a stale "should take a + `BibleReference`" TODO (it already did). + +**Fix applied:** +- The conversion now lives in one place: `ESVApiService.toBibleApiResponse(data, ref)` + (a static helper accepting the loosely-typed shape from both a fresh API response + and a note's parsed frontmatter). `downloadFromESVApi` and + `BibleContentService.getBibleContent` both call it; the duplicate private method + and both TODOs are gone. +- `BibleChapterFiles` was merged into `BibleFiles` and deleted. `BibleFiles` now + owns both the static path/file lookups (unchanged) and instance methods + `openChapterNote` / `createChapterNote` / `scrollToVerse` (constructed once in + `onload` with the content service). The static lookup split keeps + `BibleContentService` / `ESVApiService` free of a circular dependency (the type-only + `BibleContentService` import is erased at runtime). +- `openChapterNote` now takes a `BibleReference`; the modal, navigation, renderer, + and plugin call sites pass one directly (no more `toString()` → re-parse round-trip), + and its invalid-string guard is gone since the type guarantees validity. +- The dead `getFullContentPath()` and the stale `downloadFromESVApi` TODO were removed. diff --git a/src/components/BibleNavigation.ts b/src/components/BibleNavigation.ts index cfb8e65..d8f6ff7 100644 --- a/src/components/BibleNavigation.ts +++ b/src/components/BibleNavigation.ts @@ -1,18 +1,18 @@ import {App, ButtonComponent, DropdownComponent, Notice} from "obsidian"; import {BibleReference} from "../core/BibleReference"; import {BookNames} from "../services/BookNames"; -import {BibleChapterFiles} from "../services/BibleChapterFiles"; +import {BibleFiles} from "../services/BibleFiles"; import { BookSuggest } from "./BookSuggest"; /** * Component for generating Bible navigation elements */ export class BibleNavigation { - private bibleBookFiles: BibleChapterFiles; + private bibleFiles: BibleFiles; private app: App; - constructor(noteCreationService: BibleChapterFiles, app: App) { - this.bibleBookFiles = noteCreationService; + constructor(bibleFiles: BibleFiles, app: App) { + this.bibleFiles = bibleFiles; this.app = app; } @@ -186,7 +186,7 @@ export class BibleNavigation { */ public async navigateToChapter(book: string, chapter: number): Promise { try { - await this.bibleBookFiles.openChapterNote(new BibleReference(book, chapter).toString()); + await this.bibleFiles.openChapterNote(new BibleReference(book, chapter)); } catch (error) { console.error('Error navigating to chapter:', error); const message = error instanceof Error ? error.message : String(error); diff --git a/src/components/BibleReferenceRenderer.ts b/src/components/BibleReferenceRenderer.ts index 3f6c930..ad89a42 100644 --- a/src/components/BibleReferenceRenderer.ts +++ b/src/components/BibleReferenceRenderer.ts @@ -3,7 +3,7 @@ import {BibleContentService} from "../services/BibleContentService"; import {BibleNavigation} from "./BibleNavigation"; import DisciplesJournalPlugin from "src/core/DisciplesJournalPlugin"; import {BibleEventHandlers} from "src/core/BibleEventHandlers"; -import {BibleChapterFiles} from "../services/BibleChapterFiles"; +import {BibleFiles} from "../services/BibleFiles"; import {BibleReference} from "../core/BibleReference"; import {BiblePassage} from "../utils/BiblePassage"; @@ -18,12 +18,12 @@ export class BibleReferenceRenderer { constructor( bibleContentService: BibleContentService, - noteCreationService: BibleChapterFiles, + bibleFiles: BibleFiles, plugin: DisciplesJournalPlugin ) { this.bibleContentService = bibleContentService; this.plugin = plugin; - this.bibleNavigation = new BibleNavigation(noteCreationService, plugin.app); + this.bibleNavigation = new BibleNavigation(bibleFiles, plugin.app); } /** @@ -160,8 +160,7 @@ export class BibleReferenceRenderer { try { // Call the method to open the chapter note - // TODO - openChapterNote should take in a BibleReference instead of string - void this.plugin.openChapterNote(passage.reference.toString()); + void this.plugin.openChapterNote(passage.reference); // Close the preview - using a method available in BibleEventHandlers // Remove the popup directly instead of trying to access the private property diff --git a/src/components/OpenBibleModal.ts b/src/components/OpenBibleModal.ts index 9e48dd1..b7766de 100644 --- a/src/components/OpenBibleModal.ts +++ b/src/components/OpenBibleModal.ts @@ -1,6 +1,7 @@ import {App, SuggestModal, Notice} from "obsidian"; import {BookNames} from "../services/BookNames"; -import {BibleChapterFiles} from "../services/BibleChapterFiles"; +import {BibleFiles} from "../services/BibleFiles"; +import {BibleReference} from "../core/BibleReference"; interface BibleTarget { label: string; @@ -13,12 +14,12 @@ interface BibleTarget { * First shows all books; after selecting a book, shows its chapters. */ export class OpenBibleModal extends SuggestModal { - private bibleBookFiles: BibleChapterFiles; + private bibleFiles: BibleFiles; private selectedBook: string | null = null; - constructor(app: App, bibleBookFiles: BibleChapterFiles) { + constructor(app: App, bibleFiles: BibleFiles) { super(app); - this.bibleBookFiles = bibleBookFiles; + this.bibleFiles = bibleFiles; this.setPlaceholder('Type a book name...'); } @@ -67,16 +68,16 @@ export class OpenBibleModal extends SuggestModal { if (chapterCount === 1) { // Single-chapter book, open directly const loadingNotice = new Notice("Loading chapter...", 0); - await this.bibleBookFiles.openChapterNote(`${item.book} 1`); + await this.bibleFiles.openChapterNote(new BibleReference(item.book, 1)); loadingNotice.hide(); } else { // Reopen for chapter selection - const chapterModal = new OpenBibleChapterModal(this.app, this.bibleBookFiles, item.book); + const chapterModal = new OpenBibleChapterModal(this.app, this.bibleFiles, item.book); chapterModal.open(); } } else { const loadingNotice = new Notice("Loading chapter...", 0); - await this.bibleBookFiles.openChapterNote(`${item.book} ${item.chapter}`); + await this.bibleFiles.openChapterNote(new BibleReference(item.book, item.chapter)); loadingNotice.hide(); } } @@ -86,12 +87,12 @@ export class OpenBibleModal extends SuggestModal { * Second-stage modal: pick a chapter within a specific book. */ class OpenBibleChapterModal extends SuggestModal { - private bibleBookFiles: BibleChapterFiles; + private bibleFiles: BibleFiles; private book: string; - constructor(app: App, bibleBookFiles: BibleChapterFiles, book: string) { + constructor(app: App, bibleFiles: BibleFiles, book: string) { super(app); - this.bibleBookFiles = bibleBookFiles; + this.bibleFiles = bibleFiles; this.book = book; this.setPlaceholder(`${book} -- type a chapter number...`); } @@ -123,7 +124,7 @@ class OpenBibleChapterModal extends SuggestModal { private async openChapter(item: BibleTarget): Promise { const loadingNotice = new Notice("Loading chapter...", 0); - await this.bibleBookFiles.openChapterNote(`${item.book} ${item.chapter}`); + await this.bibleFiles.openChapterNote(new BibleReference(item.book, item.chapter)); loadingNotice.hide(); } } diff --git a/src/core/DisciplesJournalPlugin.ts b/src/core/DisciplesJournalPlugin.ts index e47541a..54ba8f7 100644 --- a/src/core/DisciplesJournalPlugin.ts +++ b/src/core/DisciplesJournalPlugin.ts @@ -8,7 +8,7 @@ import { DEFAULT_SETTINGS, DisciplesJournalSettingsTab } from '../settings/DisciplesJournalSettings'; -import {BibleChapterFiles} from 'src/services/BibleChapterFiles'; +import {BibleFiles} from 'src/services/BibleFiles'; import {BibleMarkupProcessor} from './BibleMarkupProcessor'; import {createInlineReferenceExtension} from "../components/BibleReferenceInlineExtension"; import {BibleReference} from './BibleReference'; @@ -26,7 +26,7 @@ export default class DisciplesJournalPlugin extends Plugin { // Services private esvApiService: ESVApiService; private bibleContentService: BibleContentService; - private bibleBookFiles: BibleChapterFiles; + private bibleFiles: BibleFiles; // Components private bibleStyles: BibleStyles; @@ -49,11 +49,11 @@ export default class DisciplesJournalPlugin extends Plugin { // Initialize components this.bibleStyles = new BibleStyles(); - this.bibleBookFiles = new BibleChapterFiles(this, this.bibleContentService); + this.bibleFiles = new BibleFiles(this, this.bibleContentService); this.bibleReferenceRenderer = new BibleReferenceRenderer( this.bibleContentService, - this.bibleBookFiles, + this.bibleFiles, this ); @@ -84,7 +84,7 @@ export default class DisciplesJournalPlugin extends Plugin { this.addCommand({ id: 'open-bible', name: 'Open Bible', - callback: () => new OpenBibleModal(this.app, this.bibleBookFiles).open() + callback: () => new OpenBibleModal(this.app, this.bibleFiles).open() }); this.addCommand({ @@ -95,7 +95,7 @@ export default class DisciplesJournalPlugin extends Plugin { // Ribbon icon this.addRibbonIcon('book-open', 'Open Bible', () => { - new OpenBibleModal(this.app, this.bibleBookFiles).open(); + new OpenBibleModal(this.app, this.bibleFiles).open(); }); // Register settings tab @@ -160,8 +160,8 @@ export default class DisciplesJournalPlugin extends Plugin { /** * Open or create a chapter note (Public method for external access) */ - public async openChapterNote(reference: string) { - return this.bibleBookFiles.openChapterNote(reference); + public async openChapterNote(reference: BibleReference) { + return this.bibleFiles.openChapterNote(reference); } /** diff --git a/src/services/BibleChapterFiles.ts b/src/services/BibleChapterFiles.ts deleted file mode 100644 index e5731be..0000000 --- a/src/services/BibleChapterFiles.ts +++ /dev/null @@ -1,135 +0,0 @@ -import {Notice, TFile, WorkspaceLeaf} from 'obsidian'; -import {BibleContentService} from './BibleContentService'; -import {BibleReference} from '../core/BibleReference'; -import DisciplesJournalPlugin from "../core/DisciplesJournalPlugin"; -import {BibleApiResponse} from "../utils/BibleApiResponse"; -import {BibleFiles} from "./BibleFiles"; - -/** - * Service for creating and opening Bible (chapter) files - */ -// TODO - logic in this class needs to move to BibleFiles -export class BibleChapterFiles { - private plugin: DisciplesJournalPlugin; - private bibleContentService: BibleContentService; - - constructor( - plugin: DisciplesJournalPlugin, - bibleContentService: BibleContentService, - ) { - this.plugin = plugin; - this.bibleContentService = bibleContentService; - } - - /** - * Open or create a chapter note - */ - public async openChapterNote(reference: string) { - try { - // Parse the reference string to ensure it's valid - const parsedRef = BibleReference.parse(reference); - if (!parsedRef) { - console.error(`Invalid reference: ${reference}`); - new Notice(`Disciples Journal: "${reference}" is not a valid Bible reference.`, 10000); - return; - } - - const chapterPath = BibleFiles.pathForPassage(parsedRef, this.plugin); - if (!BibleFiles.fileExistsForPassage(parsedRef, this.plugin)) { - if (this.plugin.settings.downloadOnDemand) { - // TODO - create chapter note - const response = await this.createChapterNote(parsedRef); - if (response.isError()) { - // Surface the underlying reason (e.g. missing ESV API token) to the user - // rather than failing silently. - new Notice(`Disciples Journal: ${response.errorMessage}`, 10000); - return; - } - } else { - console.error(`Passage download disabled (see settings): ${chapterPath}`); - new Notice("Disciples Journal: On-demand passage download is turned off. Enable it in the plugin settings to open this chapter.", 10000); - return null; - } - } - - const passageNoteFile = BibleFiles.getFileForPassage(parsedRef, this.plugin); - if (passageNoteFile && passageNoteFile instanceof TFile) { - const leaf = this.plugin.app.workspace.getLeaf(false); - await leaf.openFile(passageNoteFile); - - // If there's a specific verse, scroll to it once it has rendered - if (parsedRef.verse) { - this.scrollToVerse(leaf, parsedRef.verse); - } - } else { - console.error(`Could not find or create the chapter note: ${chapterPath}`); - new Notice(`Disciples Journal: Could not open the chapter note for ${parsedRef.toString()}.`, 10000); - } - } catch (error) { - console.error('Error opening chapter note:', error); - const detail = error instanceof Error ? error.message : String(error); - new Notice(`Disciples Journal: Failed to open chapter note. ${detail}`, 10000); - } - } - - /** - * Scroll the opened note to a verse element. The note's body renders - * asynchronously after `openFile`, so rather than guessing with a fixed - * delay we watch the view for the verse element to appear and scroll the - * moment it does. A bounded fallback disconnects the observer if the verse - * never renders (e.g. wrong reference, or the view is in an edit mode that - * doesn't produce verse elements) so it can't linger. - */ - private scrollToVerse(leaf: WorkspaceLeaf, verse: number): void { - const container = leaf.getContainer(); - const selector = `.verse-${verse}`; - - const tryScroll = (): boolean => { - const verseEl = container.doc.querySelector(selector); - if (verseEl) { - verseEl.scrollIntoView({behavior: 'smooth', block: 'center'}); - return true; - } - return false; - }; - - // Already rendered (e.g. note was cached)? Scroll right away. - if (tryScroll()) { - return; - } - - const win = container.win; - const observer = new MutationObserver(() => { - if (tryScroll()) { - observer.disconnect(); - win.clearTimeout(fallbackId); - } - }); - observer.observe(leaf.view.containerEl, {childList: true, subtree: true}); - - const fallbackId = win.setTimeout(() => observer.disconnect(), 5000); - } - - /** - * Create a new chapter note. Returns the API response so callers can surface - * the reason for any failure (e.g. a missing ESV API token) to the user. - */ - private async createChapterNote(reference: BibleReference): Promise { - // Create reference only from book+chapter - const bookChapter = new BibleReference(reference.book, reference.chapter); - - // Get the content from the Bible API and format it for a note - const response: BibleApiResponse = await this.bibleContentService.getBibleContent(bookChapter); - if (response.isError()) { - console.error(`Failed to get Bible content for ${bookChapter.toString()}: ${response.errorMessage}`); - } - return response; - } - - /** - * Get the full path with version - */ - private getFullContentPath(): string { - return `${this.plugin.settings.bibleContentVaultPath}/${this.plugin.settings.preferredBibleVersion}`; - } -} diff --git a/src/services/BibleContentService.ts b/src/services/BibleContentService.ts index 8732640..1de72b5 100644 --- a/src/services/BibleContentService.ts +++ b/src/services/BibleContentService.ts @@ -40,7 +40,7 @@ export class BibleContentService { const frontmatter = getFrontMatterInfo(fileContent); if (frontmatter && frontmatter.frontmatter) { - return this.convertEsvApiResponseToGeneric(parseYaml(frontmatter.frontmatter), ref); + return ESVApiService.toBibleApiResponse(parseYaml(frontmatter.frontmatter), ref); } else { const message = `No frontmatter found in file for reference ${ref.toString()}. Loading from API instead.`; console.error(message); @@ -69,21 +69,6 @@ export class BibleContentService { } } - // TODO - I would like this to go away once I normalize things a bit. This code is duplicated within ESVAPIService - private convertEsvApiResponseToGeneric(frontmatter: unknown, ref: BibleReference) { - const fm = frontmatter as { canonical?: unknown; passages?: unknown } | null; - const canonical = fm && typeof fm.canonical === 'string' ? fm.canonical : null; - const canonicalRef = canonical ? BibleReference.parse(canonical) : null; - if (!canonicalRef) { - const message = `Failed to parse canonical reference (${String(canonical)}) from ESV API for ${ref.toString()}`; - console.error(message); - return BibleApiResponse.error(message, ErrorType.BadApiResponse); - } - const passages = fm && Array.isArray(fm.passages) ? (fm.passages as unknown[]) : []; - const html = typeof passages[0] === 'string' ? passages[0] : ''; - return BibleApiResponse.success(new BiblePassage(canonicalRef, html)); - } - private getCachedRef(ref: BibleReference): BiblePassage | undefined { if (this.passageCache.has(ref)) { return this.passageCache.get(ref); diff --git a/src/services/BibleFiles.ts b/src/services/BibleFiles.ts index add86af..48b7cd4 100644 --- a/src/services/BibleFiles.ts +++ b/src/services/BibleFiles.ts @@ -1,11 +1,25 @@ -import {normalizePath, TFile} from 'obsidian'; +import {Notice, normalizePath, TFile, WorkspaceLeaf} from 'obsidian'; import {BibleReference} from '../core/BibleReference'; import DisciplesJournalPlugin from "../core/DisciplesJournalPlugin"; +import {BibleApiResponse} from "../utils/BibleApiResponse"; +import type {BibleContentService} from './BibleContentService'; /** - * Service for creating and opening Bible (chapter) files + * Service for resolving, creating, and opening Bible passage/chapter notes. + * + * The path/file lookup helpers are static (they only need the plugin), while the + * note-opening helpers are instance methods because they depend on the content + * service used to download passages that aren't on disk yet. */ export class BibleFiles { + private plugin: DisciplesJournalPlugin; + private bibleContentService: BibleContentService; + + constructor(plugin: DisciplesJournalPlugin, bibleContentService: BibleContentService) { + this.plugin = plugin; + this.bibleContentService = bibleContentService; + } + public static getFileForPassage(passage: BibleReference, plugin: DisciplesJournalPlugin): TFile | null { const passageNotePath = BibleFiles.pathForPassage(passage, plugin); const file = plugin.app.vault.getAbstractFileByPath(passageNotePath); @@ -21,9 +35,6 @@ export class BibleFiles { return plugin.app.vault.getAbstractFileByPath(BibleFiles.pathForPassage(passage, plugin)) instanceof TFile; } - // TODO - openChapterNote function to open a note pointing to a chapter passage - // TODO - openPassageNote function to open a note pointing to a non-chapter passage - public static pathForPassage(passage: BibleReference, plugin: DisciplesJournalPlugin): string { const base = `${BibleFiles.getFullContentPath(plugin)}/${passage.book}`; if (passage.isChapterReference()) { @@ -42,7 +53,6 @@ export class BibleFiles { return `${plugin.settings.bibleContentVaultPath}/${plugin.settings.preferredBibleVersion}`; } - public static async clearData(plugin: DisciplesJournalPlugin) { console.debug(`Clearing bible data from ${plugin.settings.bibleContentVaultPath}`); const folderPath = normalizePath(plugin.settings.bibleContentVaultPath); @@ -52,4 +62,102 @@ export class BibleFiles { await plugin.app.fileManager.trashFile(folder); } } + + /** + * Open a chapter note for the given reference, creating it from the API first + * if it isn't on disk yet. If the reference includes a verse, the note is + * scrolled to that verse once it renders. + */ + public async openChapterNote(reference: BibleReference): Promise { + try { + const chapterPath = BibleFiles.pathForPassage(reference, this.plugin); + if (!BibleFiles.fileExistsForPassage(reference, this.plugin)) { + if (this.plugin.settings.downloadOnDemand) { + const response = await this.createChapterNote(reference); + if (response.isError()) { + // Surface the underlying reason (e.g. missing ESV API token) to the user + // rather than failing silently. + new Notice(`Disciples Journal: ${response.errorMessage}`, 10000); + return; + } + } else { + console.error(`Passage download disabled (see settings): ${chapterPath}`); + new Notice("Disciples Journal: On-demand passage download is turned off. Enable it in the plugin settings to open this chapter.", 10000); + return; + } + } + + const passageNoteFile = BibleFiles.getFileForPassage(reference, this.plugin); + if (passageNoteFile instanceof TFile) { + const leaf = this.plugin.app.workspace.getLeaf(false); + await leaf.openFile(passageNoteFile); + + // If there's a specific verse, scroll to it once it has rendered + if (reference.verse) { + this.scrollToVerse(leaf, reference.verse); + } + } else { + console.error(`Could not find or create the chapter note: ${chapterPath}`); + new Notice(`Disciples Journal: Could not open the chapter note for ${reference.toString()}.`, 10000); + } + } catch (error) { + console.error('Error opening chapter note:', error); + const detail = error instanceof Error ? error.message : String(error); + new Notice(`Disciples Journal: Failed to open chapter note. ${detail}`, 10000); + } + } + + /** + * Scroll the opened note to a verse element. The note's body renders + * asynchronously after `openFile`, so rather than guessing with a fixed + * delay we watch the view for the verse element to appear and scroll the + * moment it does. A bounded fallback disconnects the observer if the verse + * never renders (e.g. wrong reference, or the view is in an edit mode that + * doesn't produce verse elements) so it can't linger. + */ + private scrollToVerse(leaf: WorkspaceLeaf, verse: number): void { + const container = leaf.getContainer(); + const selector = `.verse-${verse}`; + + const tryScroll = (): boolean => { + const verseEl = container.doc.querySelector(selector); + if (verseEl) { + verseEl.scrollIntoView({behavior: 'smooth', block: 'center'}); + return true; + } + return false; + }; + + // Already rendered (e.g. note was cached)? Scroll right away. + if (tryScroll()) { + return; + } + + const win = container.win; + const observer = new MutationObserver(() => { + if (tryScroll()) { + observer.disconnect(); + win.clearTimeout(fallbackId); + } + }); + observer.observe(leaf.view.containerEl, {childList: true, subtree: true}); + + const fallbackId = win.setTimeout(() => observer.disconnect(), 5000); + } + + /** + * Create a new chapter note. Returns the API response so callers can surface + * the reason for any failure (e.g. a missing ESV API token) to the user. + */ + private async createChapterNote(reference: BibleReference): Promise { + // Create reference only from book+chapter + const bookChapter = new BibleReference(reference.book, reference.chapter); + + // Get the content from the Bible API and format it for a note + const response: BibleApiResponse = await this.bibleContentService.getBibleContent(bookChapter); + if (response.isError()) { + console.error(`Failed to get Bible content for ${bookChapter.toString()}: ${response.errorMessage}`); + } + return response; + } } diff --git a/src/services/ESVApiService.ts b/src/services/ESVApiService.ts index e70fd20..50150e7 100644 --- a/src/services/ESVApiService.ts +++ b/src/services/ESVApiService.ts @@ -42,10 +42,28 @@ export class ESVApiService { this.plugin = plugin; } + /** + * Convert a raw ESV API response into a BibleApiResponse. Handles both freshly + * downloaded responses and ones re-read from a saved note's frontmatter (hence + * the loosely-typed input). `contextRef` is only used for error messages. + */ + public static toBibleApiResponse(data: unknown, contextRef: BibleReference): BibleApiResponse { + const response = data as { canonical?: unknown; passages?: unknown } | null; + const canonical = response && typeof response.canonical === 'string' ? response.canonical : null; + const canonicalRef = canonical ? BibleReference.parse(canonical) : null; + if (!canonicalRef) { + const message = `Failed to parse canonical reference (${String(canonical)}) from ESV API for ${contextRef.toString()}`; + console.error(message); + return BibleApiResponse.error(message, ErrorType.BadApiResponse); + } + const passages = response && Array.isArray(response.passages) ? response.passages : []; + const html = typeof passages[0] === 'string' ? passages[0] : ''; + return BibleApiResponse.success(new BiblePassage(canonicalRef, html)); + } + /** * Download Bible content from the ESV API */ - // TODO - this should probably take in a BibleReference instead of a string public async downloadFromESVApi(ref: BibleReference): Promise { if (!this.plugin.settings.esvApiToken) { console.error('ESV API token not set. Cannot download content.'); @@ -80,13 +98,7 @@ export class ESVApiService { await this.saveESVApiResponseAsMdNote(data); // Return the content - const canonicalRef = BibleReference.parse(data.canonical); - if (!canonicalRef) { - const message = `Failed to parse canonical reference (${data.canonical}) from ESV API for ${ref.toString()}`; - console.error(message); - return BibleApiResponse.error(message, ErrorType.BadApiResponse); - } - return BibleApiResponse.success(new BiblePassage(canonicalRef, data.passages[0])); + return ESVApiService.toBibleApiResponse(data, ref); } else { console.error(`ESV API request failed with status ${response.status}: ${response.text}`); return BibleApiResponse.error(