New plugin to open the selected file path
Needs ReviewPublic

Authored by nononux on Jul 1 2019, 7:30 PM.



The plugin allow the user to open the selected file path in the current document. If there is no selection, the plugin try to detect the path under the cursor.
I've see that functionnality in the editor called "nedit" and find it great as I often manipulate "address files" containing a list of paths.

Diff Detail

R40 Kate
openselection (branched from master)
No Linters Available
No Unit Test Coverage
Build Status
Buildable 13532
Build 13550: arc lint + arc unit
nononux created this revision.Jul 1 2019, 7:30 PM
Restricted Application added projects: Kate, Documentation. · View Herald TranscriptJul 1 2019, 7:30 PM
Restricted Application added subscribers: kde-doc-english, kwrite-devel. · View Herald Transcript
nononux requested review of this revision.Jul 1 2019, 7:30 PM
yurchor added a subscriber: yurchor.Jul 2 2019, 4:25 AM

Thanks in advance for fixing the issue with docs.


This seems to be a non-standard way to define a menu item. The other items are just verbs "Open", "Save, not "Opens" or "Saves".


Should be


pino added a subscriber: pino.Jul 2 2019, 5:16 AM

In addition to the assumption done about a path being a local file: what if the selection contains more than one path/URL?


Please do not add translations manually, there is a KDE-wide system to handle them.




I guess there is no need for commented code in new code.


In addition to what Yuri said:a better action text, more in line with our HIG [1] is IMHO "Open Selected Path".
"Opens the selected path" can be a good tooltip or whatsthis text.



selection.chop(1) is easier. Even better, directly trim the string to remove all whitespaces at the beginning and at the end.


This condition (and the same below for end) is hard to read, as it mixes checks and an assignment mid-way.
A suggestion to make it simpler, and also avoid the duplicated checks is to put the character checks in a small helper:

static bool isValidChar(QChar c)
  return c != QLatin1Char(' ') && c != QLatin1Char('\t') && c != QLatin1Char('"') && c != QLatin1Char('\'');

and thus using it:

while (start > 0 && isValidChar( - 1))

Way more readable IMHO.

Also, most probably other characters can be excluded from what is a file path -- for example word boundaries, newlines, etc. Check the QChar API documentation to see whether there are character classes that can help here.


Excluding the newline in the code above (see my isValidChar() suggestion) can avoid the need to check for newline here.


This assumes the string is a local file -- what if under the cursor there is a remote URL?


Since it is a new plugin, I'd start with version=1.

dhaumann added inline comments.

A view always has a valid document, no need to check editView->document()


QString is implicitly shared. Please remove the '&' here.


In general: Please use spaces around operators and adhere to the standard KDE coding guidelines (also used in Kate everywhere else).

nononux updated this revision to Diff 61022.Jul 2 2019, 6:14 PM

Taking into account the proposed changes

nononux marked 11 inline comments as done.Jul 2 2019, 6:36 PM

Thanks for your advices for my first KDE dev. I hope I've taken them into account in a good way.


I removed all the translations. Is it a dedicated team who do the translations ?


I've put "Open selected path" as it seems the others texts in my menu doesn't have all first letters capitalized.


Done, I use isSpace() to capture more characters (\n, \r, ...). Maybe it is better to put the function in the class as a private function ?


I think the test should be kept : if a multi-line text is selected, it can contain a newline (not removed by the trimmed function).


The name of the function 'fromLocalFile' is a bit ambiguous. The QUrl doc ( says this function work with remote files, with a path starting with //.

yurchor added inline comments.Jul 2 2019, 6:46 PM

Sure. We (translation team) do.

Then scripty (our friendly translation script) put the translations where they should be automatically.

nononux updated this revision to Diff 61026.Jul 2 2019, 7:17 PM

Improve coding guidelines respect

nononux updated this revision to Diff 61027.Jul 2 2019, 7:21 PM

Correction, forgot to test before send

nononux marked 9 inline comments as done.Jul 5 2019, 7:21 PM
nononux updated this revision to Diff 61239.Jul 5 2019, 7:22 PM

Change the shortcut to Alt+O as Ctrl+Shift+O was already used for the spelling correction

pino added inline comments.Jul 6 2019, 3:31 AM
59 ↗(On Diff #61026)

This should be "Open Selected Paths"; see


This is more general thing to take into account, as I mentioned in another comment: what if the selection contains more than two paths? A space or a newline between them does not make much difference.


No, QUrl::fromLocalFile() always assumes it is a local file, using a local "file" protocol. The "remote files" mentioned there are simply Qt things, not a general stuff -- it will not handle at all.

nononux updated this revision to Diff 61266.Jul 7 2019, 8:04 AM

Add support of remote paths (http://) + change action name

nononux marked 2 inline comments as done.Jul 7 2019, 8:12 AM
nononux added inline comments.
59 ↗(On Diff #61026)

Ok, changed. In fact, menus are not capitalized in my native language, but they are with "LANG=en_US.UTF-8 kate", hence the misunderstanding.


It won't open anything if there are more than one path in the selection (the file existence test will fail). I don't think it's possible to manage multiple paths without doing important assumptions on separators. Maybe you have a good idea to manage them ?


I changed the code to support http://-like paths.

Hi, can the code be pushed ? I don't think there are still changes to do.
(I don't have a dev account)

dhaumann requested changes to this revision.Jul 21 2019, 7:09 PM

The KTextEditor::Command implementation is wrong, see my other comments. Please fix this first :-)

81 ↗(On Diff #61026)

A PluginViewKateOpenSelection instance is created here for every KTextEditor::MainWindow - this is ok and works as designed / intended.

However, PluginViewKateOpenSelection inherits KTextEditor::Command, which itself registers in its constructor with the command "open-selection" in the Kate's command line. Now when you create multiple MainWindows in Kate (View > New Window), then you will add *the same* "open-selection" command twice, which is wrong.

In short: PluginViewKateOpenSelection must not inherit KTextEditor::Command. Instead, the PluginKateOpenSelection should inherit it.

108 ↗(On Diff #61026)

Please factor this code part out into a separate function: QString findPathAtCursor(...).

Reasoning: Currently, you are mixing different levers of logic, i.e. slotOpenSelection() should open the selection, but not itself do the work to find the path under a cursor.

This revision now requires changes to proceed.Jul 21 2019, 7:09 PM
nononux updated this revision to Diff 62250.Jul 21 2019, 10:08 PM

Fix the required parts, I hope it's ok this time :)

nononux marked 2 inline comments as done.Jul 21 2019, 10:11 PM
nononux added inline comments.
81 ↗(On Diff #61026)

As a class cannot inherit from 2 QObjects, I've made another class included in the plugin as a member.