Open in KolourPaint or another associated application #50

Signed-off-by: Emanoil Kotsev deloptes@gmail.com

This is a handy function if one wants to highlight or edit a screenshot before doing something else with it.

However, I need a bit of help to finalize this PR.

How can I pass the content of the buffer from ksnapshot to kolourpaint automatically?
ATM, I can CTRL+V after kolourpaint opens.

Also an idea around 383 - check or not check if kolourpaint is running

You could use a KTempFile to store buffer data and pass that to KolourPaint.

It would be nice to offer other options along Kolourpaint, such as GIMP. Also, hardcoding paths is bad. Better use something like TDETrader

It would be nice to offer other options along Kolourpaint, such as GIMP. Also, hardcoding paths is bad. Better use something like TDETrader

Thank you for the ideas.

I remember a saying - you give it the finger, it bites the hand.

What is TDETrader?

TDETrader is part of TDEIO. In short, it allows you to dynamically query which applications support a given file type. If you want I can help you implement it. I'll test the PR shortly.

PS: Another reason we need TDETrader or something similar is so as to not hardcode paths. A user might have kolourpaint installed in a different prefix, such as /opt/tde, /usr or whatever (I know for sure Ray-V's SlackBuild scripts offer these prefixes as options for installing TDE).

Also an idea around 383 - check or not check if kolourpaint is running

Could you please explain why we need this? Launching a new instance would be better in any case IMO since this is what the user expects.

How can I pass the content of the buffer from ksnapshot to kolourpaint automatically?

I just pushed a commit that solves this problem by using a temporary file. When KolourPaint opens, it has loaded this file (seamlessly to the user) and allows editing. When the user saves the changes and closes the editor, the snapshot within KSnapshot gets updated, allowing the user to save/copy/print/drag the screenshot as usual.

Implemented KService/TDETrader functionality. Now we can be sure that KolourPaint is installed and provide options to open the snapshot in other applications associated with the according mimetype.

@deloptes I'm waiting for your feedback. I hope that my work was helpful.

Also an idea around 383 - check or not check if kolourpaint is running

Could you please explain why we need this? Launching a new instance would be better in any case IMO since this is what the user expects.

yes I also think so

How can I pass the content of the buffer from ksnapshot to kolourpaint automatically?

I just pushed a commit that solves this problem by using a temporary file. When KolourPaint opens, it has loaded this file (seamlessly to the user) and allows editing. When the user saves the changes and closes the editor, the snapshot within KSnapshot gets updated, allowing the user to save/copy/print/drag the screenshot as usual.

Thank you, it looks very good.

Now the "Open in KolourPaint" button is obsolete.

PS: I am sorry for answering that late - perhaps you had other expectations, but it is Sunday - a family day (and if it is not Sunday - it is work day).

I intentionally left the KolourPaint button as a quick shortcut when you need simple editing. IMO it is nice to have. And late reply is totally OK. Actually I was originally going to apologize for just giving bare pointers and not some working code.

I intentionally left the KolourPaint button as a quick shortcut when you need simple editing. IMO it is nice to have. And late reply is totally OK. Actually I was originally going to apologize for just giving bare pointers and not some working code.

Hi Philippe,
thank you for the great work!
I am not the finest developer and there is too much about TDE I do not know.

My original use case was to be able to highlight some part of screenshot like this

And with your help it looks perfect. See my comment.

Also it is great that it cleans up the tmp file after closing.

So I would say, I remove the WIP and it can go.

image.png

28 KiB

I just answered your comment. Once we resolve that little question you can freely merge this (of course, you might want to squash the commits into one first). Thank you for the initial work. It will be a great addition to KSnapshot!

Thank you Philippe,
I am satisfied with the result and the comment by Slavek is also fine.
I will squash and push and leave it for merging.

BR

It looks good. One small note: the git log message of your opening commit could also contain the prefix KSnaphot: .

Git log message updated, removed #include <tqbuffer.h> that is no longer needed. So we are ready to merge.

All looks good.

Needs some fixes for potential SEGV. I will post steps to reproduce it in the Conversation section soon after.

First of all, well done everyone for this. It is a very good improvement and something that was indeed missing by today's standard.

The current code can easily lead to SEGV. Steps to reproduce it:

take a snapshop and open it in an application (say KolourPaint)
without closing KSnapshot, open the same snapshot or a new one in Kolourpaint or another application.
close the application launched at step 1 (or 2)
close the other application --> KSnapshot crashes, because it is now dereferencing an invalid tmpFile.

I will create a fix and upload (either tonight or tomorrow)

There is also another issue. Take a snapshot, open the file in KolourPaint and close KSnapshot. The temporary file is not getting deleted.

Please test PR #52, which proposes a solution for the issues raised above.

 }
 void KSnapshot::slotOpenWithKP() {
     KService::Ptr kpaint = KService::serviceByDesktopName("kolourpaint");

     }
 }
 static KTempFile *tmpFile = nullptr;

 }
 void KSnapshot::slotExternalAppClosed() {
     snapshot.load(tmpFile->name());

Open in KolourPaint or another associated application #50

Reviewers