In D29886#676950, @ngraham wrote:

Thanks! However we are using GitLab now. Any chance you could close this and re-submit it as a merge request at https://invent.kde.org/graphics/gwenview/-/merge_requests/?

In D26264#584826, @ngraham wrote:

FWIW we're planning to remove KHotKeys in favor of KGlobalAccel: T2050: sunsetting KHotKeys. I'm not sure how much sense it makes to add features to it.

LGTM

LGTM

In D18967#418001, @cfeck wrote:

Does it work when Caps Lock is on?

Fix grammar mistake

May I ask for an another round of review (and if looks good merge?)
Thanks in advance for your time!

Thanks for the review, I have changed the raw char literals to QChar-s.

Fix review comment.

Updated revision to 8 from 7 as suggested in the review.

Any news on the reviewing of this one guys?

In D10555#207396, @cfeck wrote:

Thanks Marton, I will review in the coming days. There are some issues in the patch.

In D7847#151101, @ltoscano wrote:

I can't judge the patch, but if this is accepted remember to:

• use the authorship information of Arjun AK
• and possibly add a Co-Authored-By: line with the information of martonmiklos

Many thanks for the reviews, I have fixed them.

In D7847#151233, @ngraham wrote:

Thanks for those stylistic changes, @martonmiklos. However the title is still a bit long. How about something more like this:

"Don't block unmounting when terminal panel's cwd is the mountpoint"

@elvisangelaccio
After several trials I consider the "set unmounted directory to home" out of scope of this patch because I have not found any method which could reliably set the tabs with unmounted paths to home in time.
At the moment I can only think about some QTimer based hack, but I would not like to include it to this patch.

@elvisangelaccio
I have implemented the "storage teardown requested -> set the opened tabs to home if mounting path is open" method, but this approach cannot close the terminal in time when the teardown request came externally. (Umount requested from the indicator applet for e.g.). The tabs are set to the home directory properly both the terminal, but the unmount manager says that the mountpath is opened in an application.
I would rather merge the two approaches: asking the terminal to cd to home first and then loop over the tabs and set them to home if mountpath is opened.

In D7847#146547, @elvisangelaccio wrote:

I'm not sure I agree with this patch.
A standard behavior in other filemanagers when umounting a device is to redirect to the view from the device URL to the home URL.
So imho this patch is using the wrong approach: instead of the mainwindow telling the terminal panel to go to the home, it should be the mainwindow the one that changes its URL to the home.

This way:

1. We get an usable DolphinView after unmounting the device (matching the behavior of e.g. Nautilus)
2. We don't need the goHome() method in TerminalPanel, it would be automatic.

In D7367#143391, @cullmann wrote:

Should be fixed now:

commit fd79b6842c6f33a1aca4f500354ca2a69c52689e
Author: Christoph Cullmann <cullmann@kde.org>
Date: Wed Sep 6 08:08:49 2017 +0200

add missing reference files for tests, looks ok, I think

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

In D7367#143314, @cullmann wrote:

syntax-highlighting.git, on first glance not landed atm.

Can you commit on your own or should I push it for you Miklos?

In D7367#138232, @cullmann wrote:

Hmm, the diff still looks like it has the old names, perhaps the update went wrong somehow?

In D7367#138030, @cullmann wrote:

I am happy beside one thing I missed: I think the file should be called intelhex.xml, given its for the intel hex format only (the name is already fine, did miss that the filename is only hex).
Otherwise: very nice, thanks again for the contribution.

Okay so:

• License updated to MIT
• Added detection for invalid RecordType (05<)
• Changed formatting to use builtin formats
• Added autotest case with a sample file with two malformed lines and a HTML output
• I think folding tests are not acceptible for this file type.

In D7367#136687, @cullmann wrote:

Could you add a minimal example as unit test?

Sure, I have just seen that these xmls could be autotested.