Wouldn't this have the effect of un-indexing most files? A quick check of my documents (text, word processing, excel, etc) reveals that none of them have the execute bit set. As-is, I think this would render Baloo mostly useless.
Files/folders should not be automatically indexed if the execute bit is unset. Downloads from most if not all popular browsers are not executable by default.
Here's how the checkmark idea looks. I prefer the plain wall since it fits in with the other icons better.
Thanks for the review. I will address your comments and hopefully have a patch ready in the next days. :)
Technically, this check isn't actually needed, though it does prevent the user from entering a path more than once. At first glance this looks like it should work:
For the benefit of others, here's what they look like now:
+1 for something rather than nothing. No comment on the technical aspect, but I'm adding more reviewers who can hopefully help un-wedge this patch.
since I'm still unsure about those things: the q2t function was not declared static, but was never exported. It is still safe to remove, right?
Looks good to me. A nice and simple way to drastically increase test coverage.
- don't change test strings for now
@zccrs Hi. Does D15405 fix things for you, so is this patch here no longer needed? If so, please close this review request by selecting and submitting the action "Abandon Revision", so it does no longer appear in our "Please review" list :)
In general this looks good, but I would like two changes:
- Do the conversion to QTest first, and leave out the change for unicode testing (e.g. Title1 -> Title €)
- Add a third column like "stringsuffix", and then add another test (row) for each format. QStringLiteral("Title1") then becomes QStringLiteral("Title1") + stringsuffix
Improve commit message
Patch looks ok but I'm surprised by the commit log. Doesn't this method also work for directories, even after the patch?
Updated with styling suggestions.
Very much +1 on the end goal!
Sat, Sep 22
So this doesn't completely fix https://bugs.kde.org/show_bug.cgi?id=375765? More is needed?