After considering "Developer," "Developer (retired)" and "Code contributor" I decided to wait until next release - if I manage to keep contributing :)

My bad about kdesu, it's not being used at all here.

In D4738#92533, @asensi wrote:

Using Kubuntu 16.10: that patch works if the password has been typed recently, if it's not that way, if you already had an opened Kate, then you are asked for a password but after typing it nothing happens.
I'm available to perform more tests. Thanks.

Yes, I meant that it won't benefit too many people as long as the default viewer isn't the lister.

In D4725#90687, @abika wrote:

I also don't get it: You pointed out the flaw with the "kdesu" config setting yourself and fixed it. At the same time, a security flaw with the very same impact is not that important?

If I understand correctly, if the distributor has kdesu installed under /usr/local/bin then find_program() will pick up that path and hardcode it for everybody. Even if we assume that distributors generally have kde-cli-tools installed, this will make build results depend on the state of a particular machine. Doesn't really sound desirable, I'd rather stick with an explicit override for non-standard locations.

Fine with me, it sounds like duplicating locked state is indeed undesired with that use case.

Frankly, we explicitly tested that "locked" setting was being duplicated when this was implemented. I have no idea however what users' expectation here might be. I managed to understand what this feature does but not what it is good for. What are people using "Lock Tab" for?

Right, I forgot to remove a piece of unused code in KrServices, done that now.

Forgot to reply:

Toni, thank you for pointing this out! The issue here is pretty obvious: sudoedit waits for the editor to exit, then it copies the temporary file back. Yet in case of an existing Kate session the new instance will delegate the editing to it and exit - so sudoedit thinks that the editor is already done when the file just opened up.

I updated this patch for changes in D4725 and improved the error message while at it. Now it should be obvious to the user where kdesu is expected to be.

Heh, the important part of this patch got lost in merges. Fixed it again.

I updated this patch for changes in D4725 and improved the error message while at it. So now it should be obvious to the user where kdesu is supposed to be.

One more small update - rather than have a special method for kdesu in KrServices, I added a general-purpose KrServices::isExecutable() method. The actual path is available everywhere anyway.

Uploaded a new patch, now there is an explicit build variable called KDESU_PATH that can be overridden - I added this to the documentation. Also, there is a new KrServices::kdesuPathName() method which can be used to access that path, no more messy logic. I didn't update the error message in krslots.cpp yet - it isn't really correct already, it's much easier to fix it in D4734.

There is more information in a presentation by the same Martin Gräßlin. I was mostly interested in the details of his proof of concept but the other information turned out very useful as well. So what we are talking about is an open secret and X11 is inherently insecure (e.g. even with the changes here a key logger can still steal the password entered into kdesu). Things will hopefully get better with Wayland, eventually. But from the look of it, there is little point in keeping this review secret.

Strike the "ugly" part - you don't have to install the ssh-askpass package, there are alternatives. On Ubuntu I see ksshaskpass, ssh-askpass-gnome and a bunch of others. I installed ssh-askpass-gnome and now that command presents a pretty decent password prompt.

Martin, thank you for the pointer to https://blog.martin-graesslin.com/blog/2017/02/editing-files-as-root/, this is very useful information (I'm not really familiar with Linux and X11 security architecture). I created D4738 to address editing files as root. However, the more important implication here is that running Krusader as root is clearly a bad idea. At the very least, this functionality deserves a huge warning pointing out that it isn't safe. But I'd also like to make it unnecessary for the most important scenarios. As discussed on the mailing list, copying and moving files to write protected directories should offer "Execute as root" as another option beyond "Retry."

The bogus whitespace in the patch should be gone now. Looks like I should stop copying patches from the terminal, it messed up whitespace somehow.

Too bad, I meant to push this one myself - I have the necessary privileges now :)

Previous patch had unrelated changes, removed these now.

Alex, thank you, I missed that indeed. I thought that isValid() would cover that case already but apparently it doesn't. I updated the patch and everything seems to work correctly now.

I reverted to the original approach and applied suggested changes. As far as locked tabs are concerned, everything seems to work correctly for me. Could it be that the issue there was being caused by the extra ListPanel::start() call?

Sure, that would be easy - but there is little point as long as Lister isn't the default.

Well, I'm stuck. The way I see it, the searchText method of KTextEditor::Document merely gives you the search results but doesn't affect the UI. There doesn't seem to be a way to manipulate the search bar, in particular the method KateSearchBar::setSearchPattern() is only called by unit tests.

KParts::TextExtension would have been too easy. At least on my system both viewer and editor are using KTextEditor which currently doesn't appear to implement KParts::TextExtension. It implements its own KTextEditor::Document interface instead which also allows searching but requires introducing a new dependency (not to mention fallback code). I am trying to make it work.

I now implemented "proper" settings duplication as suggested. Frankly, I'm not convinced that this approach is better. It introduces quite a bit of code which is almost the same as what was there before but not quite. Also, is there really any setting where it makes sense to restore it on startup but not to duplicate when you duplicate the tab? The only one I left out here is the tab history but even that one should arguably better be duplicated.

Well, grep is a command line utility - usability isn't a huge concern with those. I think that grep's default behavior wasn't motivated by expectations of human beings (people rarely expect case-sensitive search) but rather performance concerns (back when grep was conceived the performance overhead of lowercasing strings was considerable). As far as GUI tools go, I don't know any where case-sensitive search would be the default, and I wouldn't expect it either. In fact, I filed this bug after wondering why my search didn't turn up any results. So IMHO remembering these settings across restarts (or rather the selected search profile) would be nice to have but not a replacement for sane defaults.

Yes, I used a temporary config group because the code in PanelManager::slotRecreatePanels() does it like that - this might not be the cleanest approach however. I will add a ListPanel::duplicateSettings(ListPanel*) method instead that restores a different set than ListPanel::restoreSettings(KConfigGroup). The danger here is however that these two methods get out of sync as new settings are added - some new settings will be restored by the latter but not the former even where both would make sense.

This mockup is great but implementation will certainly take time. Unless somebody plans to work on this really soon, maybe a simpler solution is worth considering: when opening the viewer from the file search with non-empty text, do a search for this text in the viewer automatically. So the first result line should be selected immediately and pressing F3 should bring up the other matches. That's what Total Commander does. These proposals aren't mutually exclusive of course.

In D3994#77448, @abika wrote:

I'm actually unsure if there is any benefit for a "real" async refresh. The panel currently depends on it and the Qt event loop prevents an UI freeze during a long refresh.