As I'm not allowed to leave this comment empty I might as well state that I am completely new to *ALL* technologies involved here i.e. C++, Qt, git, phabricator etc
(I can read though).

Yippieh: Smiling from ear to ear here. Thanks for your reply!

Up to a few minutes ago I didn't even think about Licensing, sorry. The heap of things I have to learn just grew a little. Now there are 2 Questions:

1. How did you find out who is responsible for that dolphin code?
2. What would have been the proper way to ask the author for his agreement?

I guess you do not have git commit access?

I guess not. (Not even quite sure what that is.). I fetched all the code with kdesrc-build, which was an excellent way to get started BTW

I also guess this touches my third Question: What next?

I'll give some hints of what I did:

• you gave me the hint to src/kde/applications/dolphin/src/panels/terminal/terminalpanel.cpp@167
• I checked out the dolphin git repo
• went to the terminal folder, and typed: git annotate terminalplane.cpp <enter>, and went to line 167
• This tells me Peter Penz, but it's not yet sure he was really the initial author.
• then I entered: git log -S "kill(processId, SIGINT);"
• essentially, this command shows me ALL commits with changed lines that contain the string "kill(processId, SIGINT);"
• It shows 2 commits, both from Peter Penz.
• I copied the sha 14d5a32475ef646e51f6750a80f216020b648039 and entered in the github search, which gives 3 code matches.

With that, I have a final link to the file, showing the commit on the web that added the change. :-)

There may be better ways to find this out, but I once learned the git log -S "text" trick and it works for me. You may want to see if you find better ways, though.

Now that we know it was Peter Penz who added this, we usually should add him as reviewer here in Phabricator. But he stopped KDE development many years ago. The only way to reach him would be to send a direct mail, CCing a public mailing list. However, I think this is overkill for these three lines, and Peter also entered himself here: https://cgit.kde.org/kde-dev-scripts.git/tree/relicensecheck.pl#n344 - stating that he in general is also fine with LGPL variants (see more details here: https://techbase.kde.org/Projects/KDE_Relicensing).

Which leaves us with what to do next: Someone with commit access (git push to KDE servers will work, it will not currently work for you) will push this to the server, closing this review request.

What I now did is:

• copy the link behind "Download Raw Diff" in the top right
• in the terminal, I cd'ed into kate.git, and typed: wget https://phabricator.kde.org/file/data/dvyfeaphmpg6urfgoip2/PHID-FILE-zw34xdqxdltejw3t56wf/D9590.diff
• I applied the patch, typing:

dh:~/kde/kf5/src/kde/applications/kate> patch -p1 < D9590.diff

patching file addons/konsole/kateconsole.cpp
Hunk #1 succeeded at 21 (offset -1 lines).
Hunk #2 succeeded at 250 (offset -10 lines).

• git diff shows me that the changes look good
• then I type: git add addons/konsole/kateconsole.cpp
• git status now lists that I added the changes to staging
• Now I locally commit with: `git commit --author="Michael Heidelbach <email@address.abc>"

Unfortunately, this is how far I get, since your email address is invisible to me. So either I commit in my name (which would not be the correct author) - or I wait until you tell me (e.g. to dhaumann@kde.org).

Hope that helps so far :-)

Btw, I just posed on kde-core-devel to find out how to get email addressed on phabricator: https://marc.info/?l=kde-core-devel&m=151492773414285&w=2

Hope that helps so far :-)

Of course it does! Thank you very much.
I did not intend to hide my email. A quick scan over my profile settings revealed nothing so I leave that for later. And my email is ...
ottwolt@gmail.com

Hmm... This patch causes my terminal to just flash up and then disappear immediately...

It's the same for me - I'll revert for now, since this seems to entirely break Kate's terminal toolview.

Reverted, see https://commits.kde.org/kate/6303cc4a0788d84190db74951fc2db639456a8bc

Now why does this work in Dolphin, but not in Kate? :-)

With KDevelop:
make clean
make
make install

Debug current launch => works as expected
Execute current launch => panel does not show, clicking buttons activates them but respective panel does not show
Debug current launch again => does not work: same as Execute current launch

What's happening here?

$ git checkout origin/Applications/17.12
In KDevelop:
clean
build
install
run

In Kate:
Trying to show panels ends in this

Messed setup or deeper problem?
Qt 5.10.0-2.2

I have the same issue. I saw things like this before, not sure whether Kate really picks up the correct plugins. Maybe it's messed up with the plugins from the stable system.

@michaelh The button issue was introduced in KWidgetAddons and is meanwhile fixed: https://phabricator.kde.org/D9884

Unfortunately, this bug was released in KDE Frameworks 5.42.

So in this review request, we can focus on why Konsole crashed for me and @sars .

After update to KF 5.42
In KDevelop:
First run => OK
Consecutive runs =>

 cd /tmp/test
^@

Warning: Program '/bin/bash' crashed.

First run in debugger => OK
Consecutive runs => bash crash
BUT:
Setting a breakpoint at equivalent of line 252 => runs always OK, even if breakpoint is disabled after first hit.

Please remove the accepted flag

Another comment: Could you remove trailing spaces in your changes lines?

Btw, I just tested: If I remove slotSync() in KateConsole::loadConsoleIfNeeded() then it works. So this indeed is a timing issue we face. The ^C in this case then immediately tells the embedded konsole to quit, which is wrong of course. I have not debugged further, but understanding what's going on would certainly help to get the correct patch.

@hindenburg Would it make sense to extend the TerminalInterface somehow to clear input data of current line or send this ^C as done by Dolphin and proposed in this patch?

Back in 2012 Konsole implemented this in 693cecbae73896f03a1c47f4c41a0fa30759d917 - Konsole keeps a list of valid programs that accept Ctrl+C

Having this in TerminalInterface seems reasonable