Skip to content

Add reader-only startup and preview find#447

Open
clambertus wants to merge 1 commit into
schuyler:mainfrom
clambertus:feature/reader-mode-find
Open

Add reader-only startup and preview find#447
clambertus wants to merge 1 commit into
schuyler:mainfrom
clambertus:feature/reader-mode-find

Conversation

@clambertus

Copy link
Copy Markdown

Summary

This adds a preference to open documents in reader-only mode, with the editor pane hidden, and makes Find commands work when only the preview/reader pane is visible.

Changes

  • Add a “Start in reader-only mode” general preference.
  • Preserve the hidden editor pane when resizing the document window.
  • Route Find menu actions through the active document so reader-only windows can handle them.
  • Add preview-pane Find support, including:
    • Cmd-F / Find
    • Cmd-G / Find Next
    • Shift-Cmd-G / Find Previous
    • Use Selection for Find
    • Jump to Selection
  • Disable Replace in reader-only mode.
  • Keep editor-only formatting actions disabled when the editor pane is hidden.
  • Restore normal editor Find behavior when the editor pane is visible.
  • Quote build-number script paths so builds work from directories with spaces.
  • Add tests for the new preference and reader-only command validation behavior.

Testing

  • git diff --check
  • xcodebuild build -workspace "MacDown 3000.xcworkspace" -scheme MacDown
  • xcodebuild test -workspace "MacDown 3000.xcworkspace" -scheme MacDown -enableCodeCoverage YES -derivedDataPath DerivedData -resultBundlePath TestResults.xcresult
  • xcodebuild analyze -workspace "MacDown 3000.xcworkspace" -scheme MacDown

Test result: 875 tests, 0 failures.

Note: CocoaPods regenerated only the PODFILE CHECKSUM in Podfile.lock; dependency versions did not change.

Assisted-by: OpenAI Codex

Assisted-by: OpenAI Codex

@schuyler schuyler left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the PR, @clambertus!

Issues

  • Find result count never shows: in searchPreviewForString:forward:, self.previewFindStatusLabel.hidden = found hides the label whenever matches exist, so the computed match count is set but never displayed and only "No Results" is ever visible.
  • Headless test skips pass silently: three new tests return early when panes aren't initialized, so they report green in CI without asserting anything, where XCTSkip() would make those skips visible.

Suggestions

  • Highlights on re-render: preview find highlights and the current-match index are wiped when the preview re-renders, leaving the find bar visible but the highlights gone, so re-applying after render (or noting the limitation) would help.
  • Find pasteboard on miss: setFindPasteboardString: runs before a match is confirmed, so a failed search still overwrites the shared system find pasteboard.
  • Find bar teardown: the find bar added to window.contentView isn't explicitly removed on an abrupt close, so an explicit teardown or a note would be clearer.
  • Validation fallthrough: MPMainController.validateUserInterfaceItem: returns YES for unrecognized actions, where calling [super validateUserInterfaceItem:item] would be safer against future MPMainController-targeted items.
  • Inline search JS: the large search script is embedded as a string literal and run via the synchronous stringByEvaluatingJavaScriptFromString:, where an external .js resource (as with updateHeaderLocations.js) and the existing JSContext pattern would fit the codebase better.
  • WebView readiness guard: clearPreviewFindHighlights evaluates JS on self.preview without an isPreviewReady-style guard, which could fire after the WebView is torn down.
  • Preference migration: the loadDefaultUserDefaults seeding of editorStartInReaderMode runs every launch without a version guard, where folding it into the versioned applyPreferencesMigrations system would be more consistent.
  • First-responder timing: updateFirstResponderForVisiblePanes targets preview.mainFrame.frameView.documentView, which can be nil between frame loads during layout and drop key events.
  • Pure-function test: MPJavaScriptStringLiteral sits on the find-string-to-JS boundary and is easily unit-testable (quotes, backslashes, %, unicode, empty/nil).
  • In-app help: help.md doesn't mention the new preview/reader Find shortcuts and its preferences screenshot still shows the old "Start in preview mode" label.

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.

2 participants