Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-06 into lp:ubuntu-filemanager-app
Answer from comment on line 35: Yes, we can compare instances here because there will be just one instance for each type: one for DiskLocation, one for TrashLocation and another for SmbLocation. Locations are created by LocationsFactory creator class. See http://bazaar.launchpad.net/~ubuntu-filemanager-dev/ubuntu-filemanager-app/trunk/view/head:/src/plugin/folderlistmodel/locationsfactory.cpp#L46. There is a Location::type() which can be used here, I can change that if you think that it is really necessary to make it clear. Please let me know about that. -- https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-actions-06/+merge/265197 Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-06 into lp:ubuntu-filemanager-app
The proposal to merge lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-06 into lp:ubuntu-filemanager-app has been updated. Status: Approved = Merged For more details, see: https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-actions-06/+merge/265197 -- Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-06 into lp:ubuntu-filemanager-app
Review: Approve Thanks for the explanation, now it makes sense. I guess some clarification would be helpful there for people not so familiar with the details. I have three suggestions and you can decide whether to do one of them: - Add a comment to explain it - Use the type() - Create operator overload Location::operator==() to compare the objects and there do the pointer comparison. -- https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-actions-06/+merge/265197 Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-06 into lp:ubuntu-filemanager-app
The proposal to merge lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-06 into lp:ubuntu-filemanager-app has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-actions-06/+merge/265197 -- Your team Ubuntu File Manager Developers is subscribed to branch lp:ubuntu-filemanager-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-06 into lp:ubuntu-filemanager-app
Review: Needs Information
Comment on line 35.
Diff comments:
=== modified file 'src/plugin/folderlistmodel/filesystemaction.cpp'
--- src/plugin/folderlistmodel/filesystemaction.cpp 2015-07-18 21:50:43
+
+++ src/plugin/folderlistmodel/filesystemaction.cpp 2015-07-18 21:50:44
+
@@ -128,10 +128,36 @@
//it is not necessary to delete
auxAction = 0;
copyFile.clear();
-
-}
-
-
+sourceLocation = 0;
+targetLocation = 0;
+
+}
+
+/*!
+ * \brief FileSystemAction::Action::toggleLocation()
+ *
+ * It may be useful if there is a Undo Action to do a inverse Action
+ */
+void FileSystemAction::Action::toggleLocation()
+{
+Location * tmp = sourceLocation;
+sourceLocation = targetLocation;
+targetLocation = tmp;
+}
+
+/*!
+ * \brief FileSystemAction::Action::matchLocations
+ * \return true if sourceLocation is equal targetLocation
+ */
+bool FileSystemAction::Action::matchLocations() const
+{
+return sourceLocation == targetLocation;
I don't understand this. These are pointers to Location object. So it only
compares if they are the same instance? It does not actually compare the
objects are equal to each other. Is this the purpose?
+}
+
+bool FileSystemAction::Action::isRemote() const
+{
+return sourceLocation-isRemote() || targetLocation-isRemote();
+}
//===
FileSystemAction::ActionEntry::ActionEntry(): newName(0)
--
https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-actions-06/+merge/265197
Your team Ubuntu File Manager Developers is subscribed to branch
lp:ubuntu-filemanager-app.
--
Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers
Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers
More help : https://help.launchpad.net/ListHelp
[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-06 into lp:ubuntu-filemanager-app
Carlos Jose Mazieri has proposed merging
lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-06 into
lp:ubuntu-filemanager-app with
lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-05 as a prerequisite.
Commit message:
Introduced Location on Actions, every Action will keep its sourceLocation and
targetLocation.
It will be possible to distinguish if an Action is performed in Local Disk only
or if there is a remote Location involved.
Some DirItemInfo objects are created by respective Locations which items belong
to.
Requested reviews:
Ubuntu File Manager Developers (ubuntu-filemanager-dev)
For more details, see:
https://code.launchpad.net/~carlos-mazieri/ubuntu-filemanager-app/samba-actions-06/+merge/265197
Introduced Location on Actions.
--
Your team Ubuntu File Manager Developers is requested to review the proposed
merge of lp:~carlos-mazieri/ubuntu-filemanager-app/samba-actions-06 into
lp:ubuntu-filemanager-app.
=== modified file 'src/plugin/folderlistmodel/filesystemaction.cpp'
--- src/plugin/folderlistmodel/filesystemaction.cpp 2015-07-18 21:50:43 +
+++ src/plugin/folderlistmodel/filesystemaction.cpp 2015-07-18 21:50:44 +
@@ -128,10 +128,36 @@
//it is not necessary to delete
auxAction = 0;
copyFile.clear();
-
-}
-
-
+sourceLocation = 0;
+targetLocation = 0;
+
+}
+
+/*!
+ * \brief FileSystemAction::Action::toggleLocation()
+ *
+ * It may be useful if there is a Undo Action to do a inverse Action
+ */
+void FileSystemAction::Action::toggleLocation()
+{
+Location * tmp = sourceLocation;
+sourceLocation = targetLocation;
+targetLocation = tmp;
+}
+
+/*!
+ * \brief FileSystemAction::Action::matchLocations
+ * \return true if sourceLocation is equal targetLocation
+ */
+bool FileSystemAction::Action::matchLocations() const
+{
+return sourceLocation == targetLocation;
+}
+
+bool FileSystemAction::Action::isRemote() const
+{
+return sourceLocation-isRemote() || targetLocation-isRemote();
+}
//===
FileSystemAction::ActionEntry::ActionEntry(): newName(0)
@@ -224,15 +250,40 @@
//===
/*!
- * \brief FileSystemAction::createAction
+ * \brief FileSystemAction::createAction() Creates an Action struture
* \param type
- * \param origBase
+ * \param pathUrl the source URL (just the first one) to find the location
* \return
*/
-FileSystemAction::Action* FileSystemAction::createAction(ActionType type)
+FileSystemAction::Action* FileSystemAction::createAction(ActionType type, const QString pathUrl)
{
Action * action = new Action();
action-type = type;
+
+//get Locations, normal case for paste/remove
+action-sourceLocation = m_locationsFactory-parse(pathUrl);
+action-targetLocation = m_locationsFactory-currentLocation();
+switch (type)
+{
+ case ActionMoveToTrash:
+action-targetLocation = m_locationsFactory-getTrashLocation();
+break;
+ case ActionRestoreFromTrash: // the current location must already be TrashLocation
+action-sourceLocation = m_locationsFactory-getTrashLocation();
+//TODO check the URL from trash
+action-targetLocation = m_locationsFactory-getDiskLocation();
+break;
+ default:
+break;
+}
+if (action-sourceLocation == 0)
+{
+action-sourceLocation = m_locationsFactory-getDiskLocation();
+}
+if (action-targetLocation == 0)
+{
+action-targetLocation = m_locationsFactory-getDiskLocation();
+}
return action;
}
@@ -263,11 +314,18 @@
bool FileSystemAction::populateEntry(Action* action, ActionEntry* entry)
{
-DirItemInfo info(entry-itemPaths.source());
-if (!info.exists())
+QScopedPointerDirItemInfo info(action-sourceLocation-newItemInfo(entry-itemPaths.source()));
+if (!info-exists())
{
emit error(QObject::tr(File or Directory does not exist),
- info.absoluteFilePath() + QObject::tr( does not exist)
+ info-absoluteFilePath() + QObject::tr( does not exist)
+ );
+return false;
+}
+if (info-needsAuthentication())
+{
+emit error(QObject::tr(Cannot access File or Directory),
+ info-absoluteFilePath() + QObject::tr( it needs Authentication)
);
return false;
}
@@ -283,24 +341,36 @@
break;
}
//this is the item being handled
-entry-reversedOrder.append(info);
+entry-reversedOrder.append(*info);
// verify if the destination item already exists and it the destination path is in other file system
if (entry-type == ActionCopy ||
entry-type == ActionMove
)
{
-DirItemInfo destination(entry-itemPaths.target());
-
