Re: Review Request 129741: Add renaming capability to ioslaves
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129741/ --- (Updated Feb. 26, 2017, 11:10 p.m.) Status -- This change has been discarded. Review request for KDE Frameworks, David Faure and Emmanuel Pescosta. Repository: kio Description --- An ioslave protocol might support moving files but not "renaming" them (rename here means "the F2 shortcut in filemanagers"). trash:/ is a good example. The new `renaming` field defaults to `moving`, so that we don't have to add renaming=true to every protocol file out there. Diffs - src/core/kfileitemlistproperties.h 2b4a5b33166513493e6499e6479a04965a895b57 src/core/kfileitemlistproperties.cpp 5c6e6bba693f8b9bfc942ff39bee5d24f159fd7f src/core/kprotocolinfo.cpp 0290c63b37a45a22995238f9cfcc11b8334d339c src/core/kprotocolinfo_p.h 8d05bd194fdaa7b7e7552e0d1d22bf16b28ffbc1 src/core/kprotocolmanager.h 13b8c0756f8e355b1ec84cdf1c44086f41fa05c5 src/core/kprotocolmanager.cpp 9a0a96fe749a11c66a22bb3eff62d0601e3b7b36 src/ioslaves/trash/tests/testtrash.cpp 67a6130e7c86af00e596dc439125c29eb74fc99f src/ioslaves/trash/trash.json d7dc03eb073c7bfdde3c7eebfbac144ad62964fe src/protocoltojson/main.cpp 0bf3c7062d076412c779f6cae25a98e1b2c61be4 Diff: https://git.reviewboard.kde.org/r/129741/diff/ Testing --- Using the new `supportsRenaming()` api from Dolphin, F2 in Trash is now disabled. More context in https://git.reviewboard.kde.org/r/129714 Thanks, Elvis Angelaccio
Re: Review Request 129741: Add renaming capability to ioslaves
> On Jan. 2, 2017, 9:38 a.m., David Faure wrote: > > Renaming is really a special case of moving. Saying that "kio_trash doesn't > > support renaming" is correct but only a partial truth. It also doesn't > > support moving from trash:/ to trash:/subdir/. So it would be more correct > > to say that kio_trash supports moving trash-to-file and file-to-trash but > > not trash-to-trash. > > > > So it seems to me that this patch models the wrong thing, and is likely to > > give us further trouble down the line. > > > > I wonder if we could invent something more dynamic than the .protocol keys. > > An additional way to talk to a slave and ask if a specific operation (for > > specific URLs) is implemented. Something like KIO::CapabilityJob *job = > > KIO::capability(Move, url1, url2); connect; and in the slot, enable/disable > > the action accordingly. By default this would just use the information from > > the .protocol files, but in addition a new SlaveBase method (fake-virtual > > until KF6, using virtual_hook) would allow slaves to answer the query with > > more precision, depending on the actual URLs. What do you think? > > > > Of course the alternative (which is actually simpler short term) is to > > implement renaming in kio_trash ;) > > Actual renaming should be easy, moving between subdirs is a bit more tricky > > but doable too. I can take a look at that next weekend. > > Elvis Angelaccio wrote: > > Renaming is really a special case of moving. Saying that "kio_trash > doesn't support renaming" is correct but only a partial truth. It also > doesn't support moving from trash:/ to trash:/subdir/. So it would be more > correct to say that kio_trash supports moving trash-to-file and file-to-trash > but not trash-to-trash. > > Good point... > > Something like KIO::CapabilityJob *job = KIO::capability(Move, url1, > url2); connect; and in the slot, enable/disable the action accordingly. > > This would be more flexible indeed. On the other hand, it's more verbose > and low-level. Also, for the "renaming" case one needs a way to make up url2 > (url1 would just be the selected url in the dolphin view). > > Of course the alternative (which is actually simpler short term) is to > implement renaming in kio_trash ;) > > Definitely :) > All right, let's discuss this again later, waiting for news from > kio_trash. > > David Faure wrote: > Renaming implemented in kio_trash (for toplevel entries) > https://commits.kde.org/kio/20f0b84f51ff7f0767da118de79eda28af091ec9 > > Elvis Angelaccio wrote: > Thanks! Works fine from kioclient, hower it doesn't from Dolphin, which > doesn't prepend the 0- prefix to the destination url (so it uses > `trash:/destination` instead of `trash:/0-destination`). I wonder how this > could be fixed... > > David Faure wrote: > Good point. Does it work better now after > https://commits.kde.org/kio/9afce8395c6604697379046581bce24786a1bcb7 ? > > Elvis Angelaccio wrote: > Yes, now I can rename from dolphin. There is a (malformed url) error if I > try to restore a file that I just renamed, but it does work after if I reload > the view. Anyway I think I can discard this patch now! > > David Faure wrote: > Oh, I see. Dolphin (via KIO) asks to rename trash:/0-foo to trash:/bar, > and the resulting file has a URL of trash:/0-bar. > So the KFileItem has the wrong URL, anything you do with it will fail > (e.g. renaming it again). Argh. > > Ah, the KDirNotify signal emits the wrong URL, that's why. > https://commits.kde.org/kio/a1c040a43a19bc896c7a2f19703ce43c4c9b9423 > fixes this. > > Of course there's always one bug left: now the view shows 0-newname > instead of newname. It seems the item text is simply extracted from the URL > rather than UDS_DISPLAY_NAME being used. But now that looks like a KIO (or > Dolphin) bug, not a kio_trash issue. I can confirm. Smells like a bug in the dolphin's KFileItemModel... Thanks again, discarding this patch. - Elvis --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129741/#review101715 --- On Jan. 1, 2017, 11:01 p.m., Elvis Angelaccio wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129741/ > --- > > (Updated Jan. 1, 2017, 11:01 p.m.) > > > Review request for KDE Frameworks, David Faure and Emmanuel Pescosta. > > > Repository: kio > > > Description > --- > > An ioslave protocol might support moving files but not "renaming" them > (rename here means "the F2 shortcut in filemanagers"). trash:/ is a good > example. > > The new `renaming` field defaults to `moving`, so that we don't have
Re: Review Request 129741: Add renaming capability to ioslaves
> On Jan. 2, 2017, 9:38 a.m., David Faure wrote: > > Renaming is really a special case of moving. Saying that "kio_trash doesn't > > support renaming" is correct but only a partial truth. It also doesn't > > support moving from trash:/ to trash:/subdir/. So it would be more correct > > to say that kio_trash supports moving trash-to-file and file-to-trash but > > not trash-to-trash. > > > > So it seems to me that this patch models the wrong thing, and is likely to > > give us further trouble down the line. > > > > I wonder if we could invent something more dynamic than the .protocol keys. > > An additional way to talk to a slave and ask if a specific operation (for > > specific URLs) is implemented. Something like KIO::CapabilityJob *job = > > KIO::capability(Move, url1, url2); connect; and in the slot, enable/disable > > the action accordingly. By default this would just use the information from > > the .protocol files, but in addition a new SlaveBase method (fake-virtual > > until KF6, using virtual_hook) would allow slaves to answer the query with > > more precision, depending on the actual URLs. What do you think? > > > > Of course the alternative (which is actually simpler short term) is to > > implement renaming in kio_trash ;) > > Actual renaming should be easy, moving between subdirs is a bit more tricky > > but doable too. I can take a look at that next weekend. > > Elvis Angelaccio wrote: > > Renaming is really a special case of moving. Saying that "kio_trash > doesn't support renaming" is correct but only a partial truth. It also > doesn't support moving from trash:/ to trash:/subdir/. So it would be more > correct to say that kio_trash supports moving trash-to-file and file-to-trash > but not trash-to-trash. > > Good point... > > Something like KIO::CapabilityJob *job = KIO::capability(Move, url1, > url2); connect; and in the slot, enable/disable the action accordingly. > > This would be more flexible indeed. On the other hand, it's more verbose > and low-level. Also, for the "renaming" case one needs a way to make up url2 > (url1 would just be the selected url in the dolphin view). > > Of course the alternative (which is actually simpler short term) is to > implement renaming in kio_trash ;) > > Definitely :) > All right, let's discuss this again later, waiting for news from > kio_trash. > > David Faure wrote: > Renaming implemented in kio_trash (for toplevel entries) > https://commits.kde.org/kio/20f0b84f51ff7f0767da118de79eda28af091ec9 > > Elvis Angelaccio wrote: > Thanks! Works fine from kioclient, hower it doesn't from Dolphin, which > doesn't prepend the 0- prefix to the destination url (so it uses > `trash:/destination` instead of `trash:/0-destination`). I wonder how this > could be fixed... > > David Faure wrote: > Good point. Does it work better now after > https://commits.kde.org/kio/9afce8395c6604697379046581bce24786a1bcb7 ? > > Elvis Angelaccio wrote: > Yes, now I can rename from dolphin. There is a (malformed url) error if I > try to restore a file that I just renamed, but it does work after if I reload > the view. Anyway I think I can discard this patch now! Oh, I see. Dolphin (via KIO) asks to rename trash:/0-foo to trash:/bar, and the resulting file has a URL of trash:/0-bar. So the KFileItem has the wrong URL, anything you do with it will fail (e.g. renaming it again). Argh. Ah, the KDirNotify signal emits the wrong URL, that's why. https://commits.kde.org/kio/a1c040a43a19bc896c7a2f19703ce43c4c9b9423 fixes this. Of course there's always one bug left: now the view shows 0-newname instead of newname. It seems the item text is simply extracted from the URL rather than UDS_DISPLAY_NAME being used. But now that looks like a KIO (or Dolphin) bug, not a kio_trash issue. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129741/#review101715 --- On Jan. 1, 2017, 11:01 p.m., Elvis Angelaccio wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129741/ > --- > > (Updated Jan. 1, 2017, 11:01 p.m.) > > > Review request for KDE Frameworks, David Faure and Emmanuel Pescosta. > > > Repository: kio > > > Description > --- > > An ioslave protocol might support moving files but not "renaming" them > (rename here means "the F2 shortcut in filemanagers"). trash:/ is a good > example. > > The new `renaming` field defaults to `moving`, so that we don't have to add > renaming=true to every protocol file out there. > > > Diffs > - > > src/core/kfileitemlistproperties.h 2b4a5b33166513493e6499e6479a04965a895b57 >
Re: Review Request 129741: Add renaming capability to ioslaves
> On Jan. 2, 2017, 9:38 a.m., David Faure wrote: > > Renaming is really a special case of moving. Saying that "kio_trash doesn't > > support renaming" is correct but only a partial truth. It also doesn't > > support moving from trash:/ to trash:/subdir/. So it would be more correct > > to say that kio_trash supports moving trash-to-file and file-to-trash but > > not trash-to-trash. > > > > So it seems to me that this patch models the wrong thing, and is likely to > > give us further trouble down the line. > > > > I wonder if we could invent something more dynamic than the .protocol keys. > > An additional way to talk to a slave and ask if a specific operation (for > > specific URLs) is implemented. Something like KIO::CapabilityJob *job = > > KIO::capability(Move, url1, url2); connect; and in the slot, enable/disable > > the action accordingly. By default this would just use the information from > > the .protocol files, but in addition a new SlaveBase method (fake-virtual > > until KF6, using virtual_hook) would allow slaves to answer the query with > > more precision, depending on the actual URLs. What do you think? > > > > Of course the alternative (which is actually simpler short term) is to > > implement renaming in kio_trash ;) > > Actual renaming should be easy, moving between subdirs is a bit more tricky > > but doable too. I can take a look at that next weekend. > > Elvis Angelaccio wrote: > > Renaming is really a special case of moving. Saying that "kio_trash > doesn't support renaming" is correct but only a partial truth. It also > doesn't support moving from trash:/ to trash:/subdir/. So it would be more > correct to say that kio_trash supports moving trash-to-file and file-to-trash > but not trash-to-trash. > > Good point... > > Something like KIO::CapabilityJob *job = KIO::capability(Move, url1, > url2); connect; and in the slot, enable/disable the action accordingly. > > This would be more flexible indeed. On the other hand, it's more verbose > and low-level. Also, for the "renaming" case one needs a way to make up url2 > (url1 would just be the selected url in the dolphin view). > > Of course the alternative (which is actually simpler short term) is to > implement renaming in kio_trash ;) > > Definitely :) > All right, let's discuss this again later, waiting for news from > kio_trash. > > David Faure wrote: > Renaming implemented in kio_trash (for toplevel entries) > https://commits.kde.org/kio/20f0b84f51ff7f0767da118de79eda28af091ec9 > > Elvis Angelaccio wrote: > Thanks! Works fine from kioclient, hower it doesn't from Dolphin, which > doesn't prepend the 0- prefix to the destination url (so it uses > `trash:/destination` instead of `trash:/0-destination`). I wonder how this > could be fixed... > > David Faure wrote: > Good point. Does it work better now after > https://commits.kde.org/kio/9afce8395c6604697379046581bce24786a1bcb7 ? Yes, now I can rename from dolphin. There is a (malformed url) error if I try to restore a file that I just renamed, but it does work after if I reload the view. Anyway I think I can discard this patch now! - Elvis --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129741/#review101715 --- On Jan. 1, 2017, 11:01 p.m., Elvis Angelaccio wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129741/ > --- > > (Updated Jan. 1, 2017, 11:01 p.m.) > > > Review request for KDE Frameworks, David Faure and Emmanuel Pescosta. > > > Repository: kio > > > Description > --- > > An ioslave protocol might support moving files but not "renaming" them > (rename here means "the F2 shortcut in filemanagers"). trash:/ is a good > example. > > The new `renaming` field defaults to `moving`, so that we don't have to add > renaming=true to every protocol file out there. > > > Diffs > - > > src/core/kfileitemlistproperties.h 2b4a5b33166513493e6499e6479a04965a895b57 > src/core/kfileitemlistproperties.cpp > 5c6e6bba693f8b9bfc942ff39bee5d24f159fd7f > src/core/kprotocolinfo.cpp 0290c63b37a45a22995238f9cfcc11b8334d339c > src/core/kprotocolinfo_p.h 8d05bd194fdaa7b7e7552e0d1d22bf16b28ffbc1 > src/core/kprotocolmanager.h 13b8c0756f8e355b1ec84cdf1c44086f41fa05c5 > src/core/kprotocolmanager.cpp 9a0a96fe749a11c66a22bb3eff62d0601e3b7b36 > src/ioslaves/trash/tests/testtrash.cpp > 67a6130e7c86af00e596dc439125c29eb74fc99f > src/ioslaves/trash/trash.json d7dc03eb073c7bfdde3c7eebfbac144ad62964fe > src/protocoltojson/main.cpp 0bf3c7062d076412c779f6cae25a98e1b2c61be4 > > Diff: https://git.reviewboard.kde.org/r/129741/diff/ > > >
Re: Review Request 129741: Add renaming capability to ioslaves
> On Jan. 2, 2017, 9:38 a.m., David Faure wrote: > > Renaming is really a special case of moving. Saying that "kio_trash doesn't > > support renaming" is correct but only a partial truth. It also doesn't > > support moving from trash:/ to trash:/subdir/. So it would be more correct > > to say that kio_trash supports moving trash-to-file and file-to-trash but > > not trash-to-trash. > > > > So it seems to me that this patch models the wrong thing, and is likely to > > give us further trouble down the line. > > > > I wonder if we could invent something more dynamic than the .protocol keys. > > An additional way to talk to a slave and ask if a specific operation (for > > specific URLs) is implemented. Something like KIO::CapabilityJob *job = > > KIO::capability(Move, url1, url2); connect; and in the slot, enable/disable > > the action accordingly. By default this would just use the information from > > the .protocol files, but in addition a new SlaveBase method (fake-virtual > > until KF6, using virtual_hook) would allow slaves to answer the query with > > more precision, depending on the actual URLs. What do you think? > > > > Of course the alternative (which is actually simpler short term) is to > > implement renaming in kio_trash ;) > > Actual renaming should be easy, moving between subdirs is a bit more tricky > > but doable too. I can take a look at that next weekend. > > Elvis Angelaccio wrote: > > Renaming is really a special case of moving. Saying that "kio_trash > doesn't support renaming" is correct but only a partial truth. It also > doesn't support moving from trash:/ to trash:/subdir/. So it would be more > correct to say that kio_trash supports moving trash-to-file and file-to-trash > but not trash-to-trash. > > Good point... > > Something like KIO::CapabilityJob *job = KIO::capability(Move, url1, > url2); connect; and in the slot, enable/disable the action accordingly. > > This would be more flexible indeed. On the other hand, it's more verbose > and low-level. Also, for the "renaming" case one needs a way to make up url2 > (url1 would just be the selected url in the dolphin view). > > Of course the alternative (which is actually simpler short term) is to > implement renaming in kio_trash ;) > > Definitely :) > All right, let's discuss this again later, waiting for news from > kio_trash. > > David Faure wrote: > Renaming implemented in kio_trash (for toplevel entries) > https://commits.kde.org/kio/20f0b84f51ff7f0767da118de79eda28af091ec9 > > Elvis Angelaccio wrote: > Thanks! Works fine from kioclient, hower it doesn't from Dolphin, which > doesn't prepend the 0- prefix to the destination url (so it uses > `trash:/destination` instead of `trash:/0-destination`). I wonder how this > could be fixed... Good point. Does it work better now after https://commits.kde.org/kio/9afce8395c6604697379046581bce24786a1bcb7 ? - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129741/#review101715 --- On Jan. 1, 2017, 11:01 p.m., Elvis Angelaccio wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129741/ > --- > > (Updated Jan. 1, 2017, 11:01 p.m.) > > > Review request for KDE Frameworks, David Faure and Emmanuel Pescosta. > > > Repository: kio > > > Description > --- > > An ioslave protocol might support moving files but not "renaming" them > (rename here means "the F2 shortcut in filemanagers"). trash:/ is a good > example. > > The new `renaming` field defaults to `moving`, so that we don't have to add > renaming=true to every protocol file out there. > > > Diffs > - > > src/core/kfileitemlistproperties.h 2b4a5b33166513493e6499e6479a04965a895b57 > src/core/kfileitemlistproperties.cpp > 5c6e6bba693f8b9bfc942ff39bee5d24f159fd7f > src/core/kprotocolinfo.cpp 0290c63b37a45a22995238f9cfcc11b8334d339c > src/core/kprotocolinfo_p.h 8d05bd194fdaa7b7e7552e0d1d22bf16b28ffbc1 > src/core/kprotocolmanager.h 13b8c0756f8e355b1ec84cdf1c44086f41fa05c5 > src/core/kprotocolmanager.cpp 9a0a96fe749a11c66a22bb3eff62d0601e3b7b36 > src/ioslaves/trash/tests/testtrash.cpp > 67a6130e7c86af00e596dc439125c29eb74fc99f > src/ioslaves/trash/trash.json d7dc03eb073c7bfdde3c7eebfbac144ad62964fe > src/protocoltojson/main.cpp 0bf3c7062d076412c779f6cae25a98e1b2c61be4 > > Diff: https://git.reviewboard.kde.org/r/129741/diff/ > > > Testing > --- > > Using the new `supportsRenaming()` api from Dolphin, F2 in Trash is now > disabled. More context in https://git.reviewboard.kde.org/r/129714 > > > Thanks, > > Elvis Angelaccio > >
Re: Review Request 129741: Add renaming capability to ioslaves
> On Jan. 2, 2017, 9:38 a.m., David Faure wrote: > > Renaming is really a special case of moving. Saying that "kio_trash doesn't > > support renaming" is correct but only a partial truth. It also doesn't > > support moving from trash:/ to trash:/subdir/. So it would be more correct > > to say that kio_trash supports moving trash-to-file and file-to-trash but > > not trash-to-trash. > > > > So it seems to me that this patch models the wrong thing, and is likely to > > give us further trouble down the line. > > > > I wonder if we could invent something more dynamic than the .protocol keys. > > An additional way to talk to a slave and ask if a specific operation (for > > specific URLs) is implemented. Something like KIO::CapabilityJob *job = > > KIO::capability(Move, url1, url2); connect; and in the slot, enable/disable > > the action accordingly. By default this would just use the information from > > the .protocol files, but in addition a new SlaveBase method (fake-virtual > > until KF6, using virtual_hook) would allow slaves to answer the query with > > more precision, depending on the actual URLs. What do you think? > > > > Of course the alternative (which is actually simpler short term) is to > > implement renaming in kio_trash ;) > > Actual renaming should be easy, moving between subdirs is a bit more tricky > > but doable too. I can take a look at that next weekend. > > Elvis Angelaccio wrote: > > Renaming is really a special case of moving. Saying that "kio_trash > doesn't support renaming" is correct but only a partial truth. It also > doesn't support moving from trash:/ to trash:/subdir/. So it would be more > correct to say that kio_trash supports moving trash-to-file and file-to-trash > but not trash-to-trash. > > Good point... > > Something like KIO::CapabilityJob *job = KIO::capability(Move, url1, > url2); connect; and in the slot, enable/disable the action accordingly. > > This would be more flexible indeed. On the other hand, it's more verbose > and low-level. Also, for the "renaming" case one needs a way to make up url2 > (url1 would just be the selected url in the dolphin view). > > Of course the alternative (which is actually simpler short term) is to > implement renaming in kio_trash ;) > > Definitely :) > All right, let's discuss this again later, waiting for news from > kio_trash. > > David Faure wrote: > Renaming implemented in kio_trash (for toplevel entries) > https://commits.kde.org/kio/20f0b84f51ff7f0767da118de79eda28af091ec9 Thanks! Works fine from kioclient, hower it doesn't from Dolphin, which doesn't prepend the 0- prefix to the destination url (so it uses `trash:/destination` instead of `trash:/0-destination`). I wonder how this could be fixed... - Elvis --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129741/#review101715 --- On Jan. 1, 2017, 11:01 p.m., Elvis Angelaccio wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129741/ > --- > > (Updated Jan. 1, 2017, 11:01 p.m.) > > > Review request for KDE Frameworks, David Faure and Emmanuel Pescosta. > > > Repository: kio > > > Description > --- > > An ioslave protocol might support moving files but not "renaming" them > (rename here means "the F2 shortcut in filemanagers"). trash:/ is a good > example. > > The new `renaming` field defaults to `moving`, so that we don't have to add > renaming=true to every protocol file out there. > > > Diffs > - > > src/core/kfileitemlistproperties.h 2b4a5b33166513493e6499e6479a04965a895b57 > src/core/kfileitemlistproperties.cpp > 5c6e6bba693f8b9bfc942ff39bee5d24f159fd7f > src/core/kprotocolinfo.cpp 0290c63b37a45a22995238f9cfcc11b8334d339c > src/core/kprotocolinfo_p.h 8d05bd194fdaa7b7e7552e0d1d22bf16b28ffbc1 > src/core/kprotocolmanager.h 13b8c0756f8e355b1ec84cdf1c44086f41fa05c5 > src/core/kprotocolmanager.cpp 9a0a96fe749a11c66a22bb3eff62d0601e3b7b36 > src/ioslaves/trash/tests/testtrash.cpp > 67a6130e7c86af00e596dc439125c29eb74fc99f > src/ioslaves/trash/trash.json d7dc03eb073c7bfdde3c7eebfbac144ad62964fe > src/protocoltojson/main.cpp 0bf3c7062d076412c779f6cae25a98e1b2c61be4 > > Diff: https://git.reviewboard.kde.org/r/129741/diff/ > > > Testing > --- > > Using the new `supportsRenaming()` api from Dolphin, F2 in Trash is now > disabled. More context in https://git.reviewboard.kde.org/r/129714 > > > Thanks, > > Elvis Angelaccio > >
Re: Review Request 129741: Add renaming capability to ioslaves
> On Jan. 2, 2017, 9:38 a.m., David Faure wrote: > > Renaming is really a special case of moving. Saying that "kio_trash doesn't > > support renaming" is correct but only a partial truth. It also doesn't > > support moving from trash:/ to trash:/subdir/. So it would be more correct > > to say that kio_trash supports moving trash-to-file and file-to-trash but > > not trash-to-trash. > > > > So it seems to me that this patch models the wrong thing, and is likely to > > give us further trouble down the line. > > > > I wonder if we could invent something more dynamic than the .protocol keys. > > An additional way to talk to a slave and ask if a specific operation (for > > specific URLs) is implemented. Something like KIO::CapabilityJob *job = > > KIO::capability(Move, url1, url2); connect; and in the slot, enable/disable > > the action accordingly. By default this would just use the information from > > the .protocol files, but in addition a new SlaveBase method (fake-virtual > > until KF6, using virtual_hook) would allow slaves to answer the query with > > more precision, depending on the actual URLs. What do you think? > > > > Of course the alternative (which is actually simpler short term) is to > > implement renaming in kio_trash ;) > > Actual renaming should be easy, moving between subdirs is a bit more tricky > > but doable too. I can take a look at that next weekend. > > Elvis Angelaccio wrote: > > Renaming is really a special case of moving. Saying that "kio_trash > doesn't support renaming" is correct but only a partial truth. It also > doesn't support moving from trash:/ to trash:/subdir/. So it would be more > correct to say that kio_trash supports moving trash-to-file and file-to-trash > but not trash-to-trash. > > Good point... > > Something like KIO::CapabilityJob *job = KIO::capability(Move, url1, > url2); connect; and in the slot, enable/disable the action accordingly. > > This would be more flexible indeed. On the other hand, it's more verbose > and low-level. Also, for the "renaming" case one needs a way to make up url2 > (url1 would just be the selected url in the dolphin view). > > Of course the alternative (which is actually simpler short term) is to > implement renaming in kio_trash ;) > > Definitely :) > All right, let's discuss this again later, waiting for news from > kio_trash. Renaming implemented in kio_trash (for toplevel entries) https://commits.kde.org/kio/20f0b84f51ff7f0767da118de79eda28af091ec9 - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129741/#review101715 --- On Jan. 1, 2017, 11:01 p.m., Elvis Angelaccio wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129741/ > --- > > (Updated Jan. 1, 2017, 11:01 p.m.) > > > Review request for KDE Frameworks, David Faure and Emmanuel Pescosta. > > > Repository: kio > > > Description > --- > > An ioslave protocol might support moving files but not "renaming" them > (rename here means "the F2 shortcut in filemanagers"). trash:/ is a good > example. > > The new `renaming` field defaults to `moving`, so that we don't have to add > renaming=true to every protocol file out there. > > > Diffs > - > > src/core/kfileitemlistproperties.h 2b4a5b33166513493e6499e6479a04965a895b57 > src/core/kfileitemlistproperties.cpp > 5c6e6bba693f8b9bfc942ff39bee5d24f159fd7f > src/core/kprotocolinfo.cpp 0290c63b37a45a22995238f9cfcc11b8334d339c > src/core/kprotocolinfo_p.h 8d05bd194fdaa7b7e7552e0d1d22bf16b28ffbc1 > src/core/kprotocolmanager.h 13b8c0756f8e355b1ec84cdf1c44086f41fa05c5 > src/core/kprotocolmanager.cpp 9a0a96fe749a11c66a22bb3eff62d0601e3b7b36 > src/ioslaves/trash/tests/testtrash.cpp > 67a6130e7c86af00e596dc439125c29eb74fc99f > src/ioslaves/trash/trash.json d7dc03eb073c7bfdde3c7eebfbac144ad62964fe > src/protocoltojson/main.cpp 0bf3c7062d076412c779f6cae25a98e1b2c61be4 > > Diff: https://git.reviewboard.kde.org/r/129741/diff/ > > > Testing > --- > > Using the new `supportsRenaming()` api from Dolphin, F2 in Trash is now > disabled. More context in https://git.reviewboard.kde.org/r/129714 > > > Thanks, > > Elvis Angelaccio > >
Re: Review Request 129741: Add renaming capability to ioslaves
> On Jan. 2, 2017, 9:38 a.m., David Faure wrote: > > Renaming is really a special case of moving. Saying that "kio_trash doesn't > > support renaming" is correct but only a partial truth. It also doesn't > > support moving from trash:/ to trash:/subdir/. So it would be more correct > > to say that kio_trash supports moving trash-to-file and file-to-trash but > > not trash-to-trash. > > > > So it seems to me that this patch models the wrong thing, and is likely to > > give us further trouble down the line. > > > > I wonder if we could invent something more dynamic than the .protocol keys. > > An additional way to talk to a slave and ask if a specific operation (for > > specific URLs) is implemented. Something like KIO::CapabilityJob *job = > > KIO::capability(Move, url1, url2); connect; and in the slot, enable/disable > > the action accordingly. By default this would just use the information from > > the .protocol files, but in addition a new SlaveBase method (fake-virtual > > until KF6, using virtual_hook) would allow slaves to answer the query with > > more precision, depending on the actual URLs. What do you think? > > > > Of course the alternative (which is actually simpler short term) is to > > implement renaming in kio_trash ;) > > Actual renaming should be easy, moving between subdirs is a bit more tricky > > but doable too. I can take a look at that next weekend. > Renaming is really a special case of moving. Saying that "kio_trash doesn't > support renaming" is correct but only a partial truth. It also doesn't > support moving from trash:/ to trash:/subdir/. So it would be more correct to > say that kio_trash supports moving trash-to-file and file-to-trash but not > trash-to-trash. Good point... > Something like KIO::CapabilityJob *job = KIO::capability(Move, url1, url2); > connect; and in the slot, enable/disable the action accordingly. This would be more flexible indeed. On the other hand, it's more verbose and low-level. Also, for the "renaming" case one needs a way to make up url2 (url1 would just be the selected url in the dolphin view). > Of course the alternative (which is actually simpler short term) is to > implement renaming in kio_trash ;) Definitely :) All right, let's discuss this again later, waiting for news from kio_trash. - Elvis --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129741/#review101715 --- On Jan. 1, 2017, 11:01 p.m., Elvis Angelaccio wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129741/ > --- > > (Updated Jan. 1, 2017, 11:01 p.m.) > > > Review request for KDE Frameworks, David Faure and Emmanuel Pescosta. > > > Repository: kio > > > Description > --- > > An ioslave protocol might support moving files but not "renaming" them > (rename here means "the F2 shortcut in filemanagers"). trash:/ is a good > example. > > The new `renaming` field defaults to `moving`, so that we don't have to add > renaming=true to every protocol file out there. > > > Diffs > - > > src/core/kfileitemlistproperties.h 2b4a5b33166513493e6499e6479a04965a895b57 > src/core/kfileitemlistproperties.cpp > 5c6e6bba693f8b9bfc942ff39bee5d24f159fd7f > src/core/kprotocolinfo.cpp 0290c63b37a45a22995238f9cfcc11b8334d339c > src/core/kprotocolinfo_p.h 8d05bd194fdaa7b7e7552e0d1d22bf16b28ffbc1 > src/core/kprotocolmanager.h 13b8c0756f8e355b1ec84cdf1c44086f41fa05c5 > src/core/kprotocolmanager.cpp 9a0a96fe749a11c66a22bb3eff62d0601e3b7b36 > src/ioslaves/trash/tests/testtrash.cpp > 67a6130e7c86af00e596dc439125c29eb74fc99f > src/ioslaves/trash/trash.json d7dc03eb073c7bfdde3c7eebfbac144ad62964fe > src/protocoltojson/main.cpp 0bf3c7062d076412c779f6cae25a98e1b2c61be4 > > Diff: https://git.reviewboard.kde.org/r/129741/diff/ > > > Testing > --- > > Using the new `supportsRenaming()` api from Dolphin, F2 in Trash is now > disabled. More context in https://git.reviewboard.kde.org/r/129714 > > > Thanks, > > Elvis Angelaccio > >
Re: Review Request 129741: Add renaming capability to ioslaves
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129741/#review101715 --- Renaming is really a special case of moving. Saying that "kio_trash doesn't support renaming" is correct but only a partial truth. It also doesn't support moving from trash:/ to trash:/subdir/. So it would be more correct to say that kio_trash supports moving trash-to-file and file-to-trash but not trash-to-trash. So it seems to me that this patch models the wrong thing, and is likely to give us further trouble down the line. I wonder if we could invent something more dynamic than the .protocol keys. An additional way to talk to a slave and ask if a specific operation (for specific URLs) is implemented. Something like KIO::CapabilityJob *job = KIO::capability(Move, url1, url2); connect; and in the slot, enable/disable the action accordingly. By default this would just use the information from the .protocol files, but in addition a new SlaveBase method (fake-virtual until KF6, using virtual_hook) would allow slaves to answer the query with more precision, depending on the actual URLs. What do you think? Of course the alternative (which is actually simpler short term) is to implement renaming in kio_trash ;) Actual renaming should be easy, moving between subdirs is a bit more tricky but doable too. I can take a look at that next weekend. - David Faure On Jan. 1, 2017, 11:01 p.m., Elvis Angelaccio wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129741/ > --- > > (Updated Jan. 1, 2017, 11:01 p.m.) > > > Review request for KDE Frameworks, David Faure and Emmanuel Pescosta. > > > Repository: kio > > > Description > --- > > An ioslave protocol might support moving files but not "renaming" them > (rename here means "the F2 shortcut in filemanagers"). trash:/ is a good > example. > > The new `renaming` field defaults to `moving`, so that we don't have to add > renaming=true to every protocol file out there. > > > Diffs > - > > src/core/kfileitemlistproperties.h 2b4a5b33166513493e6499e6479a04965a895b57 > src/core/kfileitemlistproperties.cpp > 5c6e6bba693f8b9bfc942ff39bee5d24f159fd7f > src/core/kprotocolinfo.cpp 0290c63b37a45a22995238f9cfcc11b8334d339c > src/core/kprotocolinfo_p.h 8d05bd194fdaa7b7e7552e0d1d22bf16b28ffbc1 > src/core/kprotocolmanager.h 13b8c0756f8e355b1ec84cdf1c44086f41fa05c5 > src/core/kprotocolmanager.cpp 9a0a96fe749a11c66a22bb3eff62d0601e3b7b36 > src/ioslaves/trash/tests/testtrash.cpp > 67a6130e7c86af00e596dc439125c29eb74fc99f > src/ioslaves/trash/trash.json d7dc03eb073c7bfdde3c7eebfbac144ad62964fe > src/protocoltojson/main.cpp 0bf3c7062d076412c779f6cae25a98e1b2c61be4 > > Diff: https://git.reviewboard.kde.org/r/129741/diff/ > > > Testing > --- > > Using the new `supportsRenaming()` api from Dolphin, F2 in Trash is now > disabled. More context in https://git.reviewboard.kde.org/r/129714 > > > Thanks, > > Elvis Angelaccio > >
Review Request 129741: Add renaming capability to ioslaves
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129741/ --- Review request for KDE Frameworks, David Faure and Emmanuel Pescosta. Repository: kio Description --- An ioslave protocol might support moving files but not "renaming" them (rename here means "the F2 shortcut in filemanagers"). trash:/ is a good example. The new `renaming` field defaults to `moving`, so that we don't have to add renaming=true to every protocol file out there. Diffs - src/core/kfileitemlistproperties.h 2b4a5b33166513493e6499e6479a04965a895b57 src/core/kfileitemlistproperties.cpp 5c6e6bba693f8b9bfc942ff39bee5d24f159fd7f src/core/kprotocolinfo.cpp 0290c63b37a45a22995238f9cfcc11b8334d339c src/core/kprotocolinfo_p.h 8d05bd194fdaa7b7e7552e0d1d22bf16b28ffbc1 src/core/kprotocolmanager.h 13b8c0756f8e355b1ec84cdf1c44086f41fa05c5 src/core/kprotocolmanager.cpp 9a0a96fe749a11c66a22bb3eff62d0601e3b7b36 src/ioslaves/trash/tests/testtrash.cpp 67a6130e7c86af00e596dc439125c29eb74fc99f src/ioslaves/trash/trash.json d7dc03eb073c7bfdde3c7eebfbac144ad62964fe src/protocoltojson/main.cpp 0bf3c7062d076412c779f6cae25a98e1b2c61be4 Diff: https://git.reviewboard.kde.org/r/129741/diff/ Testing --- Using the new `supportsRenaming()` api from Dolphin, F2 in Trash is now disabled. More context in https://git.reviewboard.kde.org/r/129714 Thanks, Elvis Angelaccio
