Bug 398998 - Allow kioclient exec to open kwrite/kate documents at specified cursor
Summary: Allow kioclient exec to open kwrite/kate documents at specified cursor
Status: RESOLVED FIXED
Alias: None
Product: kate
Classification: Applications
Component: general (show other bugs)
Version: Git
Platform: unspecified All
: NOR wishlist
Target Milestone: ---
Assignee: KWrite Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-09-23 19:43 UTC by Gleb Popov
Modified: 2019-07-20 15:21 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: Applications 19.04.0 with Frameworks 5.58 or later


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gleb Popov 2018-09-23 19:43:54 UTC
Kate/KWrite support "file.cpp:123:456" notion to open a text file at the specified line/row position. I use "kioclient exec" extensively to open text files and it would be super-neat to be able to open where compiler reported a error.

I poked around code a bit and it boiled down to making KRun accept plain QString instead of QUrl, because the latter can't hold cursor information. I'm ready to implement this, but first wanted to consult code owners.

For reference, Kate/KWrite use UrlInfo helper class that converts QString to QUrl with good old QUrl::fromUserInput, but additionaly chops and saves ":123:456" part at the end.
Comment 1 Aleix Pol 2018-09-23 22:03:20 UTC
I'd say best way to deal with it would be to use url semantics (e.g. file.cpp?l=23). Either way, this needs to be decided by kate maintainers.
Comment 2 Gleb Popov 2018-09-24 05:47:58 UTC
(In reply to Aleix Pol from comment #1)
> I'd say best way to deal with it would be to use url semantics (e.g.
> file.cpp?l=23). Either way, this needs to be decided by kate maintainers.

This is less comfortable to type and as I said, the main idea is to be able to copy-paste error line produced by the compiler, like:

/absolute/path/to/foo.cpp:123:1:
Couldn't find #include <foo.h>
Comment 3 Gleb Popov 2018-09-24 08:27:27 UTC
How about this:

We add KRun::setCursorInformation(int,int) method and in KRun::runApplicationImpl() or whatever actually runs the program do 

if(serviceSupportsPassingCursorInfo())
    args << "--line " << line << "--column" << column;

where

bool serviceSupportsPassingCursorInfo() { return service.name() == "KWrite" || service.name() == "Kate; }

Looks pretty clean and non-invasive to me.
Comment 4 Aleix Pol 2018-09-24 10:34:30 UTC
Moving to Kate for now.
Comment 5 Elvis Angelaccio 2019-01-05 16:52:09 UTC
(In reply to Gleb Popov from comment #2)
> (In reply to Aleix Pol from comment #1)
> > I'd say best way to deal with it would be to use url semantics (e.g.
> > file.cpp?l=23). Either way, this needs to be decided by kate maintainers.
> 
> This is less comfortable to type and as I said, the main idea is to be able
> to copy-paste error line produced by the compiler, like:
> 
> /absolute/path/to/foo.cpp:123:1:
> Couldn't find #include <foo.h>

What if kioclient converts from "foo.cpp:123:1" to url parameters? This way you can just copy-paste the thing and the patch in KIO would be less invasive (no need to add/change the KRun API).
Comment 6 Elvis Angelaccio 2019-01-05 16:55:19 UTC
Actually, if Kate adds support for row/column as URL parameters, we probably wouldn't need to patch KRun at all.
Comment 7 Gleb Popov 2019-01-05 17:36:58 UTC
(In reply to Elvis Angelaccio from comment #6)
> Actually, if Kate adds support for row/column as URL parameters, we probably
> wouldn't need to patch KRun at all.

Sounds OK, I'll try implementing it.
Comment 8 Gleb Popov 2019-01-15 11:17:19 UTC
Git commit 1fa3a0e85e7146a7a3535980aadda6ebbab11ee6 by Gleb Popov.
Committed on 15/01/2019 at 11:16.
Pushed by arrowdodger into branch 'master'.

Add ability to get cursor data from URL query arguments, like file://foo.txt?line=123&column=456

Summary: CCBUG: 398998

Test Plan: Opening a file with such URL works in both KWrite and Kate.

Reviewers: #kate

Subscribers: kwrite-devel

Tags: #kate

Differential Revision: https://phabricator.kde.org/D18099

M  +15   -0    kate/kateapp.cpp
M  +16   -0    kwrite/main.cpp

https://commits.kde.org/kate/1fa3a0e85e7146a7a3535980aadda6ebbab11ee6
Comment 9 Gleb Popov 2019-04-11 07:46:41 UTC
Git commit 8072a6acf221b8b609a781290ee2870ca2eea3dd by Gleb Popov.
Committed on 11/04/2019 at 07:41.
Pushed by arrowdodger into branch 'master'.

Add support for passing cursor information via URL parameters when running kioclient exec.

Summary:
Second part of fix for a BUG 398998 ( https://bugs.kde.org/show_bug.cgi?id=398998 ).
Kate changes are in D18099.

Test Plan: Printed resulted URL with `qDebug()`.

Reviewers: #plasma, #ktexteditor, broulik, cullmann, #frameworks, pino, cfeck, dfaure, elvisangelaccio

Reviewed By: dfaure

Subscribers: apol, cullmann, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D18296

M  +16   -5    kioclient/kioclient.cpp
M  +1    -2    kioclient/kioclient.h
A  +108  -0    kioclient/urlinfo.h     [License: LGPL]

https://commits.kde.org/kde-cli-tools/8072a6acf221b8b609a781290ee2870ca2eea3dd
Comment 10 Pit 2019-07-17 10:35:07 UTC
Please either revert or fix this patch, it is BREAKING OTHER APPLICATIONS,
see https://bugs.kde.org/show_bug.cgi?id=408632
Comment 11 Gleb Popov 2019-07-17 11:03:41 UTC
Since it was me who made this change, I'll look at it ASAP. Sorry for missing initial bug report.
Comment 12 Gleb Popov 2019-07-20 15:21:54 UTC
Fixed with https://phabricator.kde.org/D22525