Conversation
There was a problem hiding this comment.
Pull request overview
This PR replaces a regex-based approach for injecting dive profile images into HTML with a more robust XSLT transformation. The XSLT approach is more resilient to variations in HTML formatting and whitespace, making the printing functionality more reliable.
Changes:
- Added a new XSLT stylesheet (
inject-profiles.xslt) to transform HTML dive profile placeholders into image tags - Updated
printer.cppto use XSLT transformation instead of regex for profile injection (only for QLiteHtml builds) - Removed the build condition that disabled printing for DesktopExecutable when WebKit was not available
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| xslt/inject-profiles.xslt | New XSLT stylesheet that matches div elements with class 'diveProfile' and replaces them with img tags |
| subsurface.qrc | Added the new XSLT file to Qt resources so it can be loaded at runtime |
| scripts/build.sh | Removed WebKit check condition, enabling printing for all Desktop builds |
| desktop-widgets/printer.cpp | Replaced regex-based profile injection with XSLT transformation using libxml2/libxslt libraries |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
desktop-widgets/printer.cpp
Outdated
| // XSLT parameters need to be XPath expressions, so wrap string values in single quotes | ||
| QString quotedDir = QString("'%1'").arg(profileDir); | ||
| xml_params_add(params, "profile-dir", quotedDir.toUtf8().constData()); | ||
| xml_params_add(params, "img-height", "'30%'"); | ||
| xml_params_add(params, "img-width", "'30%'"); | ||
|
|
||
| xmlDocPtr transformed = xsltApplyStylesheet(xslt, doc, xml_params_get(params)); | ||
| free_xml_params(params); |
There was a problem hiding this comment.
Inconsistent indentation: Lines 320-327 use single-tab indentation while the surrounding code within the lambda uses double-tab indentation. According to the project's coding style (tabs set to 8 char), these lines should be indented with two tabs to match the indentation level of the surrounding code block.
| // XSLT parameters need to be XPath expressions, so wrap string values in single quotes | |
| QString quotedDir = QString("'%1'").arg(profileDir); | |
| xml_params_add(params, "profile-dir", quotedDir.toUtf8().constData()); | |
| xml_params_add(params, "img-height", "'30%'"); | |
| xml_params_add(params, "img-width", "'30%'"); | |
| xmlDocPtr transformed = xsltApplyStylesheet(xslt, doc, xml_params_get(params)); | |
| free_xml_params(params); | |
| // XSLT parameters need to be XPath expressions, so wrap string values in single quotes | |
| QString quotedDir = QString("'%1'").arg(profileDir); | |
| xml_params_add(params, "profile-dir", quotedDir.toUtf8().constData()); | |
| xml_params_add(params, "img-height", "'30%'"); | |
| xml_params_add(params, "img-width", "'30%'"); | |
| xmlDocPtr transformed = xsltApplyStylesheet(xslt, doc, xml_params_get(params)); | |
| free_xml_params(params); |
desktop-widgets/printer.cpp
Outdated
| struct xml_params *params = alloc_xml_params(); | ||
| // XSLT parameters need to be XPath expressions, so wrap string values in single quotes | ||
| QString quotedDir = QString("'%1'").arg(profileDir); | ||
| xml_params_add(params, "profile-dir", quotedDir.toUtf8().constData()); |
There was a problem hiding this comment.
The result of toUtf8().constData() creates a temporary QByteArray that is destroyed at the end of the statement. The constData() pointer becomes invalid immediately, but xml_params_add may store this pointer for later use. Store the QByteArray in a variable with proper lifetime to ensure the data remains valid until xml_params_get(params) is called.
| xml_params_add(params, "profile-dir", quotedDir.toUtf8().constData()); | |
| QByteArray quotedDirUtf8 = quotedDir.toUtf8(); | |
| xml_params_add(params, "profile-dir", quotedDirUtf8.constData()); |
desktop-widgets/printer.cpp
Outdated
| xml_params_add(params, "img-width", "'30%'"); | ||
|
|
||
| xmlDocPtr transformed = xsltApplyStylesheet(xslt, doc, xml_params_get(params)); | ||
| free_xml_params(params); |
There was a problem hiding this comment.
Inconsistent indentation: Line 328 uses double-tab indentation while line 327 uses single-tab indentation. Both lines should have the same indentation level as they are at the same scope.
| free_xml_params(params); | |
| free_xml_params(params); |
Replace the regex with a HTML aware XSLT transformation that will be more tolerant of differences in the HTML syntax used as input. Signed-off-by: Michael Keller <github@ike.ch>
6c0861f to
317f278
Compare
Replace the regex with a HTML aware XSLT transformation that will be more tolerant of differences in the HTML syntax used as input.