ping? :)

In D6473#124300, @dhaumann wrote:

So the question is: Why is the cursor not updated? If we can answer this, we may be able to either find that this patch is correct, or that the root of the real issue lies somewhere else...

In D6473#124287, @kfunk wrote:

Could you add a test in e.g. autotests/src/searchbar_test.cpp?

ping?

In D6473#121136, @ltoscano wrote:

If the review is accepted and you want to close the bug, replace

Fixes bug 381080
https://bugs.kde.org/show_bug.cgi?id=381080

with simply

BUG: 381080

Will recheck and create a new one....

In D5110#96623, @kkofler wrote:

Unfortunately, I don't think your approach at taking the common part of source and destination as the directory and file name is going to work in all cases:

In D5110#96624, @kkofler wrote:

And also, you are again trying to fix 2 bugs in one review request. Please submit one review per issue you fix.

In D4947#95893, @cullmann wrote:

Sorry for late response, thanks for ping!

ping?

In D5075#95809, @kkofler wrote:

Well, my first nitpick is that you are fixing 2 separate bugs and so should be submitting 2 separate code reviews.

In D4947#94562, @dhaumann wrote:

Reading this API, I still have some general thoughts:
This is the first time we expose the concept of "visible lines". So far, this only exists internally in katetextfolding.h/cpp. By itself, this is fine, but given we do not expose folding information so far, I think the API is not complete.
If we already provide infos about visible first / last visible line in a view, the complete API should also contain functions like isLineVisible(int line). But here, we have a problem, since the fistVisibleLine() and lastVisibleLine() are not the text folding lines, instead, its the visually visible drawn line on screen. Here, the term "visible" is used in two different contexts: once for screen visibility, and once for folding information. Internally, KTextEditor uses terms viewLine or displayedLine for this. Maybe displayedLine is the best here?

Any comments? Are we lacking more API to make it feel complete?

According to @cullmann's and @dhaumann's feedback created the enum LineType and also renamed firstVisibleLine() and lastVisibleLine() to firstDisplayedLine() and lastDisplayedLine()

in firstVisibleLine() and lastVisibleLine() provide the option for real lines (in cases of folded text).

Implemented additional functions as non-virtuals, according to @cullmann's suggestion.

ping?

Diff updated. Hope I addressed all issues :\

Thank you all for your feedback. I'll update soon....

In D4947#92995, @anthonyfieroni wrote:

Milian had in mind that you can take charge of Kompare and implement your ideas rather than release new app. :)

In D4947#92889, @mwolff wrote:

You will have to create a separate interface for this, similar to what we have done in the past, with a TODO note that this should be merged in time for KF6.

You mean an interface similar to TextHintInterface, CodeCompletionInterface, ConfigInterface, AnnotationViewInterface, etc or something else? If something else, could you please provide an example?

In D4940#92656, @dhaumann wrote:

Looks good, please commit. Do you have commit rights, or should I commit for you?

In D4940#92646, @dhaumann wrote:

In general, looks good, but please also extend the documentat in src/include/configinterface.h, and add an updated patch here again :-)

Added documentation for the new options in configinterface.h

In D4491#89982, @anthonyfieroni wrote:

ElideMiddle on basic regex match for path

In D4800#89954, @subdiff wrote:

Removing the undefineds increases the margins between the lines again. @anthonyfieroni had another fix for the elide a few days ago by setting the maximumLineCount property. He hadn't yet pushed it though. I would go for his solution if it retains the margins. You could try if it works for you as well. It's the last revision of D4491.

If you agree, I believe this should be committed to 5.9 branch and not wait until 5.10.

In D4699#89262, @cullmann wrote:

Don't we need to honor "m_reloading" to avoid to do this on document reloads?

Honor "m_reloading" to avoid to avoid reseting the file type on document reloads as suggested by @cullmann

In D4699#89262, @cullmann wrote:

Don't we need to honor "m_reloading" to avoid to do this on document reloads?

In D4699#89179, @dhaumann wrote:

Looks reasonable to me as well... I don't see any negative side effects, so should be fine from my side.
@cullmann Can you confirm?

In D4696#89174, @dhaumann wrote:

Indeed the getter case is missing, please commit. (Can you commit yourself, or should I take care of this?)

1. I abandoned the 3rd issue I mention in the summary about the line scrollPos(max, max.column(), calledExternally); as I could neither verify nor disprove if this was intended or not.

Seems to work.

BTW: I don't know how I did it but while testing it, at some point configuration didn't open at all and plasma produced the following error

In D4153#77549, @jgrulich wrote:

Icons problem solved

kcm.patch14 KBDownload

. Can you please test this patch?

In D4153#77525, @jgrulich wrote:

I'm trying to get rid of Plasma QML components so it uses same colors which are used by the rest of application.

Previous diff was incomplete.

Updated diff as per emmanuelp's comments

Make sure member variable declaration in the .h file is in the same order as the initializations in the constructor