Complete TODO : Show more user friendly labels

[nishanthkarthik]

This PR completes the task at

zeal/src/libs/ui/docsetsdialog.cpp

Line 521 in 3bdb8ee

// TODO: Show more user friendly labels, like "5 hours ago"

[trollixx]

Linux build failed because QDateTime::toSecsSinceEpoch() was added in Qt 5.8. I'll bump the requirement for 0.7.x.

[nishanthkarthik]

I'll try replacing it with a backwards compatible method later today

[trollixx]

No need, the plan is to switch to newer Qt in 0.7 anyway.

[trollixx]

All these should just be constant strings in an anonymous namespace in the source file.

[trollixx]

Maybe just make the whole thing a static method?

[nishanthkarthik]

@trollixx Done (I also fixed some clang tidy warnings). Please let me know if there's anything else :)

[trollixx]

This pull request introduces 3 alerts when merging 7d4e43c into d9bb3c5 - view on LGTM.com

new alerts:

• 3 for Comparison result is always the same

---

Comment posted by LGTM.com

[nishanthkarthik]

Hey, on the three LGTM warnings of comparing with known value at compile time, how do I transform this snippet to a compile time branch?

const int X = 3;

int a = 0;
if (m && ++a < X) 
    f();
if (n && ++a<X)
    g();

Here, the increment occurs only if m or n is true. If not, this has to go to the next branch until a = X. I am not sure how to optimize this out in compile time as we have no idea if first condition is true or not until runtime.

I think this is a false alert since we depend on the boolean expression short-circuiting to increment the counter.

[trollixx]

This is dangerous, see static initialization order fiasco and bullet point 3 here.

[trollixx]

Always try to return ASAP, and avoid unnecessary conditional blocks of code.

if (delta == 0) {
    return ZERO_INTERVAL_STRING;
}

QStingList list;
...

[trollixx]

Also, it's better to avoid making translatable strings constant. Just put them inside tr() inline.

[nishanthkarthik]

I'm leaving plural forms of units with a (s). Once we apply translation to this project, qt should automatically detect the quantity and append s.

http://doc.qt.io/qt-5/i18n-source-translation.html#handling-plurals

[trollixx]

I see rightfully LGTM complains about always true conditions. Maybe rewrite the whole thing as a loop?

[trollixx]

I am still thinking on how to implement this in a cleaner way. The current proposal is a bit too heavy for the task it accomplishes. Perhaps, worth checking if we can use std::chrono for the duration manipulations...

[nishanthkarthik]

Hey @trollixx

std::chrono::duration looks like a viable alternative on the computation part where we split seconds to multiple duration units. It doesn't look like there is a way in this lib to print it in human readable format.

If you tell me which part of my snippet seems heavy, I can try replacing the same with chrono equivalent.

[trollixx]

Right, we still need to print generate the text ourselves, but I think std::chrono should be used for manipulating the duration. That should reduce the amount of code required as well.

[nishanthkarthik]

Sure, let me update this PR with chrono