D23674: Use QString::count(QChar)+1 over QString::splitRef().length()

Friedrich W. H. Kossebau noreply at phabricator.kde.org
Tue Sep 10 12:49:40 BST 2019


kossebau added a comment.


  In D23674#528430 <https://phabricator.kde.org/D23674#528430>, @cullmann wrote:
  
  > Hmm, is lastLineLenght correct? Must one not decrement by one?
  
  
  Yes, someone missed in their brain to properly turn `- (x + 1)` into `- x - 1`, just happened again with me when redoing the code in brain on first try, darn.
  As main idea of logic is: "fulllength" - "length of all chars up to last linebreak", so "length"  - "lastindex + 1"
  
  So example:
  
    012345 <- pos
    CCCNCC <- pasted text, C: char, N: newline
    ->
    size: 6
    last N pos: 3
    last line length: 2
    
    const int lastLineLength = 6 - (3 + 1) -> 2, 
  
  Adding brackets now, so the original idea is visible

INLINE COMMENTS

> apol wrote in normalvimode.cpp:3848
> oh I see, how about just using splitRef? This way we don't have to allocate all the strings but the code still is more readable.

Still one allocation done for the vector ;)

Not sure about readibility. While the mistake done in formula spoils my point, I find the new code more readable, as the splitref would make me expect more sections are used, while it is just a hack to get the precalculated length of the last section, with info about all other sections discarded.
New code has less surprises to me. but two more error-prone calculations. Guess this is up to opinions, so up to maintainer :)

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D23674

To: kossebau, #kate
Cc: cullmann, apol, kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, ngraham, bruns, demsking, sars, dhaumann
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwrite-devel/attachments/20190910/0e9072ba/attachment.html>


More information about the KWrite-Devel mailing list