simplify isProfileDeletable
ClosedPublic

Authored by tcanabrava on Jun 13 2018, 1:54 PM.

Details

Summary

Maintenability ++

Diff Detail

Repository
R319 Konsole
Branch
smallPartsLogicSimplification
Lint
No Linters Available
Unit
No Unit Test Coverage
tcanabrava created this revision.Jun 13 2018, 1:54 PM
Restricted Application added a project: Konsole. · View Herald TranscriptJun 13 2018, 1:54 PM
Restricted Application added a subscriber: konsole-devel. · View Herald Transcript
tcanabrava requested review of this revision.Jun 13 2018, 1:54 PM

Thanks, we put {} around all "if" for consistency and following KDE's style

Thanks, we put {} around all "if" for consistency and following KDE's style

I work on kde for 10 years and I *still* forget those silly braces.
ahah. updating.

tcanabrava updated this revision to Diff 36102.Jun 13 2018, 3:00 PM
  • Add missing braces
ahmadsamir added inline comments.
src/settings/ProfileSettings.cpp
385

I think that if the profile pointer is null, it should return false, otherwise you'd be deleting some unknown .profile.

390

Same thing here, should be false (if the .profile doesn't exist to begin with it can't be deleted).

tcanabrava added inline comments.Jun 13 2018, 8:09 PM
src/settings/ProfileSettings.cpp
385

Considering that the code works right now, it’s returning true when we don’t have a profile. While I agree with your reasoning I tried to port the code without changing its behavior.

390

Same answer, this is the same behavior as before. If the profile doesn’t exists or it’s not writable, we returned trues

ahmadsamir added inline comments.Jun 14 2018, 8:09 AM
src/settings/ProfileSettings.cpp
385

Yeah, well, this is still a good time to change it, if the change makes more sense.

tcanabrava marked 4 inline comments as done.
  • Fix old logic: don't permit to delete inexistent profiles
hindenburg accepted this revision.Jun 14 2018, 12:31 PM
This revision is now accepted and ready to land.Jun 14 2018, 12:31 PM
This revision was automatically updated to reflect the committed changes.