Skip to content

Add pinned attributes to entry preview panel - #13574

Open
eric-lemesre wants to merge 1 commit into
keepassxreboot:developfrom
eric-lemesre:feature/pinned-attributes
Open

eric-lemesre wants to merge 1 commit into
keepassxreboot:developfrom
eric-lemesre:feature/pinned-attributes

Conversation

@eric-lemesre

@eric-lemesre eric-lemesre commented Aug 6, 2026 •

Copy link
Copy Markdown
Contributor

This PR adds the ability to pin additional attributes (custom strings) so they are displayed on the entry preview panel, below the password field.

  • A new Pin checkbox sits next to Protect in the entry edit Advanced tab.
  • Pinned attributes are stored per entry in the standard KDBX4 entry CustomData under the key KPXC_PINNED_ATTRIBUTES, as a compact JSON array of attribute names (JSON because attribute names may contain any character, including commas and quotes). The KDBX schema and the way custom strings are stored are unchanged.
  • On the preview panel, protected attributes are masked with a reveal toggle (same pattern as the Advanced tab); double click copies the value to the clipboard.
  • The pinned list follows attribute renames and removals in the editor. Pinned names that no longer exist (e.g. after a merge or an edit by another client) are silently ignored at display time.
  • The user guide (Additional Attributes section) is updated.

Design decisions we would like to flag explicitly for review:

  1. The preview stays read-only (labels + copy on double click), consistent with the rest of the preview panel — we deliberately did not add in-place editing.
  2. Double click copies the clear value even while masked, which mirrors the existing behaviour of the attributes table in the preview's Advanced tab.
  3. Pinning writes entry CustomData, so a KDBX3 database will be upgraded to KDBX4 on save — the same behaviour as the browser integration settings.
  4. Pinned names can become orphaned if an attribute is renamed/removed outside the entry editor; they are ignored at display time and never purged automatically.

Note: this PR is independent of #13573 (Entry::beginUpdate() CustomData fix), but that fix is required for the pinned state to be carried into and restored from entry history.

Screenshots

Capture d’écran du 2026-08-06 21-13-27 Capture d’écran du 2026-08-06 21-14-13 Capture d’écran du 2026-08-06 21-15-18

Testing strategy

  • New unit test TestEntry::testPinnedAttributes: round-trip including names with commas/quotes/newlines/unicode, empty-name filtering, duplicate removal, empty list removing the key, malformed JSON tolerated, null-pointer safety.
  • Extended TestGui::testEditEntry: pin checkbox, persistence, preview panel display, masking and reveal toggle, rename sync, removal sync and section hiding.
  • Locally ran testentry, testmodified, testgroup, testkdbx2/3/4, testmerge, testcli, testbrowser and the full testgui suite — all passing. clang-format clean; translation strings pushed via lupdate.

Type of change

  • ✅ New feature (change that adds functionality)

AI usage disclosure

Per the contribution guidelines: development was done with Claude Code (model: Claude Fable 5, Anthropic); an additional critical code review pass was performed with Kimi-k3. All changes were reviewed, built and tested locally by the submitter.


We would very much welcome your feedback: if there are changes you would like, or points we may have missed — naming, UX of the Pin checkbox, the design decisions above, or anything else — we will gladly rework them.

🤖 Generated with Claude Code

@eric-lemesre
eric-lemesre marked this pull request as ready for review August 6, 2026 19:21
@droidmonkey

Copy link
Copy Markdown
Member

Cool, this is a good solution to the request

@varjolintu varjolintu added the pr: ai-assisted Pull request contains significant contributions by generative AI label Aug 7, 2026
Allow pinning additional attributes (custom strings) so they show on
the entry preview panel below the password field. Pinned attributes are
stored per entry in CustomData under the KPXC_PINNED_ATTRIBUTES key as
a compact JSON array, leaving the KDBX schema untouched.

* Add a "Pin" checkbox next to "Protect" in the entry edit Advanced tab
* Show pinned attributes on the preview General tab; protected values
  are masked with a reveal toggle, double click copies the value
* Keep the pinned list in sync on attribute rename and removal;
  pinned names that no longer exist are silently ignored
* Add TestEntry::testPinnedAttributes, extend TestGui::testEditEntry
  (pin, preview display, masking, rename and removal sync)
* Document the feature in the user guide

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@eric-lemesre
eric-lemesre force-pushed the feature/pinned-attributes branch from e37b74d to 71afee1 Compare September 8, 2026 21:21
@eric-lemesre

eric-lemesre commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor Author

Rebased onto current develop (4243717a) and force-pushed — new head 71afee18. The commit content is unchanged: git range-diff reports only context shifts, because #13573 landed testHistoryItemCustomData right above the new test in tests/TestEntry.h.

Two things that were not visible from the PR page:

  • The prerequisite is gone. The body flagged Fix history items losing entry CustomData #13573 as needed for the pinned state to survive entry-history round-trips. It merged on Aug 9, so that caveat no longer applies — and testHistoryItemCustomData now runs alongside testPinnedAttributes in the same suite.
  • The red CI was not this branch. All eight TeamCity statuses were written in the same second on Aug 8 with either "removed from queue" or "canceled by a failing dependency" — nothing actually built. The two GitHub checks that did run (Analyze / CodeQL) passed. The push should give the chain a fresh shot.

Re-verified locally on the rebased head, not carried over from August:

  • testentry: 20 passed, 0 failed — including testPinnedAttributes and testHistoryItemCustomData.
  • testgui under xvfb-run: 50 passed, 0 failed, 50.5 s — including the extended testEditEntry.
  • clang-format clean on all twelve changed .cpp / .h files.

(Edited — the paragraph that stood here reported that develop does not build with GCC 16.2. That was wrong, and sorry for the noise: the failure was entirely local. CMakeLists.txt has carried check_add_gcc_compiler_cxxflag("-Wno-error=sfinae-incomplete" ...) since the Qt6 transition (7c7ca45), and it does its job — a fresh configure on GCC 16.2 records CXX_HASSFINAE_INCOMPLETE_FLAG=1 and the flag lands in flags.make. My build tree was configured back in August under an older GCC, where that check failed and was cached as failed; CMake never re-tests it, so the guard was silently missing once I upgraded the compiler. Deleting the build directory is the whole fix. Nothing to do on your side.)

@droidmonkey you called this a good solution to the request back in August — is there a milestone it could target, or something you would like reworked first? The four design decisions listed in the body (read-only preview, double-click copying while masked, pinning writing entry CustomData so KDBX3 upgrades to KDBX4 on save, and orphaned pins being ignored rather than purged) are all still open to change.

@eric-lemesre

Copy link
Copy Markdown
Contributor Author

@droidmonkey — the two red checks here are a macOS agent timeout, not the patch.

MacOS build 9148 was killed at the 50-minute execution limit partway through compilation:

[261/612] Generating keepassxc.1, keepassxc-cli.1
line 2: 1082 Terminated: 15   cmake --build . --target all --parallel ...
Process exited with code 143 (Step: Build (Command Line))
Step 3/4: Run Tests — skipped because the build was interrupted

There are no compiler errors anywhere in the log, and the test step never ran. All Builds is red only because it snapshot-depends on MacOS — Ubuntu Linux, Windows 10, Code Format, Documentation, Test Coverage and Translations all passed, as did codecov (91.53% of diff, +0.21% project).

It looks agent-side rather than branch-specific. That run took 50 minutes to reach 261/612 targets, while recent macOS builds finish in 3–7 minutes on a warm ccache — so it was compiling cold. And 13 of the last 14 ReleaseMacOS failures are Execution timeout, across unrelated PRs (#13655, #13651, #13645, #13642, #13634, #13633, #13632, #13620, #13610, #13582), going back to mid-August. One of those hit it three times in a row on retry.

Every macOS build after mine has come back green in the usual 3–7 minutes, so a re-run would most likely go through. Could you kick one off when you get a chance? Or I can force-push to retrigger the chain instead if that's easier on your side — just say the word.

The branch content is unchanged since your note that this is a good solution to the request: rebased onto develop 4243717a, with git range-diff reporting context-only differences. Happy to take review feedback whenever it suits you.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Address accessibility labeling, passkey-key filtering, and double-click clipboard coverage before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds per-entry pinning of custom attributes, persisted in KDBX4 CustomData and shown in the read-only preview with masking, reveal, and copy behavior.

Changes:

  • Adds pin controls with rename/removal synchronization.
  • Adds preview rendering and persistence logic.
  • Adds tests, translations, and documentation.
File summaries
File Summary
tests/TestEntry.h Declares pinned-attribute tests.
tests/TestEntry.cpp Tests JSON persistence and validation.
tests/gui/TestGui.cpp Tests editor and preview integration.
src/gui/EntryPreviewWidget.ui Adds the pinned-attributes layout.
src/gui/EntryPreviewWidget.h Declares preview helpers.
src/gui/EntryPreviewWidget.cpp Renders, masks, reveals, and copies pinned values.
src/gui/entry/EditEntryWidgetAdvanced.ui Adds the Pin checkbox.
src/gui/entry/EditEntryWidget.h Declares pinning controls and handlers.
src/gui/entry/EditEntryWidget.cpp Persists pin state and synchronizes edits.
src/core/Entry.h Adds pinned-attribute APIs.
src/core/Entry.cpp Implements JSON encoding and decoding.
src/core/CustomData.h Declares the custom-data key.
src/core/CustomData.cpp Defines the custom-data key.
share/translations/keepassxc_en.ts Adds new translation strings.
docs/topics/DatabaseOperations.adoc Documents pinned attributes.
Review details
  • Files reviewed: 15/15 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +470 to +473
auto button = new QToolButton(container);
button->setCheckable(true);
button->setChecked(false);
button->setIcon(icons()->onOffIcon("password-show", false));
Comment on lines +1517 to +1518
m_advancedUi->pinAttributeButton->setChecked(Entry::pinnedAttributes(m_customData.data()).contains(key));
m_advancedUi->pinAttributeButton->setEnabled(!m_history);
Comment thread src/gui/EntryPreviewWidget.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: ai-assisted Pull request contains significant contributions by generative AI user interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants