Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: HTML export from CLI tool #11590

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

AdriandMartin
Copy link

This commit introduces support for exporting a KeePassXC database in HTML format via the CLI tool. The key changes include:

  • Refactoring HtmlExporter:
    • Moved HtmlExporter to the format directory and made its API compatible with CsvExporter.
    • Since the original HtmlExporter had a direct dependency on the gui/Icons functions and indirect dependencies on the gui/DatabaseIcons class, only the non-GUI parts were moved to format/HtmlExporter.
    • All icon-related functionality was encapsulated in a new child class, gui/HtmlGuiExporter.
      • The gui/HtmlGuiExporter retains the original functionality of the HtmlExporter class.
      • The format/HtmlExporter now generates HTML export without icons. Adding icon support to format/HtmlExporter would require moving icon management logic to the core, which could have broader implications.
  • CLI integration:
    • Updated cli/Export to use format/HtmlExporter.
  • GUI Integration:
    • Updated gui/export/ExportDialog to use gui/HtmlGuiExporter.
  • Build System Updates:
    • Updated CMakeLists.txt to build HtmlExporter as part of core_SOURCES and HtmlGuiExporter as part of gui_SOURCES.
  • Testing:
    • Updated TestCli to automatically verify the output of the HTML export.

Screenshots

Testing strategy

Type of change

  • ✅ Bug fix (non-breaking change that fixes an issue)
  • ✅ New feature (change that adds functionality)
  • ✅ Breaking change (causes existing functionality to change)
  • ✅ Refactor (significant modification to existing code)
  • ✅ Documentation (non-code change)

This commit introduces support for exporting a KeePassXC database in
HTML format via the CLI tool. The key changes include:
- Refactoring HtmlExporter:
  - Moved HtmlExporter to the format directory and made its API
    compatible with CsvExporter.
  - Since the original HtmlExporter had a direct dependency on the
    gui/Icons functions and indirect dependencies on the
    gui/DatabaseIcons class, only the non-GUI parts were moved to
    format/HtmlExporter.
  - All icon-related functionality was encapsulated in a new child
    class, gui/HtmlGuiExporter.
    - The gui/HtmlGuiExporter retains the original functionality of the
      HtmlExporter class.
    - The format/HtmlExporter now generates HTML export without icons.
      Adding icon support to format/HtmlExporter would require moving
      icon management logic to the core, which could have broader
      implications.
- CLI integration:
  - Updated cli/Export to use format/HtmlExporter.
- GUI Integration:
  - Updated gui/export/ExportDialog to use gui/HtmlGuiExporter.
- Build System Updates:
  - Updated CMakeLists.txt to build HtmlExporter as part of core_SOURCES
    and HtmlGuiExporter as part of gui_SOURCES.
- Testing:
  - Updated TestCli to automatically verify the output of the HTML
    export.

Signed-off-by: AdriandMartin <adriandmartin@protonmail.com>
@droidmonkey
Copy link
Member

Since icons are actually stored as QByteArray PNG's you should be able to avoid the special GUI exporter. Don't pull the Pixmap just go straight for the data.

@droidmonkey droidmonkey self-requested a review December 23, 2024 17:56
This commit includes the required formatting changes to comply with the
project's contribution guidelines:
- Variable Naming:
  - Redefined variables to adhere to the established naming conventions.
- Code Formatting:
  - Applied `make format` to ensure all modified files follow the coding
    style guide.
- Localization Updates:
  - Ran `release-tool` to make the new strings available for translation.

Signed-off-by: AdriandMartin <adriandmartin@protonmail.com>
@AdriandMartin
Copy link
Author

Hi Jonathan,

First of all, thank you for your prompt review. I’ve tried to implement the changes you suggested, removing the dependency of the functions Icons::{entry,group}IconPixmap to have a single HtmlExporter class, but after several attempts, I encountered the following issues:

  • When {entry,group}->iconUuid() is not null, obtaining the icon as you described is straightforward. However, the transformations performed by Icons::customIconPixmap appear to be necessary to maintain the quality and size of the exported icon. As a result, it’s not possible to achieve the same result without replicating the code from that function.
  • When {entry,group}->iconUuid() is null, the icon is retrieved from the DatabaseIcons cache using DatabaseIcons::icon. I couldn’t find an easy way to access these icons without breaking or replicating the encapsulation within that class. As a workaround, I tried to move their files to core, which required updating the include macros across several files. While this allowed me to build the project successfully and the HTML export worked as expected in the GUI tool, I encountered the runtime error QPlatformPixmap: QGuiApplication required when attempting to export the database as HTML via the CLI tool. My initial research suggests this issue may be related to graphics capabilities being required for QPixmap manipulations, which are unavailable when using QCoreApplication. However, further investigation is needed to confirm this.

Given these challenges, I think the best solution might be to maintain two separate classes for handling HTML export in different execution contexts. That said, I’d appreciate your feedback if I’ve missed anything, as I’m not deeply familiar with the project’s source code. Any additional change requests are welcome.

Additionally, I've just pushed a commit that should resolve the CI errors. I have to apologize, because I hadn't read the contribution guide before, so I didn't execute make format and release-tool before opening the PR.

@droidmonkey
Copy link
Member

droidmonkey commented Dec 24, 2024

Ah yes I forgot that non custom icons actually draw from the applications icon cache which doesn't get compiled into the cli. Nevermind with my suggestion than, thank you for trying it though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants