Skip to content

Honor system dark mode in ComparePlus diff colors - #111

Merged
hybridmachine merged 2 commits into
macos-portfrom
feature/compare-plus-dark-mode
May 24, 2026
Merged

Honor system dark mode in ComparePlus diff colors#111
hybridmachine merged 2 commits into
macos-portfrom
feature/compare-plus-dark-mode

Conversation

@hybridmachine

Copy link
Copy Markdown
Owner

Summary

ComparePlus diff highlights stayed in their bright light-mode palette even when macOS was set to dark mode, washing out the underlying text. Three plugin-host messages were hardcoded to "light", and NPPN_DARKMODECHANGED was never emitted, so the plugin couldn't switch palettes.

  • nppm_handler.mmNPPM_ISDARKMODEENABLED, NPPM_GETEDITORDEFAULTBACKGROUNDCOLOR, NPPM_GETEDITORDEFAULTFOREGROUNDCOLOR now report real state. The bg/fg values (0x1E1E1E / 0xD4D4D4) match what Scintilla actually paints in appearance.mm, so plugins that derive marker shades from these (ComparePlus's "blank" marker, for instance) draw against the right base.
  • app_delegate.mm — emit NPPN_DARKMODECHANGED from appearanceChanged: after the host has re-applied its own appearance and re-colorised the editors. Safe to fire before any plugin loads (notify is a no-op on an empty plugin list).
  • appearance.{h,mm} — extracted isAppDarkMode() from the inline check in applyAppearance(); reused in nppm_handler.mm.
  • docs/compare-plus-host-inventory.mdNPPN_DARKMODECHANGED marked emitted. NPPN_WORDSTYLESUPDATED still not fired (no host style-configurator UI to drive it; ComparePlus's handler treats it identically to NPPN_DARKMODECHANGED, so dark-mode toggles already refresh its colors).

Test plan

  • Light mode: open two files with diffs, Set as First, Compare. Diff highlights match the existing light palette (yellows/greens/oranges).
  • Dark mode: same scenario. Diff highlights use the muted dark palette and the diff text stays readable.
  • With a compare active, toggle System Settings → Appearance between Light and Dark. ComparePlus colors should swap immediately without re-running compare.
  • Toggle appearance with no plugins loaded / before plugins finish loading — no crash, no log noise.
  • Other in-tree plugins (HelloPaperWasp) still load and behave the same.

🤖 Generated with Claude Code

Three plugin-host messages were hardcoded to light mode
(NPPM_ISDARKMODEENABLED, NPPM_GETEDITORDEFAULTBACKGROUNDCOLOR,
NPPM_GETEDITORDEFAULTFOREGROUNDCOLOR) and NPPN_DARKMODECHANGED was
never emitted. ComparePlus consumes these to choose its diff-color
palette, so its bright pastel backgrounds stayed put under dark
mode and washed out the underlying text.

- appearance.{h,mm}: extract isAppDarkMode() from applyAppearance().
- nppm_handler.mm: report real dark-mode state and matching default
  bg/fg colors (0x1E1E1E / 0xD4D4D4 — same values the editor paints).
- app_delegate.mm: emit NPPN_DARKMODECHANGED from appearanceChanged:
  so plugins refresh their palettes when the system theme flips.
- docs/compare-plus-host-inventory.md: mark NPPN_DARKMODECHANGED as
  emitted; note NPPN_WORDSTYLESUPDATED is still unfired (no host
  style-configurator UI to drive it, and ComparePlus handles it
  identically to NPPN_DARKMODECHANGED).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 24, 2026 01:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the macOS host’s plugin-facing API surface so ComparePlus (and other plugins) can correctly react to system dark mode by (1) reporting real dark-mode state/default editor colors and (2) emitting the expected dark-mode-change notification after the host reapplies appearance.

Changes:

  • Report correct values for NPPM_ISDARKMODEENABLED, NPPM_GETEDITORDEFAULTBACKGROUNDCOLOR, and NPPM_GETEDITORDEFAULTFOREGROUNDCOLOR based on system appearance.
  • Emit NPPN_DARKMODECHANGED from appearanceChanged: after reapplying appearance and recolorising editors.
  • Extract shared isAppDarkMode() helper and update ComparePlus host-surface inventory docs.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
macos/platform/nppm_handler.mm Return dark-aware editor default bg/fg and dark-mode enabled state to plugins.
macos/platform/appearance.mm Add isAppDarkMode() helper and reuse it in applyAppearance().
macos/platform/appearance.h Expose isAppDarkMode() to other macOS host components.
macos/platform/app_delegate.mm Notify plugins via NPPN_DARKMODECHANGED after appearance updates.
docs/compare-plus-host-inventory.md Mark NPPN_DARKMODECHANGED as emitted and update the summary list.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread macos/platform/nppm_handler.mm Outdated
Comment on lines +238 to +241
// Returned as 0x00BBGGRR (Win32 COLORREF). Must match the
// Scintilla default-style background actually painted (see
// appearance.mm:33), or plugins derive marker shades against
// the wrong base — ComparePlus's "blank" marker in particular.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Good catch — fixed in bcec1c8. Replaced the appearance.mm:33 reference with a symbol-based one (SCI_STYLESETBACK for style 32 inside applyAppearanceToView) so it can't rot again.

The comment in NPPM_GETEDITORDEFAULTBACKGROUNDCOLOR pointed at
appearance.mm:33, but adding the isAppDarkMode() helper above shifted
the actual call. Point at SCI_STYLESETBACK for style 32 inside
applyAppearanceToView instead so the reference doesn't rot again.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@hybridmachine
hybridmachine merged commit 35dea2e into macos-port May 24, 2026
17 of 32 checks passed
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