kfontinst: Port to QTemporaryDir
ClosedPublic

Authored by volkov on May 24 2016, 4:46 PM.

Details

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
volkov updated this revision to Diff 3970.May 24 2016, 4:46 PM
volkov retitled this revision from to kfontinst: Port to QTemporaryDir.
volkov updated this object.
volkov edited the test plan for this revision. (Show Details)
volkov added a reviewer: Plasma.
volkov set the repository for this revision to R119 Plasma Desktop.
Restricted Application added a project: Plasma. · View Herald TranscriptMay 24 2016, 4:46 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
apol added a subscriber: apol.May 24 2016, 6:17 PM

I'd say it's generally better to use QDir::filePath there. it will be easier to read.

kcms/kfontinst/thumbnail/FontThumbnail.cpp
96 ↗(On Diff #3970)

can't you use filePath there?

100 ↗(On Diff #3970)

filePath?

Unfortunately QTemporaryDir doesn't have filePath() method. But it's a good idea to add it :)

apol added a comment.May 25 2016, 10:47 AM

Ugh, I was convinced it would exist! My apologies for not checking.

For now, can you at least just change from "/" -> QLatin1Char('/')?
Also if you can look into adding the API to Qt that would be great as well.

Other than that, +1

Add QTemporaryDir::filePath(): https://codereview.qt-project.org/#/c/160366/

"/" are placed before macros that expand to string literals
it's a string literal concatenation, there is no need to change them to QLatin1Char('/')

apol accepted this revision.May 25 2016, 12:21 PM
apol added a reviewer: apol.
This revision is now accepted and ready to land.May 25 2016, 12:21 PM
This revision was automatically updated to reflect the committed changes.