[CI only] Integration build of all four features - #5
Draft
leoshone wants to merge 22 commits into
Draft
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.
"Refresh JSON Tree" rebuilds every node, so the tree always came back fully collapsed - even when the user only wanted to re-read a document they were already looking at. Capture which nodes are expanded and which one is selected before the tree is thrown away, then re-apply that state onto the freshly built tree, matching nodes by path. Paths that no longer exist (the document changed in the meantime) are silently dropped, and nodes that are new stay collapsed. The state is keyed by node path, which is the list of keys from the tree root down to a node. The pure path arithmetic lives in the new TreeExpansion.h/.cpp so it can be unit tested without a window. DrawJsonTree() gained a bPreserveExpansion parameter that defaults to false, so every other caller (panel opening, formatting, compressing, sorting) keeps behaving exactly as before; only the refresh button opts in.
When "Follow current tab" is off (the default) the plugin never drew the
tree on its own, but it also never cleared it: the tree kept showing the
document of some earlier tab, with no indication that it belonged there.
With this change the tree is only ever drawn when the user asks for it
("Refresh JSON Tree"). Switching tabs stores the tree of the tab being
left and puts it back verbatim when the tab is activated again, so no
re-parsing happens and the expansion state and selection survive.
The "Follow current tab" option is kept and behaves exactly as before
when enabled: the document of the activated tab is parsed immediately.
Only its "off" path changes, from "do nothing" to "remember per tab".
Notes:
- Snapshots live in memory only and are dropped when the buffer is
closed, together with the association to the current buffer.
- "Auto format on open" now formats the document without drawing the
tree, so opening a file still cannot trigger a parse.
- Formatting now redraws the tree while preserving its expansion state,
which keeps it consistent with Refresh.
- Built on top of the TreeExpansion helpers introduced for Refresh.
Adds an option, off by default, that draws the tree of a json document as soon as the file is opened. It complements the per-tab snapshot caching: the document is parsed exactly once, and switching back to the tab afterwards replays the stored snapshot instead of parsing again. The check lives in RestoreTabState(), the single place reached when the tree of a tab has never been drawn, so opening a file, switching back to a tab and showing the panel are all covered by one code path. Drawing on open is initiated by the plugin, not by the user, so parse errors are reported as a node inside the tree rather than through a modal dialog: DrawJsonTree() takes a bSilent flag for that. The tree is drawn for documents whose language is JSON, the same criterion the existing "follow current tab" uses.
Local integration branch: combines PR NPP-JSONViewer#251 (tree zoom persistence) with PR NPP-JSONViewer#253 (per-tab tree snapshot + draw on open). Not sent upstream as a PR - it exists only to build a DLL carrying all four features for local use. The three conflicts were all of the same shape: both sides add one line to the same spot (TREE_ZOOM vs DRAW_ON_OPEN in Define.h, Profile.cpp and ProfileTest.cpp). Both sides are kept. Two pre-existing upstream quirks noticed while merging, deliberately left untouched so this branch stays an exact union of the three PRs: - ProfileTest.cpp assigns expected.bAutoFormat twice (present in c448336); - SetSettings_Positive asserts on nTreeZoom without ever assigning it, so the assertion compares the default with itself. Harmless because TreeZoom_RoundTrip does exercise the real round trip.
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.
Documents the four custom features (zoom persistence, per-tab tree snapshot, expansion retention on refresh, draw-tree-on-open), the branch topology, the upstream sync procedure with per-file conflict strategies, the post-sync verification checklist, and known limitations. Written for the scenario where upstream does not merge the proposed PRs and this fork keeps syncing FROM upstream instead.
Notepad++ maps the .jsonc extension to the json5 language, so IsJsonFile() returned false for jsonc documents and the plugin skipped them entirely: no draw-on-open, no follow-tab, no auto-format. Accepting L_JSON5 alongside L_JSON is enough - the parser already handles jsonc content (ignoring comments and trailing commas is configurable and on by default), and it was verified end to end that a jsonc document parsed through the json code path works. Full JSON5 syntax beyond comments and trailing commas (unquoted keys, single-quoted strings) is not supported by the parser and keeps failing with a parse error.
AdjustDocPanelSize multiplied the pixel delta between the new panel size and the initial one by the desktop DPI scale. Both values are already in physical pixels, so on any monitor whose scale is not 100% the tree (and the node path box) grew s times faster than the panel itself and slid below the panel's client area. The tree control computed its scroll range from its own oversized height, so the scrollbar reported the end while the last (s-1)*growth/itemHeight rows were physically outside the visible panel: fully expanded long documents showed rows that could never be scrolled into view. Measurements at 150% scaling before the fix: tree bottom 213 px below the panel client area (5+ unreachable rows at 100% zoom), node path box 115 px below it. Positioning is now absolute - template rect plus the unscaled delta, with the tree ending above a node path box that is pinned to the bottom of the client area - which also makes repeated resizes idempotent instead of accumulated.
MSVC's windows.h defines max as a macro unless NOMINMAX is set, so std::max(...) failed to compile as std::(...). Parenthesize the call - the standard portable workaround - instead of touching the project's include settings.
MSVC's windows.h defines max as a macro unless NOMINMAX is set, so std::max(...) failed to compile as std::(...). Parenthesize the call - the standard portable workaround - instead of touching the project's include settings.
The tree height is now derived from the node path box position instead of the raw height delta, so the variable is gone.
The tree height is now derived from the node path box position instead of the raw height delta, so the variable is gone.
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.
IsJsonFile() used to accept every L_JSON5 document. Notepad++ maps the
.jsonc extension to the json5 language ("json5 jsonc" share one entry in
the default langs.xml), but that also pulled in genuine .json5 documents,
whose syntax (unquoted keys, single-quoted strings) the parser does not
support: they used to be silently ignored by the plugin and would now be
parsed and reported as an error.
Accept the json5 language for .jsonc files only, so a real .json5 file
keeps behaving exactly as before this change.
Verified end to end: .jsonc and .json5 are both reported as the same
language (86) by Notepad++, the .jsonc document is drawn while the .json5
one is not, and the existing jsonc assertions still pass.
FORK-MAINTENANCE.md is about merging upstream code into this fork; this new document is the other direction - every PR we pushed upstream, the review comments verbatim, why we decided what we decided, and what is still pending (#255 for the narrow jsonc recognition). The PR status table moves here so the two documents cannot drift apart. Also records the environment facts that bit us: upstream runs no CI for fork PRs, a fork account cannot re-request a review, and the language of a buffer is rewritten to L_JSON by the plugin once it draws a tree.
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
1 participant
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.
Draft PR opened only to trigger the CI workflow, which reacts to pull_request events. It is not meant to be merged anywhere: the three features it combines are already proposed upstream separately as NPP-JSONViewer#251 and NPP-JSONViewer#253.
This branch is the exact union of those two PRs, built so a single DLL carrying all four features can be produced locally.