Add Kompare as compare application with KIO support
Needs RevisionPublic

Authored by abika on Sun, Oct 27, 5:24 PM.

Details

Reviewers
pino
Group Reviewers
Krusader
Summary

It can use the original KIO URLs. No need for using temporary local files.
And remove the obsolete space character check for file paths for KDiff3.

CCBUG: 401064

Test Plan
  • Tested opening two smb:// remote files in Kompare with "Compare by content" feature
  • Tested opeing two local files with spaces in filename and folder path in KDiff3
  • Tested opeing two smb:// remote files with spaces in filename and folder path in KDiff3

Diff Detail

Repository
R167 Krusader
Branch
my-open-kompare-with-kio-urls-arc
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18673
Build 18691: arc lint + arc unit
abika requested review of this revision.Sun, Oct 27, 5:24 PM
abika created this revision.
abika added a subscriber: Krusader.
yurchor added a subscriber: yurchor.Sat, Nov 2, 7:02 PM

Tested to work as expected. Thanks for your work.

+1

pino requested changes to this revision.Sat, Nov 2, 10:48 PM
pino added a subscriber: pino.
pino added inline comments.
krusader/krslots.cpp
98

static please

218–219

why not QList::contains()?

also, the checks for spaces in url1 and url2 are removed, which seems another unrelated change

228–231

please avoid unrelated changes

240–243

ditto

This revision now requires changes to proceed.Sat, Nov 2, 10:48 PM
abika updated this revision to Diff 69535.Sun, Nov 10, 4:26 PM
  • fixup! Add Kompare as compare application with KIO support
abika marked 2 inline comments as done.Sun, Nov 10, 4:31 PM
abika added inline comments.
krusader/krslots.cpp
218–219

why not QList::contains()?

eh, of course.

also, the checks for spaces in url1 and url2 are removed, which seems another unrelated change

See the Git comment/Differential summary. It is not necessary anymore.

228–231

Although this is unrelated, these are safe changes nearby which make the code a bit cleaner. -> "Boy Scout rule"

pino requested changes to this revision.Sun, Nov 10, 5:26 PM

Mostly OK from my POV, just please remove the unrelated changes to the patch (i.e. the brackets additions in two places).
I have no idea what "Boy Scout rule" is supposed to mean, however adding unrelated changes makes history reading harder, especially when wanting to check why certain changes were done. Again, material for a different patch than this.

This revision now requires changes to proceed.Sun, Nov 10, 5:26 PM
In D24987#560752, @pino wrote:

I have no idea what "Boy Scout rule" is supposed to mean

The Boy Scout Rule can be summarized as: Leave your code better than you found it.