Persist the tree view zoom level across Notepad++ sessions - #251
Open
leoshone wants to merge 2 commits into
Open
Persist the tree view zoom level across Notepad++ sessions#251leoshone wants to merge 2 commits into
leoshone wants to merge 2 commits into
Conversation
The plugin already ships a zoom slider for the JSON tree (80%..250%), but the chosen level only lived in the slider control: closing Notepad++ and starting it again always fell back to 100%. Store the zoom percentage in JSONViewer.ini under [Others] TREE_ZOOM and re-apply it when the dialog is initialised. The value is written only when it actually changes, and while the slider thumb is being dragged (TB_THUMBTRACK) nothing is written, so a drag gesture produces a single write at the end instead of one per pixel. Purely additive: the existing ini keys and the default behaviour are untouched.
This was referenced Sep 3, 2026
WM_HSCROLL carries the notification code in LOWORD(wParam), not HIWORD: HIWORD holds the thumb position itself (80..250 here), so comparing it against TB_THUMBTRACK never matched and the zoom was written to the ini file continuously while the thumb was being dragged, instead of once when the gesture ended. Found by an independent review of the integration branch; the end-to-end harness never caught it because it only sends TB_ENDTRACK and never simulates the dragging itself.
SinghRajenM
requested changes
Sep 5, 2026
| unsigned languageType = 0; | ||
| ::SendMessage(m_NppData._nppHandle, NPPM_GETCURRENTLANGTYPE, 0, reinterpret_cast<LPARAM>(&languageType)); | ||
| return languageType == LangType::L_JSON; | ||
| return languageType == LangType::L_JSON || languageType == LangType::L_JSON5; |
Collaborator
There was a problem hiding this comment.
Let's remove JSON5 as it is not supported currently by the plugin.
Author
There was a problem hiding this comment.
Agreed - full JSON5 syntax (unquoted keys, single-quoted strings) is not supported by the parser, and the change was out of scope for this PR anyway.
Removed: IsJsonFile() is back to L_JSON only, and the extra commit is gone from this branch (and from #253). jsonc recognition stays in my fork.
| ParseOptions parseOptions {}; | ||
| int nTreeZoom = 100; // Tree view font zoom in percent (80..250) | ||
|
|
||
| std::wstring configPath; // Full path of JSONViewer.ini (not persisted) |
Collaborator
There was a problem hiding this comment.
What is the purpose and where it is used?
Author
There was a problem hiding this comment.
configPath is the full path of JSONViewer.ini.
The tree dialog needs it because the zoom level has to be written back to the ini when the user moves the zoom slider (JsonViewDlg.cpp -> ProfileSetting(m_pSetting->configPath).SetSettings(...)), and the dialog had no access to that path before - upstream only hands it to SettingsDlg through that dialog's own m_configPath (NppJsonPlugin.cpp). It is assigned once in NppJsonPlugin.cpp and is not itself persisted to the ini.
I also rewrote the PR description, which had lost its original content - sorry about the missing context.
leoshone
force-pushed
the
fix/persist-tree-zoom
branch
from
September 5, 2026 04:15
16bad52 to
7771ca0
Compare
leoshone
added a commit
to leoshone/JSON-Viewer
that referenced
this pull request
Sep 5, 2026
The maintainer asked to drop the L_JSON5 acceptance in NPP-JSONViewer#251 because the parser does not support real JSON5 syntax. The commit is removed from both PR branches (reset + force push) and the PR descriptions are rewritten; jsonc recognition stays in the integration branch, where it came from an independent merge and is unaffected by the reset. Also records that gh pr edit --body-file replaces the whole body (that is how NPP-JSONViewer#251 lost its description in the first place) and that a fork account cannot re-request a review upstream.
leoshone
added a commit
to leoshone/JSON-Viewer
that referenced
this pull request
Sep 5, 2026
Records that the plan is settled (push the narrow jsonc version as a new PR once NPP-JSONViewer#251 settles, leave the existing thread untouched), the reason the thread is not being edited now (comment edits send no notification, so the change would likely never be seen), and the exact comment id plus command for the forward-pointer to add afterwards. Also adds two environment facts that cost us time today: our own review comments can be edited but edits notify nobody, and the most recent CI run right after a push can still be the previous commit's run.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Persist the tree view zoom level across Notepad++ sessions
The plugin already ships a zoom slider for the JSON tree (80%..250%), but the
chosen level only lived in the slider control: closing Notepad++ and starting
it again always fell back to 100%.
This stores the zoom percentage in
JSONViewer.iniunder[Others] TREE_ZOOMand re-applies it when the dialog is initialised.
Details worth reviewing:
Define.h,Profile.cpp,ProfileTest.cpp,JsonViewDlg.cppand
JsonViewDlg.h. Purely additive: existing ini keys and the defaultbehaviour are untouched, and an ini without
TREE_ZOOMstill means 100%.nothing is written while the slider thumb is being dragged
(
TB_THUMBTRACK), so a drag gesture produces a single write at the endinstead of one per pixel.
Setting::configPath. The tree dialog needs the ini path to write thevalue back (
JsonViewDlg.cpp→ProfileSetting(m_pSetting->configPath).SetSettings(...)); until now thepath was only handed to
SettingsDlgthrough its ownm_configPath. The newfield is assigned once in
NppJsonPlugin.cppand is not persisted to the iniitself.
Second commit: fix the drag detection of the zoom slider
WM_HSCROLLcarries the notification code inLOWORD(wParam), notHIWORD—HIWORDholds the thumb position itself (80..250 here), so comparing itagainst
TB_THUMBTRACKnever matched and the zoom was written to the inicontinuously during a drag instead of once when the gesture ended.
Found by an independent review of the integration branch; the end-to-end
harness never caught it because it only ever sent
TB_ENDTRACKand neversimulated the dragging itself.
Verification
ProfileTestcovers theTREE_ZOOMread/write round trip andthe out-of-range clamping (80..250).
SendMessageto drive it): the zoom survives a tree rebuild, a refresh, atab switch and a Notepad++ restart. The assertion reads the actual tree item
height via
TVM_GETITEMHEIGHTrather than the slider position, so animplementation that only restored the slider would fail the test.