Add usefull functions to GDriveUrl.
ClosedPublic

Authored by barchiesi on Mar 26 2019, 5:06 PM.

Details

Summary

I added the following usefull functions in GDriveUrl:

  • QString filename() const
  • bool isTrashDir() const
  • bool inTrash() const
  • QUrl url() const
  • GDriveUrl parent() const
Test Plan

Tests in UrlTest have been updated according to the new functionalities.

Diff Detail

Repository
R219 KIO GDrive
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
barchiesi requested review of this revision.Mar 26 2019, 5:06 PM
barchiesi created this revision.

Bonus point if you add a test case in urltest.cpp ;)

src/gdriveurl.cpp
52

I know this is the old code, but while we are at it we could port it to m_components.at(1).

barchiesi updated this revision to Diff 56802.Apr 23 2019, 8:48 AM

Added test case, is the test sufficient?

barchiesi updated this revision to Diff 57257.Apr 30 2019, 12:25 PM
barchiesi retitled this revision from Add trash related to GDriveUrl to Add usefull functions to GDriveUrl and move some gdriveurl related logic there..
barchiesi edited the summary of this revision. (Show Details)
barchiesi edited the test plan for this revision. (Show Details)

Added more functions and refactored KIOGDrive.

barchiesi edited the summary of this revision. (Show Details)Apr 30 2019, 1:00 PM

Please move the kio_gdrive.cpp refactoring to another commit, depending on this one.

barchiesi updated this revision to Diff 57329.EditedMay 1 2019, 5:07 PM

I'm still new to phabricator, I hope this is what you meant.

No, I meant to split this patch into 2 commits (resulting in 2 different Phabricator revisions), to make it easier to review. One commit for the new GDriveUrl api and the other one for the code refactoring in kio_gdrive.cpp (which got much bigger than it was).

barchiesi updated this revision to Diff 57330.EditedMay 1 2019, 5:16 PM

Got it.

EDIT: uh, no that didn't work

barchiesi updated this revision to Diff 57331.May 1 2019, 5:19 PM
barchiesi retitled this revision from Add usefull functions to GDriveUrl and move some gdriveurl related logic there. to Add usefull functions to GDriveUrl..
barchiesi edited the summary of this revision. (Show Details)

Maybe this time

barchiesi updated this revision to Diff 57334.May 1 2019, 5:28 PM

Add commit to revision.

src/gdriveurl.h
37

How about isTrashed, to keep consistency with the other boolean methods?

41

It seems to me you plan to use this method in D20941 only as if (parent().isAccountRoot()), instead of if (components.length() == 2).

Do you foresee other usages for parent() ? If not, I think we could use another API instead, something like this:

bool GDriveUrl::isTopLevel() const
{
    return m_components.length() == 2;
}

Or with a similar name. This way we wouldn't need the new (private) constructor.

barchiesi updated this revision to Diff 58310.May 19 2019, 4:54 PM

You are correct, fixed those issues

autotests/urltest.cpp
50

You probably meant expectedIsTopLevel here? ;)

barchiesi updated this revision to Diff 58324.May 19 2019, 9:42 PM

Yes I did

elvisangelaccio accepted this revision.May 25 2019, 5:04 PM
This revision is now accepted and ready to land.May 25 2019, 5:04 PM
This revision was automatically updated to reflect the committed changes.