fix: file bookmarks should respect new tab preferences #182

Bookmark suggestions are both rendered and executed by the core switcher. This change makes the BookmarksHandler responsible for execution of only file-based bookmarks so that the "open in new tab" preferences can be applied when opening file-based bookmarks.

src/Handlers/__tests__/bookmarksHandler.test.ts

@@ -2,7 +2,13 @@ import { SwitcherPlusSettings } from 'src/settings';
 import { mock, MockProxy, mockReset } from 'jest-mock-extended';
 import { BookmarksHandler, BOOKMARKS_PLUGIN_ID } from 'src/Handlers';
 import { InputInfo } from 'src/switcherPlus';
-import { Mode, SearchQuery, SuggestionType } from 'src/types';
+import {
+  BookmarksItemInfo,
+  MatchType,
+  Mode,
+  SearchQuery,
+  SuggestionType,
+} from 'src/types';
 import { Searcher } from 'src/search';
 import {
   makeFuzzyMatch,
@@ -12,6 +18,8 @@ import {
   makeBookmarksPluginGroupItem,
   makeBookmarksPluginSearchItem,
   makeHeading,
+  makeInputInfo,
+  makeBookmarkedFileSuggestion,
 } from '@fixtures';
 import {
   App,
@@ -115,8 +123,19 @@ describe('bookmarksHandler', () => {
     expect(sut.renderSuggestion(null, null)).toBe(false);
   });
 
-  test('onChooseSuggestion() should return false', () => {
-    expect(sut.onChooseSuggestion(null, null)).toBe(false);
+  test('onChooseSuggestion() should open the associated file for FileBookmarks', () => {
+    const sugg = makeBookmarkedFileSuggestion();
+    const mockEvt = mock<KeyboardEvent>();
+
+    const navigateSpy = jest
+      .spyOn(BookmarksHandler.prototype, 'navigateToLeafOrOpenFile')
+      .mockReturnValueOnce();
+
+    sut.onChooseSuggestion(sugg, mockEvt);
+
+    expect(navigateSpy).toHaveBeenCalledWith(mockEvt, sugg.file, expect.any(String));
+
+    navigateSpy.mockRestore();
   });
 
   describe('getCommandString', () => {
@@ -371,4 +390,42 @@ describe('bookmarksHandler', () => {
       getFirstH1Spy.mockRestore();
     });
   });
+
+  describe('addPropertiesToStandardSuggestions', () => {
+    const mockFile = new TFile();
+    const inputInfo = makeInputInfo();
+    inputInfo.currentWorkspaceEnvList.openWorkspaceFiles = new Set([mockFile]);
+    inputInfo.currentWorkspaceEnvList.mostRecentFiles = new Set([mockFile]);
+    inputInfo.currentWorkspaceEnvList.fileBookmarks = new Map<TFile, BookmarksItemInfo[]>(
+      [[mockFile, []]],
+    );
+
+    it('should set extra properties on Bookmark suggestions', () => {
+      const sugg = makeBookmarkedFileSuggestion({
+        file: mockFile,
+        bookmarkPath: mockFile.path,
+        match: makeFuzzyMatch(),
+      });
+
+      // Note: purposefully unset the file property, because the core switcher does not provide a value for this property, unlike Files/Alias suggestions
+      sugg.file = null;
+
+      const getTFileByPathSpy = jest
+        .spyOn(BookmarksHandler.prototype, 'getTFileByPath')
+        .mockReturnValueOnce(mockFile);
+
+      sut.addPropertiesToStandardSuggestions(inputInfo.currentWorkspaceEnvList, sugg);
+
+      expect(sugg).toMatchObject({
+        ...sugg,
+        matchType: MatchType.Primary,
+        matchText: mockFile.path,
+        isOpenInEditor: true,
+        isRecent: true,
+        isBookmarked: true,
+      });
+
+      getTFileByPathSpy.mockRestore();
+    });
+  });
 });

src/Handlers/__tests__/standardExHandler.test.ts

@@ -128,7 +128,7 @@ describe('standardExHandler', () => {
     it('should set extra properties on alias suggestions', () => {
       const sugg = makeAliasSuggestion(mockFile);
 
-      sut.addPropertiesToStandardSuggestions(inputInfo, sugg);
+      sut.addPropertiesToStandardSuggestions(inputInfo.currentWorkspaceEnvList, sugg);
 
       expect(sugg).toMatchObject({
         ...sugg,
@@ -143,7 +143,7 @@ describe('standardExHandler', () => {
     it('should set extra properties on file suggestions', () => {
       const sugg = makeFileSuggestion(mockFile);
 
-      sut.addPropertiesToStandardSuggestions(inputInfo, sugg);
+      sut.addPropertiesToStandardSuggestions(inputInfo.currentWorkspaceEnvList, sugg);
 
       expect(sugg).toMatchObject({
         ...sugg,

src/Handlers/bookmarksHandler.ts

@@ -89,10 +89,23 @@ export class BookmarksHandler extends Handler<BookmarksSuggestion> {
   }
 
   onChooseSuggestion(
-    _sugg: BookmarksSuggestion,
-    _evt: MouseEvent | KeyboardEvent,
+    sugg: BookmarksSuggestion,
+    evt: MouseEvent | KeyboardEvent,
   ): boolean {
-    return false;
+    let handled = false;
+    if (BookmarksHandler.isBookmarksPluginFileItem(sugg?.item)) {
+      const { file } = sugg;
+
+      this.navigateToLeafOrOpenFile(
+        evt,
+        file,
+        `Unable to open file from BookmarkSuggestion ${file?.path}`,
+      );
+
+      handled = true;
+    }
+
+    return handled;
   }
 
   getPreferredTitle(
@@ -231,4 +244,38 @@ export class BookmarksHandler extends Handler<BookmarksSuggestion> {
   static isBookmarksPluginGroupItem(obj: unknown): obj is BookmarksPluginGroupItem {
     return isOfType<BookmarksPluginFileItem>(obj, 'type', 'group');
   }
+
+  /**
+   * Injects suggestions generated by the core switcher in Standard mode with
+   * additional properties to enable custom functionality.
+   *
+   * @param {WorkspaceEnvList} workspaceEnvList
+   * @param {BookmarksSuggestion} sugg
+   */
+  addPropertiesToStandardSuggestions(
+    workspaceEnvList: WorkspaceEnvList,
+    sugg: BookmarksSuggestion,
+  ): void {
+    const { match, item } = sugg;
+    let matchType = MatchType.None;
+    let matchText = null;
+
+    if (!sugg.file && BookmarksHandler.isBookmarksPluginFileItem(item)) {
+      // Materialize file property for file bookmarks because the core switcher does
+      // not provide the file for Bookmark suggestions (unlike Alias/File suggestions)
+      const filePath = item.path;
+      sugg.file = this.getTFileByPath(filePath);
+    }
+
+    if (match?.matches) {
+      matchType = MatchType.Primary;
+      matchText = sugg.bookmarkPath;
+    }
+
+    sugg.matchType = matchType;
+    sugg.matchText = matchText;
+
+    // patch with missing properties required for enhanced custom rendering
+    Handler.updateWorkspaceEnvListStatus(workspaceEnvList, sugg);
+  }
 }

src/Handlers/standardExHandler.ts

@@ -1,6 +1,6 @@
 import { Handler } from './handler';
 import { MetadataCache, WorkspaceLeaf } from 'obsidian';
-import { InputInfo, ParsedCommand } from 'src/switcherPlus';
+import { InputInfo, ParsedCommand, WorkspaceEnvList } from 'src/switcherPlus';
 import { isAliasSuggestion, isFileSuggestion } from 'src/utils';
 import { SwitcherPlusSettings } from 'src/settings';
 import {
@@ -39,7 +39,7 @@ export class StandardExHandler extends Handler<SupportedSystemSuggestions> {
     let handled = false;
     if (isFileSuggestion(sugg)) {
       handled = this.renderFileSuggestion(sugg, parentEl);
-    } else {
+    } else if (isAliasSuggestion(sugg)) {
       handled = this.renderAliasSuggestion(sugg, parentEl);
     }
 
@@ -114,30 +114,36 @@ export class StandardExHandler extends Handler<SupportedSystemSuggestions> {
     return handled;
   }
 
+  /**
+   * Injects suggestions generated by the core switcher in Standard mode with
+   * additional properties to enable custom functionality.
+   *
+   * @param {WorkspaceEnvList} workspaceEnvList
+   * @param {SupportedSystemSuggestions} sugg
+   */
   addPropertiesToStandardSuggestions(
-    inputInfo: InputInfo,
+    workspaceEnvList: WorkspaceEnvList,
     sugg: SupportedSystemSuggestions,
   ): void {
-    const { match, file } = sugg;
-    const matches = match?.matches;
+    const { match } = sugg;
     let matchType = MatchType.None;
     let matchText = null;
 
-    if (matches) {
+    if (match?.matches) {
       if (isAliasSuggestion(sugg)) {
         matchType = MatchType.Primary;
         matchText = sugg.alias;
-      } else {
+      } else if (isFileSuggestion(sugg)) {
         matchType = MatchType.Path;
-        matchText = file?.path;
+        matchText = sugg.file.path;
       }
     }
 
     sugg.matchType = matchType;
     sugg.matchText = matchText;
 
     // patch with missing properties required for enhanced custom rendering
-    Handler.updateWorkspaceEnvListStatus(inputInfo.currentWorkspaceEnvList, sugg);
+    Handler.updateWorkspaceEnvListStatus(workspaceEnvList, sugg);
   }
 
   static createUnresolvedSuggestion(

src/switcherPlus/__tests__/modeHandler.test.ts

@@ -856,33 +856,6 @@ describe('modeHandler', () => {
       createNoteSpy.mockRestore();
     });
 
-    test('with overrideStandardModeBehaviors enabled, renderSuggestion should use the StandardExHandler', () => {
-      mockSettings.overrideStandardModeBehaviors = true;
-      const sugg = makeFileSuggestion();
-      const mockParentEl = mock<HTMLElement>();
-
-      const addPropsSpy = jest
-        .spyOn(StandardExHandler.prototype, 'addPropertiesToStandardSuggestions')
-        .mockImplementation();
-
-      const renderSuggestionSpy = jest
-        .spyOn(StandardExHandler.prototype, 'renderSuggestion')
-        .mockReturnValue(true);
-
-      sut.updateSuggestions(chance.word(), mockChooser, null);
-
-      const handled = sut.renderSuggestion(sugg, mockParentEl);
-
-      expect(renderSuggestionSpy).toHaveBeenCalledWith(sugg, mockParentEl);
-      expect(addPropsSpy).toHaveBeenCalled();
-      expect(handled).toBe(true);
-
-      renderSuggestionSpy.mockRestore();
-      addPropsSpy.mockRestore();
-      mockClear(mockChooser);
-      mockSettings.overrideStandardModeBehaviors = false;
-    });
-
     test.each([makeFileSuggestion()])(
       'renderSuggestion should use the StandardExHandler when in Headings mode for File Suggestions',
       (sugg) => {
@@ -1347,4 +1320,42 @@ describe('modeHandler', () => {
       expect(result).toBe(true);
     });
   });
+
+  describe('addPropertiesToStandardSuggestions', () => {
+    let sut: ModeHandler;
+
+    beforeAll(() => {
+      sut = new ModeHandler(mockApp, mockSettings, mock<SwitcherPlusKeymap>());
+    });
+
+    test('with overrideStandardModeBehaviors enabled, it should add properties to a Standard mode file suggestion', () => {
+      mockSettings.overrideStandardModeBehaviors = true;
+      const sugg = makeFileSuggestion();
+      const addPropsSpy = jest
+        .spyOn(StandardExHandler.prototype, 'addPropertiesToStandardSuggestions')
+        .mockImplementation();
+
+      sut.addPropertiesToStandardSuggestions([sugg], mockSettings);
+
+      expect(addPropsSpy).toHaveBeenCalled();
+
+      addPropsSpy.mockRestore();
+      mockSettings.overrideStandardModeBehaviors = false;
+    });
+
+    test('with overrideStandardModeBehaviors enabled, it should add properties to a Standard mode bookmark suggestion', () => {
+      mockSettings.overrideStandardModeBehaviors = true;
+      const sugg = makeBookmarkedFileSuggestion();
+      const addPropsSpy = jest
+        .spyOn(BookmarksHandler.prototype, 'addPropertiesToStandardSuggestions')
+        .mockImplementation();
+
+      sut.addPropertiesToStandardSuggestions([sugg], mockSettings);
+
+      expect(addPropsSpy).toHaveBeenCalled();
+
+      addPropsSpy.mockRestore();
+      mockSettings.overrideStandardModeBehaviors = false;
+    });
+  });
 });