Skip to content

Conversation

d2dyno1
Copy link
Member

@d2dyno1 d2dyno1 commented Nov 16, 2020

This PR features complete Undo, Redo functionality. This PR is compatible with newest commit without merge issues. Please be aware that there still might be some bugs out there that sild in unnoticed.

Resolves #1975
Closes #2250

This PR contains complete Undo, Redo functionallity. This PR is compatible with newest commit without merge issues. Please be aware that there still might be some bugs out there that sild in unnoticed.
Had this commented out because it wouldn't let me compile
@d2dyno1
Copy link
Member Author

d2dyno1 commented Nov 16, 2020

The PR is Ready to be reviewed

@yaira2 yaira2 requested review from gave92 and tsvietOK November 16, 2020 15:02
gave92
gave92 previously requested changes Nov 16, 2020
Copy link
Member

@gave92 gave92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @d2dyno1 this is coming along nicely!

Some testing resulted in the following issues:

  • crash deleting from recycle bin
    2020-11-16 23:37:51.9637|ERROR|Files.App|Accesso negato. (Exception from HRESULT: 0x80070005 (E_ACCESSDENIED))|System.UnauthorizedAccessException: Accesso negato. (Exception from HRESULT: 0x80070005 (E_ACCESSDENIED))
    at Files.Helpers.StorageItemHelpers.ToStorageItemCollection(IEnumerable`1 listedItems)
    at Files.Views.Pages.ModernShellPage.KeyboardAccelerator_Invoked(KeyboardAccelerator sender, KeyboardAcceleratorInvokedEventArgs args)
    at System.Threading.WinRTSynchronizationContextBase.Invoker.InvokeCore()
  • crash on drag and drop after hovering on a folder (and so navigating to it)
    2020-11-16 23:39:21.3476|ERROR|Files.App|Object reference not set to an instance of an object.|System.NullReferenceException: Object reference not set to an instance of an object.
    at Files.Filesystem.FilesystemHelpers.MoveItemAsync(IStorageItem source, String destination, Boolean registerHistory)
    at Files.Filesystem.FilesystemHelpers.MoveItemsFromClipboard(DataPackageView packageView, String destination, Boolean registerHistory)
    at Files.Filesystem.FilesystemHelpers.PerformOperationTypeAsync(DataPackageOperation operation, DataPackageView packageView, String destination, Boolean registerHistory)
    at Files.BaseLayout.List_Drop(Object sender, DragEventArgs e)
    at System.Threading.WinRTSynchronizationContextBase.Invoker.InvokeCore()
  • cltr-x copies instead of cutting
  • cannot copy paste shortcut (.lnk) files
  • crash pasting file to windows folder
    2020-11-16 23:46:15.5820|ERROR|Files.App|Object reference not set to an instance of an object.|System.NullReferenceException: Object reference not set to an instance of an object.
    at Files.Filesystem.FilesystemOperations.CopyAsync(IStorageItem source, String destination, IProgress1 progress, IProgress1 errorCode, CancellationToken cancellationToken)
    at Files.Filesystem.FilesystemHelpers.CopyItemAsync(IStorageItem source, String destination, Boolean registerHistory)
    at Files.Filesystem.FilesystemHelpers.CopyItemsFromClipboard(DataPackageView packageView, String destination, Boolean registerHistory)
    at Files.Filesystem.FilesystemHelpers.PerformOperationTypeAsync(DataPackageOperation operation, DataPackageView packageView, String destination, Boolean registerHistory)
    at Files.Interacts.Interaction.PasteItem()
    at System.Threading.WinRTSynchronizationContextBase.Invoker.InvokeCore()
  • does not copy files to usb connected android phone anymore
  • crash when (this is difficult XD):
  1. copypaste a file to its same folder
  2. rename new file to something else
  3. ctrlz
  4. ctrlz
  5. ctrly
    2020-11-16 23:35:48.9522|ERROR|Files.App|Impossibile trovare il file specificato. (Exception from HRESULT: 0x80070002)|System.IO.FileNotFoundException: Impossibile trovare il file specificato. (Exception from HRESULT: 0x80070002)
    at Files.Helpers.StorageItemHelpers.ToStorageItem(String path)
    at Files.Filesystem.FilesystemHistory.StorageHistoryOperations.Redo(IStorageHistory history)
    at Files.Filesystem.FilesystemHistory.StorageHistoryHelpers.Redo()
    at Files.Views.Pages.ModernShellPage.KeyboardAccelerator_Invoked(KeyboardAccelerator sender, KeyboardAcceleratorInvokedEventArgs args)
    at System.Threading.WinRTSynchronizationContextBase.Invoker.InvokeCore()

@d2dyno1
Copy link
Member Author

d2dyno1 commented Nov 16, 2020

Looks like I'll be busy...

@d2dyno1
Copy link
Member Author

d2dyno1 commented Nov 16, 2020

  • crash when (this is difficult XD):
  1. copypaste a file to its same folder
  2. rename new file to something else
  3. ctrlz
  4. ctrlz
  5. ctrly

Are you trying to cast spells with this combination? 😆

@d2dyno1
Copy link
Member Author

d2dyno1 commented Nov 17, 2020

Here are all the changes requested:

  • Crash deleting from recycle bin
  • Dropping dragged items to a folder causes a crash (and so navigating to it)
  • CTRL+X copies instead of cutting
  • Cannot copy paste shortcut (.lnk) files
  • Crash pasting items to /Windows (and /Program Files (x86)) directories
  • Cannot delete files from usb connected android phone
  • Witchcraft combination
  • Remame Status to something more specific
  • Use FilesystemTasks.Wrap() on ToStorageItem()
  • Quickly pressing ctrlz + ctrly often results in a crash
  • Copy pasting a folder to the same location does not duplicate the folder
  • Copy pasting a folder "inside itself" and pressing ctrl-z causes a crash
  • Cannot Undo, Redo shortcut items

I'll report progress on them once they're completed

* - TBD

Delete dialog now works correctly. Files no longer crash when deleting from Recycle Bin. CTRL+X now moves items instead copying. Renamed Status enum to ReturnResult. Added SafeStorageItemHelpers class. Utilized FilesystemTasks on both StorageItemHelpers and SafeStorageItemHelpers
@d2dyno1
Copy link
Member Author

d2dyno1 commented Nov 17, 2020

@gave92 Hmmm... It doesn't seem to crash for me

crash pasting file to windows folder

@gave92
Copy link
Member

gave92 commented Nov 17, 2020

@d2dyno1 I'll check again 👍

@gave92
Copy link
Member

gave92 commented Nov 17, 2020

Thanks for your work on this, I can confirm that many issues have been resolved.

From testing the updated PR:

  • crash pasting file to windows folder
    -> I can confirm this, pasting to "C:\Program Files (x86)" does the same
  • witchcraft combination
    copypaste a file to its same folder
    rename new file to something else
    ctrlz
    ctrlz
    ctrly
    -> no crash anymore but the "ctrly" does nothing (should redo copy-paste) -> fixed!
  • quickly pressing ctrlz + ctrly often results in a crash
    copypaste file
    quickly press ctrlz and ctrly
    2020-11-17 19:29:54.0065|ERROR|Files.App|Object reference not set to an instance of an object.|System.NullReferenceException: Object reference not set to an instance of an object.
    at Files.Filesystem.FilesystemOperations.DeleteAsync(IStorageItem source, IProgress1 progress, IProgress1 errorCode, Boolean permanently, CancellationToken cancellationToken)
    at Files.Filesystem.FilesystemHistory.StorageHistoryOperations.Redo(IStorageHistory history)
    at Files.Filesystem.FilesystemHistory.StorageHistoryHelpers.Redo()
    at Files.Views.Pages.ModernShellPage.KeyboardAccelerator_Invoked(KeyboardAccelerator sender, KeyboardAcceleratorInvokedEventArgs args)
    at System.Threading.WinRTSynchronizationContextBase.Invoker.InvokeCore()

Code optimizations + I've created a little problem for myself in a StorageItemHelpers.ToStorageItem
@d2dyno1 d2dyno1 mentioned this pull request Nov 18, 2020
Fixed Delete!!!!!!!!!!!!!!!!!!!
@d2dyno1
Copy link
Member Author

d2dyno1 commented Nov 18, 2020

Ok, I got around my issue, but it'd be way cleaner if IStorageItemWithPath implemented IStorageItem
(We could then override the Path property)

@gave92
Copy link
Member

gave92 commented Nov 18, 2020

@d2dyno1 glad you solved it. But making IStorageItemWithPath implement IStorageItem is something that we can totally consider.

Changed private member names to camelCase
Resolved build error, fixed witchcraft combination. Added even more null checks
@d2dyno1
Copy link
Member Author

d2dyno1 commented Nov 23, 2020

50th comment!

@d2dyno1 d2dyno1 requested a review from tsvietOK November 23, 2020 18:52
@d2dyno1
Copy link
Member Author

d2dyno1 commented Nov 23, 2020

👍

@tsvietOK
Copy link
Contributor

@d2dyno1
Steps:

  1. Create bitmap with any name.
  2. Press F5.
  3. Try to rename this file

For me it shows:
image

@tsvietOK
Copy link
Contributor

@d2dyno1 By the way, when renaming item several times and trying doing undo several times, it reverts only last change.

@tsvietOK
Copy link
Contributor

@d2dyno1 Steps:

  1. Create folder and place file in this folder.
  2. Remove this folder to recycle bin.
  3. Press Ctrl+Z.
  4. It restore only folder without file inside.

Fixed an issue where, restoring a folder that contained items within in, would not restore the items. Rewritten the App.StorageHistoryIndex (once again)
@d2dyno1
Copy link
Member Author

d2dyno1 commented Nov 25, 2020

@tsvietOK Everything should be working now

@d2dyno1 d2dyno1 requested a review from tsvietOK November 26, 2020 22:22
yaira2
yaira2 previously approved these changes Nov 27, 2020
@yaira2 yaira2 requested a review from lukeblevins November 27, 2020 15:08
yaira2
yaira2 previously approved these changes Nov 27, 2020
@yaira2 yaira2 dismissed stale reviews from tsvietOK and gave92 November 27, 2020 17:16

Resolved

@yaira2 yaira2 merged commit b5d151e into files-community:master Nov 27, 2020
@d2dyno1
Copy link
Member Author

d2dyno1 commented Nov 27, 2020

I'd like to thank everyone who helped me with this PR. Without you, this PR wouldn't be as it is now!

Thanks,
@gave92 @tsvietOK @yaichenbaum @duke7553

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Undo, Redo functionallity

5 participants