Example for custom rendering in KDevelop as currently proposed with the matching KDevelop patch D8709:

I commented on some things - I did not try this, though.

What I would like to see is a comment // KF6: Merge KTextEditor::AnnotationViewInterfaceV2 into KTextEditor::AnnotationViewInterface (kossebau).
For me, this comment is really important, since this tells you that you will in 2-3 years (when Qt6 arrives) work on this and merge it down: Since there is only you (KDevelop) who is using this interface, so you have to maintain it ;)

Would you go over my comments and provide an updated version? Not all comments are real issues.

In D8708#326590, @dhaumann wrote:

What I would like to see is a comment // KF6: Merge KTextEditor::AnnotationViewInterfaceV2 into KTextEditor::AnnotationViewInterface (kossebau).
For me, this comment is really important, since this tells you that you will in 2-3 years (when Qt6 arrives) work on this and merge it down: Since there is only you (KDevelop) who is using this interface, so you have to maintain it ;)

Forgot that in the previous update, now added. Curious though why you do not expect Kate to offer users some similar annotation display experience one day if working on versioned text documents ;)

Thanks everyone who so far looked at this, especially @dhaumann for detailed comments.

I am tempted to interpret the lack of further comments, especially the lack of principal objections as an implicit +1 on this patch as it is now :)

While I still am slightly unsure about parts of the API, given no better ideas there I would leave it now to real life experience gathering. And then polishing things as found needed in any next ABI/API version.

To keep things moving, I will go and merge this patch, once KF 5.51 has been tagged the upcoming WE, so there are 4 weeks to give the new API and also the default implementation some testing and considerations before getting published with 5.52. Of course unless there are objections raised now.

As we have a usecase for this (the extension in KDevelop) I am ok if that goes in, if the extension in KDevelop is going in, too, as consumer.
We can still fix minor issues in the KF6 iteration, they will never be found if it is not there and used.

Hm, given the size of the patch, and given it introduces new public API we cannot easily change, I think a silent +1 since noone reacts is not good enough.

That said, I will not have time for another review until Oct 14th - so I would appreciate another review by someone else, if possible.

If Christoph accepts, I am fine with this.

In D8708#337408, @dhaumann wrote:

Hm, given the size of the patch, and given it introduces new public API we cannot easily change, I think a silent +1 since noone reacts is not good enough.

I agree that the amount of review done is not good enough. I would have liked some more feedback on both the patches, but most people sadly never got further than "nice feature, looking forward". I was very glad when you did your initial review :)

On the other hand I have reiterated these patches a few times while they waited on phab for review, also tried multiple approaches last year before uploading the delegate one. So while my perfectionism-self still sees some issues in the new API, my pragmatism-self is actually pretty okay with it.
The additional things I would have liked in the API actually would mean some rework of the currently line-based rendering approach inside KateIconBorder, which due to review needs might have worsened the chance of getting at least the basic feature in at all. So I dumped that work and reduced the patch to what we have now.

That said, I will not have time for another review until Oct 14th - so I would appreciate another review by someone else, if possible.

Given this is API only needed for KDevelop 5.4, which might happen only begin of next year, I am okay with delaying another KF5 cycle/month, and schedule now to target 5.23 (merge after 5.22 has been tagged). If this actually means getting deep embracing reviews :) I was only going for action seeing that no-one seems to (have time to) care, before this feature is reaching it's first anniversary here on phabricator in November :( Not sure if I should have nagged more to get you to review this? But then I dislike being nagged myself for things which are pretty visible on phabricator, unless it's quick simple patches.

So, anyone thinking they will have time & interest to do a full review in the October weeks?

And I still think the annotation border feature would be a nice tool for people using kate/kwrite on versioned documents as well (given there is some VCS support to hook in). Is this really no use case Kate users have as well? -> https://frinring.wordpress.com/2018/09/09/from-code-to-related-bug-report-or-review-in-just-a-hover-click/ Or do people not use Kate to work on old codebases? :P
(a future planned addition will be integration to addressbooks/gravatar, so you could see the faces of the authors and/or have the option to directly contact them in case of need. All that could be done as one finds fancy thanks to the new delegate API introduced here, without having to twist something in KTextEditor)

@cullmann @dhaumann So, what to do? :) Do you think if we delay one more month you will find time to give this the wanted deeper review?

Or will this continue to (understandable) lack your motivation given you are so far not a consumer of this new API?

Hi,

not that I am a expert in delegates but from a glance the interface part looks ok.
I would have no issues with merging this.
It is an optional feature, we have one user.
We can still evolve this interface with some Vx version if bad issues are discovered after some usage in KDevelop and do a cleanup for KF6.

But we can wait for Dominik's opinion.

In D8708#344872, @dhaumann wrote:

Feel free to push this for 5.52 (you'd have to adapt the @since 5.53 again).

Thanks for review again :)

Would prefer waiting for 5.53, for some full possible period of testing by people running from master, which should be at least > number of people geting and trying cross-project/repo patches :)

I only have minor comments that should not hold back this patch. One thing that pops into my eyes is that there are still some "todo" comments, which even talk about cornercases / bugs. Since I think you know what you are doing, I leave it up to you to decide wither it's ok as is or whether there need to be more fixes.

Yes, the TODOs left are about non-critical things, like issues already in the old code (those about rendering group delimiters at the lines on the view top/bottom borders), old magic numbers which could see some reasoning, or some check which is practically not needed, but might be expected by some looking at patterns. Left as TODO remarks for my older self or someone else, in case they work on this, so the rough edges are documented and not silently in the code.

In any case, it was never my (and I think I can also safely say) nor any other's intention to hold this patch back. So yes, pinging more often is ok imo. Luckily, you are not a first-time contributor (who would be very much discouraged by our review delays), so I am sure / hope we'll see more patches from you in the future as well! :-)

Okay, white card for more poking about reviews happily received :)
Some other reason might also have been that I left the ""WIP" too long in the title, which might also have sent a wrong signal, when this was rather RFC on the working prototype/pre-production sample.