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

Add template variables for pictures #3430

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

atdotde
Copy link
Collaborator

@atdotde atdotde commented Apr 1, 2022

This introduces a new list the printing templates can loop
over: pictures contains the urls (local or remote) of media
associtated to the current dive. This allows constructs like

{% for pic in pictures %}
	<img src="{{ pic.url }}" width=20%>
{% endfor %}

in the printing template.

Signed-off-by: Robert C. Helling helling@atdotde.de

Describe the pull request:

  • Bug fix
  • Functional change
  • New feature
  • Code cleanup
  • Build system change
  • Documentation change
  • Language translation

Pull request long description:

Changes made:

Related issues:

Additional information:

Release note:

Documentation change:

Mentions:

Copy link
Collaborator

@dirkhh dirkhh left a comment

Choose a reason for hiding this comment

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

the builds are failing with fascinating C++ error messages...

static std::vector<const QString> pictureList(const dive *d)
{
std::vector<const QString> res;
res.reserve(d->pictures.nr); //FIXME
Copy link
Collaborator

Choose a reason for hiding this comment

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

always love seeing FIXME comments... :(

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

According to April 1st, "FIXME" is there as a reminder to remove the "FIXME" comment. (The actual thing to fix I had already fixed but of course forgot the comment).

@atdotde atdotde force-pushed the printpictures branch 2 times, most recently from 9732406 to 5495e4a Compare April 2, 2022 05:59
This introduces a new list the printing templates can loop
over: pictures contains the urls (local or remote) of media
associtated to the current dive. This allows constructs like

{% for pic in pictures %}
	<img src="{{ pic.url }}" width=20%>
{% endfor %}

in the printing template.

Signed-off-by: Robert C. Helling <helling@atdotde.de>
@atdotde
Copy link
Collaborator Author

atdotde commented Apr 2, 2022

Yeah, these error messages were fascinating, WTF. Again, I learned I should stay away from attempting to use C++ standard library data types, they are beyond my pay scale (fixed the problem by replacing a std::vector by a QStringList).

Honestly, what is the point of abstraction when you cannot use it as a black box but have to worry about the intestines?

@atdotde
Copy link
Collaborator Author

atdotde commented Apr 2, 2022

In other news, I found this to work only intermittently. Could others please test? There seems to be a race condition with downloading the actual images. This might be related to what stopped us in #3354

@atdotde
Copy link
Collaborator Author

atdotde commented Apr 2, 2022

Here is the template I use for testing
imagetemplate.txt.

@dirkhh
Copy link
Collaborator

dirkhh commented Apr 2, 2022

There is this very weird thing going on in our printing code where we apparently cannot wait for everything to fully "render" before things get printed.

In a way I'd love to completely rethink the way we do this. Get rid of WebView and WebKit and those gigantic libraries for something that really shouldn't be that hard. There's got to be a simpler way to lay out the template and create something that we can then send to the printer routines. These don't need to be web pages.

But of course that's a major project that will take time and patience and that's the thing that I am clearly lacking - as is most everyone else.

Still - what we do today is a dead end - and the "obvious" switch to WebView doesn't work on Windows, so it's also not the right path.

There has got to be a better way to do this.

@thiagomacieira how do other Qt apps print? Do they all use WebKit / WebView? Or is there another preferred mechanism?

@dirkhh
Copy link
Collaborator

dirkhh commented Apr 3, 2022

and after my, I don't know, fifth (?) trip down this rat hole... I still have no better idea how to do this.
Yes, you can totally use widgets to assemble a page and then print this. But you need to write the code to make this flow and page-inate nicely. And that feels like such a pain in the rear. And wherever you look people just say "oh just use a WebView", ignoring the tens of megabyte of crud and platform issues that this brings.
The fact that I cannot seem to find a simpler, easy to integrate solution for this problem (which I have got to think is fairly common) is just sad...

@dirkhh
Copy link
Collaborator

dirkhh commented Apr 3, 2022

To go back to your pull request - yes, I can make it fail fairly reliably. There must be a way to ensure that things have finished rendering before we print. We MUST be doing something wrong here.
Gah.

@thiagomacieira
Copy link
Contributor

@thiagomacieira how do other Qt apps print? Do they all use WebKit / WebView? Or is there another preferred mechanism?

Poorly. QtPrintSupport has seen very little support in the last decade or so. It's been an issue for a lot of applications -- I remember KMail having the same problem.

@atdotde
Copy link
Collaborator Author

atdotde commented Apr 8, 2022

Maybe we can talk to a car manufacturer to add a printer to the car. That might add encouragement to give art printing done TLC.

@atdotde
Copy link
Collaborator Author

atdotde commented Apr 8, 2022

Next thing I will try is to ditch QWebWhatever and try if the limited html of QTextEdit might s suffice.

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.

3 participants