D15797: [kdev-clazy] : use canonical paths

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Hi @rjvbb
  
  I just came across this logic (for reasons) and wonder if the logic does what you intended with the patch:
  given the `const auto canonicalPathToCheck = checkPathIsFile ? pathInfo.canonicalFilePath() : QStringLiteral("");`, when `checkPathIsFile` is `false`, `canonicalPathToCheck` is an empty string.
  
  Which makes those two other logic using that variable behave bogus:
  a) `(canonicalPathToCheck == projectCanonicalRootPath)` will never be true, as in case of being a path to check the project root path is compared against an empty string and if a file it will be false anyway
  b) `file.startsWith(canonicalPathToCheck)` will always be `true`, so all files are added to `m_sources` in case we have a path to check
  
  So my questions:
  a) should `canonicalPathToCheck` not always be `pathInfo.canonicalFilePath()`?
  b) is the check of both `file.startsWith(m_checkPath) || file.startsWith(canonicalPathToCheck)`the intended check?
  
  I assume 2x yes, but given this patch was written by one person and reviewed by another, I am not sure if my first coffee was too weak ;)

REPOSITORY
  R32 KDevelop

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

To: rjvbb, #kdevelop, antonanikin, mwolff
Cc: kossebau, mwolff, antonanikin, kdevelop-devel, hmitonneau, christiant, glebaccon, domson, antismap, iodelay, alexeymin, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20200109/6342558f/attachment.html>