Details

Reviewers

broulik
apol

Group Reviewers

KDE Connect

Commits

R224:62e2c4c1ea8d: Respect users prefered text editor for shared text
R224:47749792f32f: Respect users prefered text editor for shared text

Summary

A user may have Kate installed but another editor set as default for text/plain. Respect that.

Also add .txt extension to the temp file name to make mime-type detection easier.

BUG: 399174

Test Plan

Set default text editor to Kate -> Text opens in Kate
Set default text editor to Atom -> Text opens in Atom

Diff Detail

Repository

R224 KDE Connect

Branch

editor2

Lint

Lint OK

Unit

No Unit Test Coverage

Build Status

Buildable 3298
Build 3316: arc lint + arc unit

nicolasfella created this revision.Sep 28 2018, 1:07 PM

Restricted Application added a project: KDE Connect. · View Herald TranscriptSep 28 2018, 1:07 PM

Restricted Application added a subscriber: kdeconnect. · View Herald Transcript

nicolasfella requested review of this revision.Sep 28 2018, 1:07 PM

Harbormaster completed remote builds in B3285: Diff 42474.Sep 28 2018, 1:07 PM

-1 I liked that using Kate didn't require writing to a temporary file.
How about asking KMimeTypeTrader for preferred service for text/plain being Kate? (KWrite also supports --stdin btw which is more common on default installs iirc)

+1 on the file template part

Check if Kate or KWrite is default and use --stdin then

Harbormaster completed remote builds in B3286: Diff 42478.Sep 28 2018, 1:58 PM

Whitespace--

Harbormaster completed remote builds in B3287: Diff 42479.Sep 28 2018, 1:59 PM

apol accepted this revision.Sep 28 2018, 2:01 PM

This revision is now accepted and ready to land.Sep 28 2018, 2:01 PM

broulik added inline comments.Sep 28 2018, 2:01 PM

plugins/share/shareplugin.cpp
115	`preferredService` can return `null`
116	Compare with `QLatin1String`
120	You can use `QString::section`

Use QString::section
Use QLatin1String
Check preferredService for null

Harbormaster completed remote builds in B3292: Diff 42484.Sep 28 2018, 2:12 PM

nicolasfella marked 3 inline comments as done.Sep 28 2018, 2:12 PM

apol requested changes to this revision.Sep 28 2018, 2:13 PM

apol added inline comments.

plugins/share/shareplugin.cpp
120	Ugh yeah, no. Extract the executable name from the server, not from the id. They may be different!

This revision now requires changes to proceed.Sep 28 2018, 2:13 PM

broulik accepted this revision.Sep 28 2018, 2:14 PM

broulik added inline comments.

plugins/share/shareplugin.cpp
115	I think that `&` should go, given you may also assign temporary `QString()` to it
120	I was about to suggest parsing `Exec` but this is complicated and might have additional incompatbile arguments, and it can only be "kate" and "kwrite" at this point given the `if` above. Could probably use the first part of `Exec` but imho this way is fine'ish.

Remove &
Use service->exec()

Harbormaster completed remote builds in B3295: Diff 42487.Sep 28 2018, 2:23 PM

broulik added inline comments.Sep 28 2018, 2:24 PM

plugins/share/shareplugin.cpp
118	Remove debug
120	Doesn't that contain stuff like `%U` or is that already resolved in `KService`?

Debug--

Harbormaster completed remote builds in B3296: Diff 42488.Sep 28 2018, 2:26 PM

nicolasfella added inline comments.Sep 28 2018, 2:26 PM

plugins/share/shareplugin.cpp
120	It does contain %U, but it doesn't seem to hurt

nicolasfella added inline comments.Sep 28 2018, 2:28 PM

plugins/share/shareplugin.cpp
120	It does for KWrite...

Revert to splitting the id, which is IMHO fine as it will only be Kate or KWrite

Harbormaster completed remote builds in B3298: Diff 42490.Sep 28 2018, 2:31 PM

broulik accepted this revision.Sep 28 2018, 2:31 PM

In D15813#333375, @nicolasfella wrote:

Revert to splitting the id, which is IMHO fine as it will only be Kate or KWrite

You can do the same you did for Purpose Telegram...

apol accepted this revision.Feb 8 2019, 10:22 PM

This revision is now accepted and ready to land.Feb 8 2019, 10:22 PM

Closed by commit R224:47749792f32f: Respect users prefered text editor for shared text (authored by nicolasfella). · Explain WhyFeb 8 2019, 11:01 PM

This revision was automatically updated to reflect the committed changes.

			Path	Packages
M			CMakeLists.txt (2 lines)
M			plugins/share/CMakeLists.txt (1 line)
M			plugins/share/shareplugin.cpp (10 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	42474	e932345		Sep 28 2018, 1:07 PM	★	★
Diff 2	42478	7a4bdcc	Check if Kate or KWrite is default and use --stdin then	Sep 28 2018, 1:58 PM	★	★
Diff 3	42479	7a4bdcc	- Whitespace--	Sep 28 2018, 1:59 PM	★	★
Diff 4	42484	7a4bdcc	- Use QString::section	Sep 28 2018, 2:11 PM	★	★
Diff 5	42487	7a4bdcc	- Remove &	Sep 28 2018, 2:23 PM	★	★
Diff 6	42488	7a4bdcc	- Debug--	Sep 28 2018, 2:26 PM	★	★
Diff 7	42490	7a4bdcc	- Revert to splitting the id, which is IMHO fine as it will only be Kate or…	Sep 28 2018, 2:31 PM	★	★
Diff 8	51228	e298b73	R224:47749792f32f4500a9efd3e3644309830723db2a	Feb 8 2019, 11:00 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
73730c4d00e1	2b7f37c87617	06309f5a6129	Nicolas Fella	Revert to splitting the id, which is IMHO fine as it will only be Kate or KWrite	Sep 28 2018, 2:31 PM
06309f5a6129	b7f7dc3a4eca	eba8a02fa3a0	Nicolas Fella	Debug--	Sep 28 2018, 2:25 PM
eba8a02fa3a0	4c20493cc640	b68141514cff	Nicolas Fella	Use service->exec()	Sep 28 2018, 2:23 PM
b68141514cff	e0aa66740ff5	17f4ed1183a0	Nicolas Fella	Check preferredService for null	Sep 28 2018, 2:11 PM
17f4ed1183a0	291d86877cec	0db1beefab54	Nicolas Fella	Use QLatin1String	Sep 28 2018, 2:06 PM
0db1beefab54	4764ee9e7af9	de19cb89129d	Nicolas Fella	Use QString::section	Sep 28 2018, 2:06 PM
de19cb89129d	e14dcd2f636f	b0c71baf202b	Nicolas Fella	Whitespace--	Sep 28 2018, 1:59 PM
b0c71baf202b	79696999d092	7a4bdcc62d32	Nicolas Fella	Check if Kate or KWrite is default and use --stdin then	Sep 28 2018, 1:57 PM

Respect users prefered text editor for shared text
ClosedPublic
Actions

Details

Diff Detail

Revision Contents
Changeset List

Diff 42490

CMakeLists.txt

plugins/share/CMakeLists.txt

plugins/share/shareplugin.cpp

Respect users prefered text editor for shared textClosedPublicActions

Details

Diff Detail

Revision ContentsChangeset List

Diff 42490

CMakeLists.txt

plugins/share/CMakeLists.txt

plugins/share/shareplugin.cpp

Respect users prefered text editor for shared text
ClosedPublic
Actions

Revision Contents
Changeset List