<table><tr><td style="">dhaumann accepted this revision.<br />dhaumann added a comment.<br />This revision is now accepted and ready to land.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D8708">View Revision</a></tr></table><br /><div><div><p>Feel free to push this for 5.52 (you'd have to adapt the <span class="phabricator-remarkup-mention-unknown">@since</span> 5.53 again).</p>
<p>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.</p>
<p>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! :-)</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D8708#inline-88508">View Inline</a><span style="color: #4b4d51; font-weight: bold;">abstractannotationitemdelegate.h:66</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #74777d">/**</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * The view where the annotation is shown</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> */</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Is the view pointer always valid when the style option is passed as argument? If so, I suggest to add this as comment to avoid unnecessary if() calls / error handling etc...</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D8708#inline-88510">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kateannotationitemdelegate.h:20</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#ifndef __KATE_ANNOTATIONITEMDELEGATE_H__</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#define __KATE_ANNOTATIONITEMDELEGATE_H__</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">All keywords starting with __ are reserved, I suggest to remove __ at the beginning and at the end.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R39 KTextEditor</div></div></div><br /><div><strong>BRANCH</strong><div><div>addAnnotationItemDelegate</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D8708">https://phabricator.kde.org/D8708</a></div></div><br /><div><strong>To: </strong>kossebau, Kate, KDevelop, dhaumann<br /><strong>Cc: </strong>cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns, demsking, sars<br /></div>