I added the following usefull functions in GDriveUrl:
- QString filename() const
- bool isTrashDir() const
- bool inTrash() const
- QUrl url() const
- GDriveUrl parent() const
| elvisangelaccio |
I added the following usefull functions in GDriveUrl:
Tests in UrlTest have been updated according to the new functionalities.
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
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). | |
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).
| 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. | |
| autotests/urltest.cpp | ||
|---|---|---|
| 50 | You probably meant expectedIsTopLevel here? ;) | |