Re: Using KCompressionDevice with QSaveFile

[Dominik Haumann]

On Tuesday 10 December 2013 23:31:33 David Faure wrote:
 On Tuesday 10 December 2013 21:48:09 Dominik Haumann wrote:
  It seems that the behavior for None is different. Interestingly,
  KSaveFile also behaved like this (KFilterDev did not change after all I
  guess).
 I'm surprised, because KCompressionDevice::None didn't exist before, it was
 all new code in KF5. KFilterDev had very different code in KDE4, and no
 support for no compression itself (deviceFromFile would simply return a
 QFile instead of a KFilterDev, in that case).
 
 Anyhow, it's fixed now, I rewrote the new none filter.

Just for reference, this works now in Kate Part.

Thanks!
Dominik
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Using KCompressionDevice with QSaveFile

[Dominik Haumann]

Hi David,

On Wednesday 04 December 2013 14:42:19 David Faure wrote:
 On Monday 02 December 2013 22:31:43 Dominik Haumann wrote:
  Hi,
  
  porting KSaveFile to QSaveFile, I stumbled into the following:
  
  // This method has been made private so that it cannot be called,
  // in order to prevent mistakes.
  void QSaveFile::close()
  {
  
  qFatal(QSaveFile::close called);
  
  }
  
  In Kate, we use a QSaveFile saveFile(...); and then use
  
KCompressionDevice compDevice(saveFile, ...);
  
  to write compressed data through compDevice into saveFile.
  
  However, KCompressionDevice at some point always seems to call close(),
  which hits the qFatal().
  
  Should I derive from QSaveFile and overwrite close() to call commit() ?
  Am I missing something here?
 
 I just added a unittest in kdelibs-frameworks's kfiltertest, and it works
 for me.
 Can you check 1) that it works for you, 2) what your code does differently
 from the unittest ? And then change the unittest to show the problem, so I
 can look into it.
 
 Thanks!

First of all, thanks a lot for adding the unit test and looking into this.
It turns out that it also runs correctly for me (not really a surprise, right?) 
;)

However, here it comes: Changing 
-KCompressionDevice device(file, false, KCompressionDevice::GZip);
+KCompressionDevice device(file, false, KCompressionDevice::None);
will run into this issue.

In that case, the unit test does the following:
QWARN  : KFilterTest::test_saveFile() QSaveFile::open: File 
(tier1/karchive/autotests/test_saveFile.gz) already open
QFATAL : KFilterTest::test_saveFile() QSaveFile::close called
FAIL!  : KFilterTest::test_saveFile() Received a fatal error.

First, we get the warning that the file is already open.
Then, the assert.

It seems that the behavior for None is different. Interestingly, KSaveFile 
also
behaved like this (KFilterDev did not change after all I guess). And this case 
was
caught with an if in Kate Part's file saving code.

So yes, we can work around it again, just before.

But: I do not really get why KCompressionDevice works completely different 
depending
on the compression type. This makes using KCompressionDevice pretty ugly 
(im_h_o),
since you have to manually check for None and take a different code path.

Greetings,
Dominik
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Using KCompressionDevice with QSaveFile

[Dominik Haumann]

On Tuesday 10 December 2013 23:31:33 David Faure wrote:
 On Tuesday 10 December 2013 21:48:09 Dominik Haumann wrote:
  However, here it comes: Changing
  -KCompressionDevice device(file, false,
  KCompressionDevice::GZip);
  +KCompressionDevice device(file, false,
  KCompressionDevice::None);
  will run into this issue.
 
 Great. A unittest patch to make it fail was exactly what I needed :)

Very good.

  In that case, the unit test does the following:
  QWARN  : KFilterTest::test_saveFile() QSaveFile::open: File
  (tier1/karchive/autotests/test_saveFile.gz) already open QFATAL :
  KFilterTest::test_saveFile() QSaveFile::close called
  FAIL!  : KFilterTest::test_saveFile() Received a fatal error.
 
 Yep.
 
  It seems that the behavior for None is different. Interestingly,
  KSaveFile also behaved like this (KFilterDev did not change after all I
  guess).
 I'm surprised, because KCompressionDevice::None didn't exist before, it was
 all new code in KF5. KFilterDev had very different code in KDE4, and no
 support for no compression itself (deviceFromFile would simply return a
 QFile instead of a KFilterDev, in that case).

I did not look too closely into the KDE4 way, but we needed some kind of if 
there, too (according to Christoph). Well, nevermind ;)

 Anyhow, it's fixed now, I rewrote the new none filter.

Thanks! I will try on the weekend.

  And this case was caught with an if in Kate Part's file saving code.
  So yes, we can work around it again, just before.
  But: I do not really get why KCompressionDevice works completely different
  depending on the compression type. This makes using KCompressionDevice
  pretty ugly (im_h_o), since you have to manually check for None and take a
  different code path.
 
 No need to convince me, I'm fully convinced :-)

;)

Best,
Dominik
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Using KCompressionDevice with QSaveFile

[Aleix Pol]

On Mon, Dec 2, 2013 at 10:31 PM, Dominik Haumann dhaum...@kde.org wrote:

 Hi,

 porting KSaveFile to QSaveFile, I stumbled into the following:

 // This method has been made private so that it cannot be called,
 // in order to prevent mistakes.
 void QSaveFile::close()
 {
 qFatal(QSaveFile::close called);
 }

 In Kate, we use a QSaveFile saveFile(...); and then use

   KCompressionDevice compDevice(saveFile, ...);

 to write compressed data through compDevice into saveFile.

 However, KCompressionDevice at some point always seems to call close(),
 which
 hits the qFatal().

 Should I derive from QSaveFile and overwrite close() to call commit() ?
 Am I missing something here?

 Greetings,
 Dominik
 ___
 Kde-frameworks-devel mailing list
 Kde-frameworks-devel@kde.org
 https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Looks to me like it's either a bug on KCompressionDevice or QSaveFile.
Personally, I think it's rather odd that some public API call invariably
crashes (it's made private in QSaveFile, but it's still public on the
parent, so it's not very useful).

I'd say that this fix should go to Qt, but David will probably have a
better insight.
Mr Faure? :D

Aleix
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel