Re: [PATCH] Fix Viewing of XHTML with Math as Images

2013-08-05 Thread Richard Heck

On 08/05/2013 05:45 PM, Vincent van Ravesteijn wrote:



Op 5 aug. 2013 23:25 schreef "Richard Heck" > het volgende:

>
> On 08/04/2013 05:43 PM, Richard Heck wrote:
>>
>>
>> The attached patch explains the problem and offers a simple 
solution. This is master-only, and it needs to go into 2.1. OK?

>
>
> Someone want to approve this? or not?
>
>> Richard
>>
>

First, please shorten the subject line to around 60 chars.



Do you mean the email subject? Or...?


Second, it is ugly that PreviewImage depends on Buffer.

Third, it doesn't feel good to leave behind preview images, just in 
case the export XHTML needs them.


Fourth, I don't like removing files in a destructor in the first 
place. It should be a conscious decision to do that. Compare also 
hideDialogs calls in Inset destructors. If you would need to delete 
the files explicitly, it would also be easier to not delete them when 
appropriate.


Fifth, do you use isClone to detect whether we are exporting ? Seems 
like a hack that deserves a comment.




I agree with all of this. But this patch more or less restores the 
behavior from before Abdel broke it (while fixing a lot else). If I were 
doing this now, I'd do it differently. And I will do it differently. I 
think we should copy the preview images during the XHTML export process, 
and then leave those lying around (in the temporary directory, of 
course, so they will eventually get deleted). But it does not seem a 
good idea to do that now. Still, if you like, I can prepare a patch for 
that behavior, and you can decide whether to do it now or wait until 
after 2.1.0.


Richard



Re: [PATCH] Fix Viewing of XHTML with Math as Images

2013-08-05 Thread Vincent van Ravesteijn
Op 5 aug. 2013 23:25 schreef "Richard Heck"  het volgende:
>
> On 08/04/2013 05:43 PM, Richard Heck wrote:
>>
>>
>> The attached patch explains the problem and offers a simple solution.
This is master-only, and it needs to go into 2.1. OK?
>
>
> Someone want to approve this? or not?
>
>> Richard
>>
>

First, please shorten the subject line to around 60 chars.

Second, it is ugly that PreviewImage depends on Buffer.

Third, it doesn't feel good to leave behind preview images, just in case
the export XHTML needs them.

Fourth, I don't like removing files in a destructor in the first place. It
should be a conscious decision to do that. Compare also hideDialogs calls
in Inset destructors. If you would need to delete the files explicitly, it
would also be easier to not delete them when appropriate.

Fifth, do you use isClone to detect whether we are exporting ? Seems like a
hack that deserves a comment.

But you'll probably want a solution to the problem, but that's difficult to
come up with right away.

Vincent


Re: [PATCH] Fix Viewing of XHTML with Math as Images

2013-08-05 Thread Richard Heck

On 08/04/2013 05:43 PM, Richard Heck wrote:


The attached patch explains the problem and offers a simple solution. 
This is master-only, and it needs to go into 2.1. OK?


Someone want to approve this? or not?


Richard





[PATCH] Fix Viewing of XHTML with Math as Images

2013-08-04 Thread Richard Heck


The attached patch explains the problem and offers a simple solution. 
This is master-only, and it needs to go into 2.1. OK?


Richard

>From 55e562c18eaff4c42629fb215e6729363bb5ca7a Mon Sep 17 00:00:00 2001
From: Richard Heck 
Date: Sun, 4 Aug 2013 17:40:35 -0400
Subject: [PATCH] Fix View> LyXXHTML when math is generated as images. The
 rewrite to the preview mechanism broke this, since the
 images get deleted just as the file is being viewed.

Probably, a better solution would be to copy the images we know we
need, and somehow tell the main buffer about them, rather than leaving
all the generated previews lying around. But this works for now.
---
 src/graphics/PreviewImage.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/graphics/PreviewImage.cpp b/src/graphics/PreviewImage.cpp
index 3185365..5772927 100644
--- a/src/graphics/PreviewImage.cpp
+++ b/src/graphics/PreviewImage.cpp
@@ -12,6 +12,7 @@
 
 #include "PreviewImage.h"
 
+#include "Buffer.h"
 #include "Dimension.h"
 #include "GraphicsImage.h"
 #include "GraphicsLoader.h"
@@ -111,7 +112,8 @@ PreviewImage::Impl::Impl(PreviewImage & p, PreviewLoader & l,
 
 PreviewImage::Impl::~Impl()
 {
-	iloader_.filename().removeFile();
+	if (!ploader_.buffer().isClone())
+		iloader_.filename().removeFile();
 }
 
 
-- 
1.7.11.7