D12896: Add Telegram plugin

[Nicolas Fella]

This revision was automatically updated to reflect the committed changes.
Closed by commit R495:6f74cbb9659f: Add Telegram plugin (authored by 
nicolasfella).

REPOSITORY
  R495 Purpose Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12896?vs=34737&id=34750

REVISION DETAIL
  https://phabricator.kde.org/D12896

AFFECTED FILES
  src/plugins/CMakeLists.txt
  src/plugins/telegram/CMakeLists.txt
  src/plugins/telegram/Messages.sh
  src/plugins/telegram/debug.h
  src/plugins/telegram/telegramplugin.cpp
  src/plugins/telegram/telegramplugin.json

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns

D12896: Add Telegram plugin

[Aleix Pol Gonzalez]

apol accepted this revision.
apol added a comment.
This revision is now accepted and ready to land.


  Okay, let's land this and I'll try to fix it myself.

REPOSITORY
  R495 Purpose Library

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D12896

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns

D12896: Add Telegram plugin

[Nicolas Fella]

nicolasfella updated this revision to Diff 34737.
nicolasfella added a comment.


  :  - Hardcode commands and try until success

REPOSITORY
  R495 Purpose Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12896?vs=34213&id=34737

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D12896

AFFECTED FILES
  src/plugins/CMakeLists.txt
  src/plugins/telegram/CMakeLists.txt
  src/plugins/telegram/Messages.sh
  src/plugins/telegram/debug.h
  src/plugins/telegram/telegramplugin.cpp
  src/plugins/telegram/telegramplugin.json

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns

D12896: Add Telegram plugin

[Nicolas Fella]

nicolasfella added a comment.


  I played around a bit and it turns out more difficult than expected.
  
  - telegram-desktop-bin from AUR has telegramdesktop.desktop whereas the 
Flatpak version has org.telegram.desktop.desktop, so we would need to look for 
at least those desktop files
  - the flatpak command needs @@ around the url, the AUR version not
  - the flatpak command contains "--" which hinders the appending of the 
-sendpath parameter
  
  This makes parsing the desktop file and building the correct command utterly 
complex and when we know whether its flatpak or not we know the overall command
  
  My much simpler suggestion would be hardcoding the two commands and try them 
all until we succeed.
  Those would be
  telegram-desktop $URL (regular package)
  /usr/bin/flatpak run --file-forwarding org.telegram.desktop -sendpath @@ $URL 
@@ (flatpak)
  
  More commands (e.g. for Snap) could be added, making it more maintainable 
than complex and error-prone parsing (which would also rely on case-handling 
and assumptions)

REPOSITORY
  R495 Purpose Library

REVISION DETAIL
  https://phabricator.kde.org/D12896

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns

D12896: Add Telegram plugin

[Aleix Pol Gonzalez]

apol added a comment.


  In D12896#267091 , @nicolasfella 
wrote:
  
  > In D12896#266713 , @apol wrote:
  >
  > > Then just read the Exec field and we'll add the sendpath by hand. This is 
ad-hoc for telegram anyway.
  >
  >
  > I'm not quite sure what to do. Best I came up with is something like
  >
  >   execStr = parseExec()
  >  
  >   if (execStr.contains("flatpak"))
  >   execStr += "-sendpath @@ url @@"
  >   else
  >   execStr += "-sendpath url"
  >
  >
  > Is this what you had in mind?
  
  
  Sure, something along the lines.  I'm sure we'll be able to find an elegant 
solution.

REPOSITORY
  R495 Purpose Library

REVISION DETAIL
  https://phabricator.kde.org/D12896

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns

D12896: Add Telegram plugin

[Nicolas Fella]

nicolasfella added a comment.


  In D12896#266713 , @apol wrote:
  
  > Then just read the Exec field and we'll add the sendpath by hand. This is 
ad-hoc for telegram anyway.
  
  
  I'm not quite sure what to do. Best I came up with is something like
  
execStr = parseExec()

if (execStr.contains("flatpak"))
execStr += "-sendpath @@ url @@"
else
execStr += "-sendpath url"
  
  Is this what you had in mind?

REPOSITORY
  R495 Purpose Library

REVISION DETAIL
  https://phabricator.kde.org/D12896

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns

D12896: Add Telegram plugin

[Aleix Pol Gonzalez]

apol added a comment.


  In D12896#266351 , @nicolasfella 
wrote:
  
  > I've played around with flatpaked Telegram and made it working. However:
  >  My Telegram.desktop file executes this:
  >  /usr/bin/flatpak run --branch=stable --arch=x86_64 
--command=telegram-desktop --file-forwarding org.telegram.desktop -- @@u %u @@
  >  I can share a file using
  >  /usr/bin/flatpak run --branch=stable --arch=x86_64 
--command=telegram-desktop --file-forwarding org.telegram.desktop -sendpath @@ 
/home/nico/test.py @@
  >  But that would require changing the command I get from the desktop file. I 
can achieve the same goal just by appending something to the command from the 
desktop file:
  >  /usr/bin/flatpak run --branch=stable --arch=x86_64 
--command=telegram-desktop --file-forwarding org.telegram.desktop -- @@u %u @@ 
-sendpath @@ /home/nico/test.py @@
  >  However, appending this to the command does not work with the regular 
command.
  >  I don't see any way to support all without some special-case handling for 
flatpak. I didn't even test snap.
  >  Any ideas?
  
  
  Then just read the Exec field and we'll add the sendpath by hand. This is 
ad-hoc for telegram anyway.

REPOSITORY
  R495 Purpose Library

REVISION DETAIL
  https://phabricator.kde.org/D12896

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns

D12896: Add Telegram plugin

[Nicolas Fella]

nicolasfella added a comment.


  I've played around with flatpaked Telegram and made it working. However:
  My Telegram.desktop file executes this:
  /usr/bin/flatpak run --branch=stable --arch=x86_64 --command=telegram-desktop 
--file-forwarding org.telegram.desktop -- @@u %u @@
  I can share a file using
  /usr/bin/flatpak run --branch=stable --arch=x86_64 --command=telegram-desktop 
--file-forwarding org.telegram.desktop -sendpath @@ /home/nico/test.py @@
  But that would require changing the command I get from the desktop file. I 
can achieve the same goal just by appending something to the command from the 
desktop file:
  /usr/bin/flatpak run --branch=stable --arch=x86_64 --command=telegram-desktop 
--file-forwarding org.telegram.desktop -- @@u %u @@ -sendpath @@ 
/home/nico/test.py @@
  However, appending this to the command does not work with the regular command.
  I don't see any way to support all without some special-case handling for 
flatpak. I didn't even test snap.
  Any ideas?

REPOSITORY
  R495 Purpose Library

REVISION DETAIL
  https://phabricator.kde.org/D12896

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns

D12896: Add Telegram plugin

[Aleix Pol Gonzalez]

apol added a comment.


  In D12896#265606 , @nicolasfella 
wrote:
  
  > Can this even work with Telegram installed as sandboxed Flatpak?
  
  
  If we use the desktop file, yes. :)

REPOSITORY
  R495 Purpose Library

REVISION DETAIL
  https://phabricator.kde.org/D12896

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns

D12896: Add Telegram plugin

[Nicolas Fella]

nicolasfella added a comment.


  Can this even work with Telegram installed as sandboxed Flatpak?

REPOSITORY
  R495 Purpose Library

REVISION DETAIL
  https://phabricator.kde.org/D12896

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns

D12896: Add Telegram plugin

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> nicolasfella wrote in telegramplugin.cpp:61
> How would I do that?

https://api.kde.org/frameworks/kio/html/classKIO_1_1DesktopExecParser.html
https://cgit.kde.org/discover.git/tree/libdiscover/backends/PackageKitBackend/runservice/main.cpp?h=Plasma/5.12

REPOSITORY
  R495 Purpose Library

REVISION DETAIL
  https://phabricator.kde.org/D12896

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns

D12896: Add Telegram plugin

[Nicolas Fella]

nicolasfella added inline comments.

INLINE COMMENTS

> apol wrote in telegramplugin.cpp:61
> Instead of finding the program, we could execute using 
> `org.telegram.desktop.desktop`, it would allow it to work even if it's 
> installed from appimage/flatpak.
> 
> I'm not sure how this would work without bumping the tier.

How would I do that?

REPOSITORY
  R495 Purpose Library

REVISION DETAIL
  https://phabricator.kde.org/D12896

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns

D12896: Add Telegram plugin

[Nicolas Fella]

nicolasfella marked 2 inline comments as done.

REPOSITORY
  R495 Purpose Library

REVISION DETAIL
  https://phabricator.kde.org/D12896

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns

D12896: Add Telegram plugin

[Nicolas Fella]

nicolasfella updated this revision to Diff 34213.
nicolasfella added a comment.


  - Use url.toLocalfile()

REPOSITORY
  R495 Purpose Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12896?vs=34200&id=34213

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D12896

AFFECTED FILES
  src/plugins/CMakeLists.txt
  src/plugins/telegram/CMakeLists.txt
  src/plugins/telegram/Messages.sh
  src/plugins/telegram/debug.h
  src/plugins/telegram/telegramplugin.cpp
  src/plugins/telegram/telegramplugin.json

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns

D12896: Add Telegram plugin

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> telegramplugin.cpp:53
> +foreach(const QJsonValue& val, array) {
> +ret += toPath(val.toString());
> +}

toUrl().toLocalFile().

Also I would Check if(url.isLocalFile()).

> telegramplugin.cpp:61
> +QProcess* process = new QProcess(this);
> +process->setProgram(QStringLiteral("telegram-desktop"));
> +QJsonArray urlsJson = 
> data().value(QStringLiteral("urls")).toArray();

Instead of finding the program, we could execute using 
`org.telegram.desktop.desktop`, it would allow it to work even if it's 
installed from appimage/flatpak.

I'm not sure how this would work without bumping the tier.

> telegramplugin.cpp:63
> +QJsonArray urlsJson = 
> data().value(QStringLiteral("urls")).toArray();
> +qCDebug(PLUGIN_TELEGRAM()) << "Foo" << urlsJson;
> +process->setArguments(QStringList(QStringLiteral("-sendpath")) 
> << arrayToList(urlsJson));

Fix or remove debug message.

REPOSITORY
  R495 Purpose Library

REVISION DETAIL
  https://phabricator.kde.org/D12896

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns

D12896: Add Telegram plugin

[Nathaniel Graham]

ngraham added a comment.


  If it's 99% identical except for the name and command, would it make more 
sense to create a generic "send to app/service" plugin that you can feed 
different executables into? Then we could even expose that configuration in the 
UI somewhere to allow people to easily define their own!

REPOSITORY
  R495 Purpose Library

REVISION DETAIL
  https://phabricator.kde.org/D12896

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns

D12896: Add Telegram plugin

[Nicolas Fella]

nicolasfella created this revision.
nicolasfella added a reviewer: apol.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
nicolasfella requested review of this revision.

REVISION SUMMARY
  Add a "Share via Telegram plugin". It's a copy of the KDE Connect plugin with 
name/command substituted. Telegram expects the path without file:// prefix so 
it is removed when found.

TEST PLAN
  Share from Okular works.
  Share from Spectacle does not work since Spectacle does not give an URL but 
the image data.

REPOSITORY
  R495 Purpose Library

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D12896

AFFECTED FILES
  src/plugins/CMakeLists.txt
  src/plugins/telegram/CMakeLists.txt
  src/plugins/telegram/Messages.sh
  src/plugins/telegram/debug.h
  src/plugins/telegram/telegramplugin.cpp
  src/plugins/telegram/telegramplugin.json

To: nicolasfella, apol
Cc: kde-frameworks-devel, apol, michaelh, ngraham, bruns