Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing #7 and #11 #10

Merged
merged 9 commits into from
Dec 2, 2018
Merged

Fixing #7 and #11 #10

merged 9 commits into from
Dec 2, 2018

Conversation

haven1433
Copy link
Owner

@haven1433 haven1433 commented Nov 30, 2018

  • Changes to a file opened for edit will be reflected in the file without having to reload.
  • If there are local changes, they will not be overwritten.
  • No undo support: I could do a full file diff here looking for the changes and building up an undo item from it, but there's no good way to do an undo to an edit to the entire file at this point. More interestingly, the undo-history could be used as a way to "merge" the changes: rewind to last saved state, reload the file from disk, and fast-forward changes back to where we were. It's something to consider for a future change if it's requested.
  • Proper validation of the model for this simple bugfix required 6 new unit tests: 2 to validate that the model is adding/removing files to be watched properly, 2 to test the reload functionality, and 2 to test the IOException cases.
  • 2 additional tests were added to demonstrate the problems with Tab name not updated when empty file is saved #11, since that was a related issue.

…isk.

first pass at implementing the windows version of the new interface methods
IViewPort now has a public method to call whenever you want to notify it that its underlying file has been changed.
Added 3 tests showing how the Editor and ViewPort should behave with this new functionality.
- Editor should hook up ViewPorts whenever a new tab is opened
- Editor should clear out ViewPorts whenever a tab is closed
- ViewPort should reload when the file is changed and it is not.

Note that the model isn't implemented yet. This is just some failing tests.
…d of all at once.

  ... because you can have the same file open in two tabs.
- Allow HexContent's ContentChanged to come from a background thread
  ... important for when the notification comes from a file change
- Allow MainWindow to track deferred work
- Allow Tabs to request delayed work
- Allow EditorViewModel to request delayed work
  ... because sometimes windows notifies about a file change before the
      other application lets go of the file handle.
- IViewPort should actually expose its FileName
  ... EditorViewModel needs to know a ViewPort's file to setup / tear down
      the file watch in IFileSystem
- Implement the reload in ViewPort
  ... note that the reload can fail if the file is still being edited.
      if this happens, then ask the app to retry next time the user clicks.
- 2 more unit tests to capture the desired behavior of requesting delayed work.
Copy link

@rtzoeller rtzoeller left a comment

Choose a reason for hiding this comment

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

  • I'm not sure this is new behavior This is the same as the existing behavior, but I found a way to crash the editor:
  1. Open a file from disk
  2. Delete or rename the folder containing the file
  3. Modify the file in the editor and attempt to save it

Exception is at line 72 of WindowsFileSystem.cs.

System.IO.DirectoryNotFoundException: Could not find a part of the path 'C:\Users\rtzoeller\Documents\New folder\test.txt'.
  at at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
  at at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
  at at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
  at at System.IO.File.InternalWriteAllBytes(String path, Byte[] bytes, Boolean checkHost)
  at at System.IO.File.WriteAllBytes(String path, Byte[] bytes)
  at HavenSoft.Gen3Hex.View.WindowsFileSystem.Save(LoadedFile file) in C:\Users\rtzoeller\gen3hex\Gen3Hex\View\WindowsFileSystem.cs:72
  at HavenSoft.Gen3Hex.ViewModel.ViewPort.SaveExecuted(IFileSystem fileSystem) in C:\Users\rtzoeller\gen3hex\Gen3Hex\ViewModel\ViewPort.cs:171
  at HavenSoft.Gen3Hex.ViewModel.ViewPort.<ImplementCommands>b__91_3(Object arg) in C:\Users\rtzoeller\gen3hex\Gen3Hex\ViewModel\ViewPort.cs:254
  at System.Windows.Input.StubCommand.System.Windows.Input.ICommand.Execute(Object parameter) in C:\Users\rtzoeller\gen3hex\HavenSoft\generatedCode\StubCommand.cs:30
  at HavenSoft.Gen3Hex.ViewModel.EditorViewModel.<>c__DisplayClass111_0.<CreateWrapperForSelected>b__1(Object arg) in C:\Users\rtzoeller\gen3hex\Gen3Hex\ViewModel\EditorViewModel.cs:279
  at System.Windows.Input.StubCommand.System.Windows.Input.ICommand.Execute(Object parameter) in C:\Users\rtzoeller\gen3hex\HavenSoft\generatedCode\StubCommand.cs:30
  at at System.Windows.Input.CommandManager.TranslateInput(IInputElement targetElement, InputEventArgs inputEventArgs)
  at at System.Windows.UIElement.OnKeyDownThunk(Object sender, KeyEventArgs e)
  at at System.Windows.Input.KeyEventArgs.InvokeEventHandler(Delegate genericHandler, Object genericTarget)
  at at System.Windows.RoutedEventArgs.InvokeHandler(Delegate handler, Object target)
  at at System.Windows.RoutedEventHandlerInfo.InvokeHandler(Object target, RoutedEventArgs routedEventArgs)
  at at System.Windows.RouteItem.InvokeHandler(RoutedEventArgs routedEventArgs)
  at at System.Windows.EventRoute.InvokeHandlersImpl(Object source, RoutedEventArgs args, Boolean reRaised)
  at at System.Windows.EventRoute.InvokeHandlers(Object source, RoutedEventArgs args)
  at at System.Windows.UIElement.RaiseEventImpl(DependencyObject sender, RoutedEventArgs args)
  at at System.Windows.UIElement.RaiseTrustedEvent(RoutedEventArgs args)
  at at System.Windows.UIElement.RaiseEvent(RoutedEventArgs args, Boolean trusted)
  at at System.Windows.Input.InputManager.ProcessStagingArea()
  at at System.Windows.Input.InputManager.ProcessInput(InputEventArgs input)
  at at System.Windows.Input.InputProviderSite.ReportInput(InputReport inputReport)
  at at System.Windows.Interop.HwndKeyboardInputProvider.ReportInput(IntPtr hwnd, InputMode mode, Int32 timestamp, RawKeyboardActions actions, Int32 scanCode, Boolean isExtendedKey, Boolean isSystemKey, Int32 virtualKey)
  at at System.Windows.Interop.HwndKeyboardInputProvider.ProcessKeyAction(MSG& msg, Boolean& handled)
  at at System.Windows.Interop.HwndSource.CriticalTranslateAccelerator(MSG& msg, ModifierKeys modifiers)
  at at System.Windows.Interop.HwndSource.OnPreprocessMessage(Object param)
  at at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
  at at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
  at at System.Windows.Threading.Dispatcher.WrappedInvoke(Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
  at at System.Windows.Threading.Dispatcher.LegacyInvokeImpl(DispatcherPriority priority, TimeSpan timeout, Delegate method, Object args, Int32 numArgs)
  at at System.Windows.Threading.Dispatcher.Invoke(DispatcherPriority priority, Delegate method, Object arg)
  at at System.Windows.Interop.HwndSource.OnPreprocessMessageThunk(MSG& msg, Boolean& handled)
  at at System.Windows.Interop.HwndSource.WeakEventPreprocessMessage.OnPreprocessMessage(MSG& msg, Boolean& handled)
  at at System.Windows.Interop.ComponentDispatcherThread.RaiseThreadMessage(MSG& msg)
  at at System.Windows.Interop.ComponentDispatcher.RaiseThreadMessage(MSG& msg)
  at at System.Windows.Threading.Dispatcher.TranslateAndDispatchMessage(MSG& msg)
  at at System.Windows.Threading.Dispatcher.PushFrameImpl(DispatcherFrame frame)
  at at System.Windows.Threading.Dispatcher.PushFrame(DispatcherFrame frame)
  at at System.Windows.Threading.Dispatcher.Run()
  at at System.Windows.Application.RunDispatcher(Object ignore)
  at at System.Windows.Application.RunInternal(Window window)
  at at System.Windows.Application.Run(Window window)
  at at System.Windows.Application.Run()
  at at HavenSoft.Gen3Hex.View.App.Main()
  • I am getting an exception when I have modified and saved a file loaded by the editor in another program at line 132 of ScrollRegion.cs.
System.NotSupportedException: This type of CollectionView does not support changes to its SourceCollection from a thread different from the Dispatcher thread.
  at at System.Windows.Data.CollectionView.OnCollectionChanged(Object sender, NotifyCollectionChangedEventArgs args)
  at at System.Collections.ObjectModel.ObservableCollection`1.OnCollectionChanged(NotifyCollectionChangedEventArgs e)
  at at System.Collections.ObjectModel.ObservableCollection`1.OnCollectionChanged(NotifyCollectionChangedAction action, Object oldItem, Object newItem, Int32 index)
  at at System.Collections.ObjectModel.ObservableCollection`1.SetItem(Int32 index, T item)
  at at System.Collections.ObjectModel.Collection`1.set_Item(Int32 index, T value)
  at HavenSoft.Gen3Hex.ViewModel.ScrollRegion.UpdateHeaders() in C:\Users\rtzoeller\gen3hex\Gen3Hex\ViewModel\ScrollRegion.cs:162
  at HavenSoft.Gen3Hex.ViewModel.ScrollRegion.UpdateScrollRange() in C:\Users\rtzoeller\gen3hex\Gen3Hex\ViewModel\ScrollRegion.cs:150
  at HavenSoft.Gen3Hex.ViewModel.ScrollRegion.set_DataLength(Int32 value) in C:\Users\rtzoeller\gen3hex\Gen3Hex\ViewModel\ScrollRegion.cs:73
  at HavenSoft.Gen3Hex.ViewModel.ViewPort.ConsiderReload(IFileSystem fileSystem) in C:\Users\rtzoeller\gen3hex\Gen3Hex\ViewModel\ViewPort.cs:312
  at HavenSoft.Gen3Hex.View.WindowsFileSystem.<>c__DisplayClass7_0.<AddListenerToFile>b__0(Object sender, FileSystemEventArgs e) in C:\Users\rtzoeller\gen3hex\Gen3Hex\View\WindowsFileSystem.cs:37
  at at System.IO.FileSystemWatcher.OnChanged(FileSystemEventArgs e)
  at at System.IO.FileSystemWatcher.NotifyFileSystemEventArgs(Int32 action, String name)
  at at System.IO.FileSystemWatcher.CompletionStatusChanged(UInt32 errorCode, UInt32 numBytes, NativeOverlapped* overlappedPointer)
  at at System.Threading._IOCompletionCallback.PerformIOCompletionCallback(UInt32 errorCode, UInt32 numBytes, NativeOverlapped* pOVERLAP)
  • I am getting an exception when I have the file loaded multiple times in the editor, and save one of them, at line 147 of HexContent.cs.
System.NullReferenceException: Object reference not set to an instance of an object.
  at HavenSoft.Gen3Hex.View.HexContent.OnRender(DrawingContext drawingContext) in C:\Users\rtzoeller\gen3hex\Gen3Hex\View\HexContent.cs:147
  at at System.Windows.UIElement.Arrange(Rect finalRect)
  at at System.Windows.ContextLayoutManager.UpdateLayout()
  at at System.Windows.ContextLayoutManager.UpdateLayoutCallback(Object arg)
  at at System.Windows.Media.MediaContext.InvokeOnRenderCallback.DoWork()
  at at System.Windows.Media.MediaContext.FireInvokeOnRenderCallbacks()
  at at System.Windows.Media.MediaContext.RenderMessageHandlerCore(Object resizedCompositionTarget)
  at at System.Windows.Media.MediaContext.RenderMessageHandler(Object resizedCompositionTarget)
  at at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
  at at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
  at at System.Windows.Threading.Dispatcher.WrappedInvoke(Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
  at at System.Windows.Threading.DispatcherOperation.InvokeImpl()
  at at System.Windows.Threading.DispatcherOperation.InvokeInSecurityContext(Object state)
  at at MS.Internal.CulturePreservingExecutionContext.CallbackWrapper(Object obj)
  at at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
  at at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
  at at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
  at at MS.Internal.CulturePreservingExecutionContext.Run(CulturePreservingExecutionContext executionContext, ContextCallback callback, Object state)
  at at System.Windows.Threading.DispatcherOperation.Invoke()
  at at System.Windows.Threading.Dispatcher.ProcessQueue()
  at at System.Windows.Threading.Dispatcher.WndProcHook(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
  at at MS.Win32.HwndWrapper.WndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
  at at MS.Win32.HwndSubclass.DispatcherCallbackOperation(Object o)
  at at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
  at at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
  at at System.Windows.Threading.Dispatcher.WrappedInvoke(Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
  at at System.Windows.Threading.Dispatcher.LegacyInvokeImpl(DispatcherPriority priority, TimeSpan timeout, Delegate method, Object args, Int32 numArgs)
  at at System.Windows.Threading.Dispatcher.Invoke(DispatcherPriority priority, Delegate method, Object arg)
  at at MS.Win32.HwndSubclass.SubclassWndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam)
  at at MS.Win32.UnsafeNativeMethods.DispatchMessage(MSG& msg)
  at at System.Windows.Threading.Dispatcher.TranslateAndDispatchMessage(MSG& msg)
  at at System.Windows.Threading.Dispatcher.PushFrameImpl(DispatcherFrame frame)
  at at System.Windows.Threading.Dispatcher.PushFrame(DispatcherFrame frame)
  at at System.Windows.Threading.Dispatcher.Run()
  at at System.Windows.Application.RunDispatcher(Object ignore)
  at at System.Windows.Application.RunInternal(Window window)
  at at System.Windows.Application.Run(Window window)
  at at System.Windows.Application.Run()
  at at HavenSoft.Gen3Hex.View.App.Main()
  • Creating a new file, modifying it, then closing the tab without saving causes an exception to be thrown at line 186 of ViewPort.cs
System.NullReferenceException: Object reference not set to an instance of an object.
  at HavenSoft.Gen3Hex.ViewModel.ViewPort.CloseExecuted(IFileSystem fileSystem) in C:\Users\rtzoeller\gen3hex\Gen3Hex\ViewModel\ViewPort.cs:186
  at HavenSoft.Gen3Hex.ViewModel.ViewPort.<ImplementCommands>b__91_5(Object arg) in C:\Users\rtzoeller\gen3hex\Gen3Hex\ViewModel\ViewPort.cs:260
  at System.Windows.Input.StubCommand.System.Windows.Input.ICommand.Execute(Object parameter) in C:\Users\rtzoeller\gen3hex\HavenSoft\generatedCode\StubCommand.cs:30
  at at System.Windows.Input.CommandManager.TranslateInput(IInputElement targetElement, InputEventArgs inputEventArgs)
  at at System.Windows.UIElement.OnMouseDownThunk(Object sender, MouseButtonEventArgs e)
  at at System.Windows.Input.MouseButtonEventArgs.InvokeEventHandler(Delegate genericHandler, Object genericTarget)
  at at System.Windows.RoutedEventArgs.InvokeHandler(Delegate handler, Object target)
  at at System.Windows.RoutedEventHandlerInfo.InvokeHandler(Object target, RoutedEventArgs routedEventArgs)
  at at System.Windows.RouteItem.InvokeHandler(RoutedEventArgs routedEventArgs)
  at at System.Windows.EventRoute.InvokeHandlersImpl(Object source, RoutedEventArgs args, Boolean reRaised)
  at at System.Windows.EventRoute.InvokeHandlers(Object source, RoutedEventArgs args)
  at at System.Windows.UIElement.RaiseEventImpl(DependencyObject sender, RoutedEventArgs args)
  at at System.Windows.UIElement.RaiseTrustedEvent(RoutedEventArgs args)
  at at System.Windows.UIElement.RaiseEvent(RoutedEventArgs args, Boolean trusted)
  at at System.Windows.Input.InputManager.ProcessStagingArea()
  at at System.Windows.Input.InputManager.ProcessInput(InputEventArgs input)
  at at System.Windows.Input.InputProviderSite.ReportInput(InputReport inputReport)
  at at System.Windows.Interop.HwndMouseInputProvider.ReportInput(IntPtr hwnd, InputMode mode, Int32 timestamp, RawMouseActions actions, Int32 x, Int32 y, Int32 wheel)
  at at System.Windows.Interop.HwndMouseInputProvider.FilterMessage(IntPtr hwnd, WindowMessage msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
  at at System.Windows.Interop.HwndSource.InputFilterMessage(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
  at at MS.Win32.HwndWrapper.WndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
  at at MS.Win32.HwndSubclass.DispatcherCallbackOperation(Object o)
  at at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
  at at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
  at at System.Windows.Threading.Dispatcher.WrappedInvoke(Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
  at at System.Windows.Threading.Dispatcher.LegacyInvokeImpl(DispatcherPriority priority, TimeSpan timeout, Delegate method, Object args, Int32 numArgs)
  at at System.Windows.Threading.Dispatcher.Invoke(DispatcherPriority priority, Delegate method, Object arg)
  at at MS.Win32.HwndSubclass.SubclassWndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam)
  at at MS.Win32.UnsafeNativeMethods.DispatchMessage(MSG& msg)
  at at System.Windows.Threading.Dispatcher.TranslateAndDispatchMessage(MSG& msg)
  at at System.Windows.Threading.Dispatcher.PushFrameImpl(DispatcherFrame frame)
  at at System.Windows.Threading.Dispatcher.PushFrame(DispatcherFrame frame)
  at at System.Windows.Threading.Dispatcher.Run()
  at at System.Windows.Application.RunDispatcher(Object ignore)
  at at System.Windows.Application.RunInternal(Window window)
  at at System.Windows.Application.Run(Window window)
  at at System.Windows.Application.Run()
  at at HavenSoft.Gen3Hex.View.App.Main()

I should not be allowed near computers.

@rtzoeller
Copy link

@haven1433 adding a second comment to trigger an email; I modified my previous one in place a few times and I wanted to point out that I found a few crashes not caught in my original review.

@rtzoeller
Copy link

I don't know why the tracebacks have double ats in them, e.g. at at HavenSoft.Gen3Hex.View.App.Main(). I built and ran the program in JetBrains Rider instead of VS; let me know if you need help reproducing any of the crashes.

@haven1433
Copy link
Owner Author

I was unable to produce this one:

I am getting an exception when I have the file loaded multiple times in the editor, and save one of them, at line 147 of HexContent.cs.

I was able to reproduce and fix the others.

  an argument for closing tabs
- don't do threading work in collection change callbacks. Instead,
  do thread switching as early as possible, as soon as I realize that work
  needs to be done (in WindowsFileSystem).
- if ConsiderReload is called because of a file change and the file is now
  missing, then do nothing.
- if ConsiderReload is called because the file got shorter, update the
  Selection.
@haven1433
Copy link
Owner Author

I don't know why the tracebacks have double ats in them, e.g. ` at at.

I've seen the double ats in other programs before. I think it has something to do with the stack trace getting flattened from exceptions that contain inner exceptions. I'm not going to worry about it.

  (see HexContent)
- add keybinding for delete
- fix keybinding for cut
- clearing data via cut or delete needs to go on the undo stack
- call CanExecuteChanged on back/forward correctly
@rtzoeller
Copy link

I found a way to reliably reproduce changes not being reflected properly:

  1. Create a new file in the editor and modify it to contain some content.
  2. Save the file, choosing an arbitrary file name.
  3. Create another new file in the editor and modify it with different contents than the first file.
  4. Save the new file, choosing to use the same file name as the first file. Confirm replacing it.
  5. Switch back to the first file/tab and notice that it has not updated.

…FileName changes.

  ... This additionally requires that the FileName shares its oldValue when it changes.
- All TabContents are ViewModels
- ViewPort should notify when its FileName changes
@haven1433
Copy link
Owner Author

I found a way to reliably reproduce changes not being reflected properly:

Fixing this also fixed #11. Updating the title.

@haven1433 haven1433 changed the title Fixing #7 Fixing #7 and #11 Dec 1, 2018
Gen3Hex/ViewModel/EditorViewModel.cs Outdated Show resolved Hide resolved
@haven1433 haven1433 merged commit d141012 into master Dec 2, 2018
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.

2 participants