feat: multi-template support via .templates files#1141
Conversation
Add functions for .templates file handling: - readTemplates() - read INI-style templates from password store - writeTemplates() - write templates to .templates file - getFolderTemplate() - find .default_template by walking up folders These enable per-folder and synced template support per issue #155.
Add method to load templates from .templates file hash and apply default template by walking up folders per issue #155.
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughAdds template-management to the password store: utilities to read/write Changes
Sequence Diagram(s)sequenceDiagram
participant UI as PasswordDialog
participant Util as Util
participant FS as File System
UI->>Util: readTemplates(storePath)
Util->>FS: open "<store>/.templates" (read, UTF-8)
FS-->>Util: file contents
Util-->>UI: QHash<templateName, fields>
UI->>UI: select template (prefer defaultTemplate or first sorted)
UI->>UI: convert fields -> newline string
UI->>UI: setTemplate(selectedName, fieldsText, true)
%% optional write flow %%
UI->>Util: writeTemplates(storePath, templates)
Util->>FS: write "<store>/.templates" (QSaveFile, atomic)
FS-->>Util: commit result
Util-->>UI: success/failure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/passworddialog.cpp`:
- Around line 249-256: The fallback selection uses templateNames =
templates.keys() (QHash order), which is non-deterministic; to make the fallback
stable, sort the key list before using templateNames.first() (e.g. call
templateNames.sort() or use a sorted container/QMap) and then validate
defaultTemplate against the sorted templateNames when setting selected; update
the code around templates.keys(), templateNames, and selected/defaultTemplate
accordingly.
In `@src/util.cpp`:
- Around line 356-363: The writeTemplates loop currently iterates a QHash via
templates.constKeyValueBegin()/constKeyValueEnd(), producing non-deterministic
ordering; change writeTemplates to collect the keys from templates (e.g., using
templates.keys()), sort them (qSort or std::sort on a
QList<QString>/std::vector<QString>), and then iterate the sorted key list to
write each key and its corresponding value list, ensuring deterministic output
when writing .templates.
- Around line 375-395: Util::getFolderTemplate currently uses
while(dir.exists(".default_template")) which prevents walking up parent
directories; change the loop to iterate until the walk reaches storePath or
cdUp() fails (e.g., while(true) with break when normalized dir.absolutePath() ==
normalized storePath or cdUp() returns false), check for the ".default_template"
file inside each iteration (open/read/trim/ignore comments) as done now, and
normalize both storePath and dir.absolutePath() using QDir::cleanPath (and
consider stripping trailing separators) before comparing to ensure the stop
condition works cross-platform.
- Around line 310-337: The readTemplates function (and sibling functions
writeTemplates and getFolderTemplate) must explicitly set QTextStream to UTF-8
for Qt5/Qt6 compatibility and should defensively handle empty section headers:
update readTemplates to set the QTextStream encoding using the project's
compatibility pattern (use in.setCodec("UTF-8") for QT_VERSION < 6 and
in.setEncoding(QStringConverter::Utf8) otherwise) and, when parsing a header
token in readTemplates (the branch that detects '['...']' and sets
currentSection), check if the extracted section name is empty and if so emit a
debug/warning (e.g., qWarning()) and skip creating that section (do not clear or
consume following field lines into a lost buffer) so fields are either ignored
until a valid section appears or handled explicitly; apply the same UTF-8 stream
setup pattern in writeTemplates and getFolderTemplate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: a98c62cf-fd7b-4806-8ab1-fa264912db5b
📒 Files selected for processing (4)
src/passworddialog.cppsrc/passworddialog.hsrc/util.cppsrc/util.h
- readTemplates: add UTF-8 codec for Qt5/Qt6, warn on empty section - writeTemplates: sort keys for deterministic output - getFolderTemplate: fix loop (while true, not while exists), use cleanPath - setAvailableTemplates: sort template names for stable fallback
Add <algorithm> and <QStringConverter> headers needed for the fixes.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/util.cpp (1)
329-343:⚠️ Potential issue | 🟡 MinorMinor: fields after an empty
[]header are silently dropped, and the inner reset is dead code.When
line.mid(1, line.length() - 2)yields an empty name, the warning fires butcurrentSectionstays empty; subsequent non-blank lines are still appended tocurrentFields(line 341) and then discarded at the next header because of the!currentSection.isEmpty()guard on line 330/344. Also, thecurrentSection.clear()on line 336 is a no-op (it is already empty), and clearingcurrentFieldsthere is immediately redone on line 339.Consider a sentinel/skip mode so fields between an invalid header and the next valid one are explicitly ignored (and reported), rather than being collected and then thrown away:
♻️ Proposed tightening
- currentSection = line.mid(1, line.length() - 2); - if (currentSection.isEmpty()) { - qWarning() << "Empty template section in .templates file"; - currentSection.clear(); - currentFields.clear(); - } - currentFields.clear(); + currentSection = line.mid(1, line.length() - 2); + currentFields.clear(); + if (currentSection.isEmpty()) { + qWarning() << "Empty template section in .templates file; " + "ignoring fields until next valid section"; + } } else if (!line.isEmpty() && !line.startsWith('#')) { - currentFields.append(line); + if (!currentSection.isEmpty()) { + currentFields.append(line); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util.cpp` around lines 329 - 343, The code currently collects fields after an empty "[]" header and then silently drops them; fix by introducing a sentinel (e.g., bool skipInvalidSection) so when currentSection = line.mid(1, ...) yields empty you set skipInvalidSection = true, clear currentFields once, and avoid appending subsequent non-comment non-empty lines to currentFields while skipInvalidSection is true; when you encounter the next valid header (non-empty section name) reset skipInvalidSection = false, set currentSection to the new name and proceed to insert the previous valid section as before; remove the redundant currentSection.clear() and extra currentFields.clear() and add a qWarning that fields were ignored for the invalid header to aid debugging (referencing currentSection, currentFields, and the new skipInvalidSection flag).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/util.cpp`:
- Line 18: The current nonstandard macro guards using QT_VERSION_LT_6_0 must be
replaced with the standard QT_VERSION_CHECK pattern: change the include guard at
the top to use `#if` QT_VERSION < QT_VERSION_CHECK(6,0,0) / `#else` / `#endif` so
<QStringConverter> is only included for Qt >= 6; likewise replace the two code
guards that select QTextStream::setEncoding with the same QT_VERSION_CHECK
conditional and in the Qt5 branch call QTextStream::setCodec(...) (or the
existing Qt5-compatible codec code) while only calling
QTextStream::setEncoding(...) inside the Qt6 branch; reference symbols to
update: QT_VERSION_CHECK, QStringConverter, QTextStream::setEncoding, and
QTextStream::setCodec.
---
Duplicate comments:
In `@src/util.cpp`:
- Around line 329-343: The code currently collects fields after an empty "[]"
header and then silently drops them; fix by introducing a sentinel (e.g., bool
skipInvalidSection) so when currentSection = line.mid(1, ...) yields empty you
set skipInvalidSection = true, clear currentFields once, and avoid appending
subsequent non-comment non-empty lines to currentFields while skipInvalidSection
is true; when you encounter the next valid header (non-empty section name) reset
skipInvalidSection = false, set currentSection to the new name and proceed to
insert the previous valid section as before; remove the redundant
currentSection.clear() and extra currentFields.clear() and add a qWarning that
fields were ignored for the invalid header to aid debugging (referencing
currentSection, currentFields, and the new skipInvalidSection flag).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 846327db-271a-41de-a687-3588bf019432
📒 Files selected for processing (2)
src/passworddialog.cppsrc/util.cpp
- Use QT_VERSION_CHECK(6,0,0) instead of nonstandard QT_VERSION_LT_6_0 - Only include <QStringConverter> for Qt6+ - Fix empty section handling with skipInvalidSection flag
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/util.cpp`:
- Around line 332-340: Section names extracted with currentSection = line.mid(1,
line.length() - 2) are not trimmed, so values like " Login " or " " slip
through; update the assignment and subsequent empty check to use a trimmed value
(e.g., QString trimmedSection = currentSection.trimmed()), assign trimmedSection
back to currentSection, and use trimmedSection.isEmpty() when setting
skipInvalidSection and logging; ensure result.insert() and later uses operate on
the trimmed currentSection.
- Around line 363-390: Replace the QFile usage in Util::writeTemplates with
QSaveFile (create QSaveFile QDir(storePath).filePath(".templates")), open it for
QIODevice::WriteOnly | QIODevice::Text, write via QTextStream as before (keeping
the encoding branch), then after writing check QTextStream::status() for errors
and return false if not Ok, else call saveFile.commit() and return its boolean
result (or false if commit fails); reference Util::writeTemplates,
QTextStream::status, and QSaveFile::commit when making these changes.
- Around line 402-428: Convert storePath to an absolute, cleaned path before the
loop and use that absolute store root for containment checks so the upward
traversal cannot escape the password store; specifically, replace cleanStore =
QDir::cleanPath(storePath) with something like creating a QDir
storeDir(storePath) and computing QString cleanStoreAbs =
QDir::cleanPath(storeDir.absolutePath()), then inside the loop compute
currentPath as you do and break if currentPath == cleanStoreAbs or if
currentPath does not start with cleanStoreAbs + QDir::separator() (i.e., enforce
containment) so the check uses absolute paths and prevents walking above the
store root when searching for .default_template (refer to variables storePath,
cleanStore, cleanStoreAbs, dir, currentPath).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 542a29f1-ad77-41a1-8708-f9995fba354c
📒 Files selected for processing (1)
src/util.cpp
- Add QDebug and QSaveFile includes - Trim section names in readTemplates - Use QSaveFile and check status in writeTemplates - Use absolute path containment check in getFolderTemplate
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/util.cpp`:
- Around line 407-434: The containment check fails on Windows because
QDir::cleanPath() uses '/' but sep = QDir::separator() yields '\' so
currentPath.startsWith(cleanStoreAbs + sep) is false; update the check to use a
literal '/' (or construct cleanStoreAbs + "/" ) instead of QDir::separator(),
e.g. replace uses of sep in the containment startsWith check (related symbols:
cleanStoreAbs, sep, currentPath, dir, QDir::cleanPath, QDir::separator) so
parent-folder .default_template files are correctly detected across platforms.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6c9ab77d-113d-411d-84f6-ead9e025ec34
📒 Files selected for processing (1)
src/util.cpp
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/util.cpp`:
- Around line 411-438: The loop in util.cpp currently reads ".default_template"
from dir before checking containment, allowing a read outside the store; move
the containment/termination checks that compute currentPath =
QDir::cleanPath(dir.absolutePath()) and compare against cleanStoreAbs to the top
of the while(true) loop (before any QFile access) so the code first verifies dir
is equal to or under cleanStoreAbs (use the existing checks: if currentPath ==
cleanStoreAbs break, if !currentPath.startsWith(cleanStoreAbs + "/") break, if
!dir.cdUp() break) and only then open and read ".default_template"; keep the
same variables and return behavior (templateName, start-with-# filtering) but
ensure the first iteration enforces containment.
- Line 409: Remove the dead local variable `sep` and its assignment `QString sep
= QDir::separator();` from util.cpp since `sep` is never used; leave the
existing use of the literal "/" and rely on `QDir::cleanPath()` normalize
behavior. Specifically delete the `sep` declaration (and any related unused
includes if introduced only for it) and ensure code continues to use the
existing `"/"` literal and `QDir::cleanPath()` as before.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 57ba8953-314f-4d5b-9a4b-aafe245ff347
📒 Files selected for processing (1)
src/util.cpp
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 2 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
145926b to
60a6f44
Compare
This pull request introduces multi-template support for password entries, allowing users to define and apply different password field structures.
Key changes include:
.templatesfile can be placed in the password store root. This file uses an INI-style format to define multiple templates, each with a name and a list of fields..default_templatefile can be placed in any folder within the password store. This file specifies the name of a default template to be used for that folder and its subfolders. The system will search parent directories up to the store root to find a default template.PasswordDialognow supports setting available templates and applying a specified default template, which populates the dialog's fields accordingly.readTemplates,writeTemplates,getFolderTemplate) have been added to handle reading, writing, and resolving templates and default template names from the file system.Summary by CodeRabbit