Don't compare string with empty string
ClosedPublic

Authored by ognarb on May 15 2020, 9:03 AM.

Details

Summary

Instead compare string length with an integer

Test Plan

Compile && run

Diff Detail

Repository
R108 KWin
Branch
string-compare (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 26937
Build 26955: arc lint + arc unit
ognarb created this revision.May 15 2020, 9:03 AM
Restricted Application added a project: KWin. · View Herald TranscriptMay 15 2020, 9:03 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
ognarb requested review of this revision.May 15 2020, 9:03 AM
broulik added inline comments.
kcmkwin/kwinrules/package/contents/ui/FileDialogLoader.qml
39

Why not just !root.lastFolder? or even

root.lastFolder || shortcuts.home
ognarb planned changes to this revision.May 15 2020, 9:37 AM
ognarb added inline comments.
kcmkwin/kwinrules/package/contents/ui/FileDialogLoader.qml
39

I didn't know that an empty string in js was returning false.

broulik added inline comments.May 15 2020, 9:38 AM
kcmkwin/kwinrules/package/contents/ui/FileDialogLoader.qml
39

It's not, but the operators in JS work a little differently. They don't cast to bool.

ognarb updated this revision to Diff 82914.May 15 2020, 9:58 AM
  • reflect comment
iasensio added inline comments.May 15 2020, 10:00 AM
kcmkwin/kwinrules/package/contents/ui/FileDialogLoader.qml
39

I also didn't know that about JS. Python operators also behave that way. Nice!

ognarb updated this revision to Diff 82915.May 15 2020, 10:04 AM

Found another one

All the changes seem nice but this one. I've not yet figure out why

kcmkwin/kwinrules/package/contents/ui/FileDialogLoader.qml
49

This seems to break importing files for me.
Checking lenght > 0 also didn't work, just old if (fileUrl != "").

broulik added inline comments.May 15 2020, 2:54 PM
kcmkwin/kwinrules/package/contents/ui/FileDialogLoader.qml
39

There's also ?? operator in Qt 5.15 which is the proper Null coalescing operator

49

If fileUrl is a QUrl, then that can lead to funky results as url basic type isn't astring. So fileUrl != "" it is (not the strict operator).. or fileUrl.toString() !== ""

iasensio requested changes to this revision.May 29 2020, 2:54 PM

Ping @ognarb? I think it's just the fileUrl thing that's missing.

This revision now requires changes to proceed.May 29 2020, 2:54 PM

sorry forgot about it, let me finish it now

No rush, I just wanted to use Requested Changes for the first and last time here at Phab :D

ognarb updated this revision to Diff 83174.May 29 2020, 6:50 PM

Revert fileUrl change

ognarb marked 2 inline comments as done.May 29 2020, 6:51 PM
ognarb updated this revision to Diff 83176.May 29 2020, 8:15 PM
ognarb marked an inline comment as done.

Rebase

iasensio accepted this revision.May 29 2020, 8:44 PM

LGTM!

This revision is now accepted and ready to land.May 29 2020, 8:44 PM
ognarb closed this revision.May 29 2020, 8:50 PM