From b2d7182dcdbe871f7e96f573200bfec8f74cc4cc Mon Sep 17 00:00:00 2001 From: Heiko <61519853+htcfreek@users.noreply.github.com> Date: Tue, 17 Jun 2025 05:35:35 +0200 Subject: [PATCH] [RegPreview] Various improvements on how files are saved (#37628) ## Summary of the Pull Request ### This PR implements various fixes and improvements into RegistryPreview for saving files: 1. Adds an unsaved file indicator in the title bar like in Notepad. (As indicator we show the * character before the file title.) 2. The save button behaves like a "save as" button, if the file does not exist on disk like in Notepad. (Without fix when running as non-admin you get an access denied error message.) 3. If the app gets closed without saving and the file does not exist on disk, then the user is now asked for the path. (Fixes crash on clicking save button while closing the app.) 4. Failed save actions are handled now correctly on dirty closing, opening files and all other actions that require saving the current state. They will stop the process. 5. A fix for an incorrect enabled state of the save button after opening, reloading and saving a file. 6. Reuse file name on save as button, if known. Otherwise use "new file" template name like in Notepad. 7. Fix an app crash if you click the window's close button a second time while the "Should save?" dialog is opened. 8. Added an reload dialog in case of unsaved changes. ![image](https://github.com/user-attachments/assets/9045446e-e9a3-4b81-8aa0-515b0821a969) ![image](https://github.com/user-attachments/assets/0888fbd2-851b-4101-a177-be9a3675b5ae) ## PR Checklist - [x] **Closes:** #36876, #36875 - [x] **Communication:** I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [x] **Localization:** All end user facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx ## Detailed Description of the Pull Request / Additional comments ## Validation Steps Performed Local test build. --------- Co-authored-by: Gordon Lam (SH) --- .../RegistryPreviewMainPage.Events.cs | 189 ++++++++++++------ .../RegistryPreviewMainPage.Utilities.cs | 161 ++++++++++++--- .../Strings/en-US/Resources.resw | 9 + 3 files changed, 267 insertions(+), 92 deletions(-) diff --git a/src/modules/registrypreview/RegistryPreviewUILib/RegistryPreviewMainPage.Events.cs b/src/modules/registrypreview/RegistryPreviewUILib/RegistryPreviewMainPage.Events.cs index c8f72837f1..b2bf8ea277 100644 --- a/src/modules/registrypreview/RegistryPreviewUILib/RegistryPreviewMainPage.Events.cs +++ b/src/modules/registrypreview/RegistryPreviewUILib/RegistryPreviewMainPage.Events.cs @@ -22,6 +22,10 @@ namespace RegistryPreviewUILib { private readonly DispatcherQueue _dispatcherQueue = DispatcherQueue.GetForCurrentThread(); + // Indicator if we loaded/reloaded/saved a file and need to skip TextChanged event one time. + // (Solves the problem that enabling the event handler fires it one time.) + private static bool editorContentChangedScripted; + /// /// Event that is will prevent the app from closing if the "save file" flag is active /// @@ -76,6 +80,67 @@ namespace RegistryPreviewUILib MonacoEditor.Focus(FocusState.Programmatic); } + /// + /// New button action: Ask to save last changes and reset editor content to reg header only + /// + private async void NewButton_Click(object sender, RoutedEventArgs e) + { + // Check to see if the current file has been saved + if (saveButton.IsEnabled) + { + ContentDialog contentDialog = new ContentDialog() + { + Title = resourceLoader.GetString("YesNoCancelDialogTitle"), + Content = resourceLoader.GetString("YesNoCancelDialogContent"), + PrimaryButtonText = resourceLoader.GetString("YesNoCancelDialogPrimaryButtonText"), + SecondaryButtonText = resourceLoader.GetString("YesNoCancelDialogSecondaryButtonText"), + CloseButtonText = resourceLoader.GetString("YesNoCancelDialogCloseButtonText"), + DefaultButton = ContentDialogButton.Primary, + }; + + // Use this code to associate the dialog to the appropriate AppWindow by setting + // the dialog's XamlRoot to the same XamlRoot as an element that is already present in the AppWindow. + if (ApiInformation.IsApiContractPresent("Windows.Foundation.UniversalApiContract", 8)) + { + contentDialog.XamlRoot = this.Content.XamlRoot; + } + + ContentDialogResult contentDialogResult = await contentDialog.ShowAsync(); + switch (contentDialogResult) + { + case ContentDialogResult.Primary: + // Save, then continue the new action + if (!AskFileName(string.Empty) || + !SaveFile()) + { + return; + } + + break; + case ContentDialogResult.Secondary: + // Don't save and continue the new action! + break; + default: + // Don't open the new action! + return; + } + } + + // mute the TextChanged handler to make for clean UI + MonacoEditor.TextChanged -= MonacoEditor_TextChanged; + + // reset editor, file info and ui. + _appFileName = string.Empty; + ResetEditorAndFile(); + + // disable buttons that do not make sense + UpdateUnsavedFileState(false); + refreshButton.IsEnabled = false; + + // restore the TextChanged handler + ButtonAction_RestoreTextChangedEvent(); + } + /// /// Uses a picker to select a new file to open /// @@ -106,11 +171,15 @@ namespace RegistryPreviewUILib { case ContentDialogResult.Primary: // Save, then continue the file open - SaveFile(); + if (!AskFileName(string.Empty) || + !SaveFile()) + { + return; + } + break; case ContentDialogResult.Secondary: // Don't save and continue the file open! - saveButton.IsEnabled = false; break; default: // Don't open the new file! @@ -137,14 +206,16 @@ namespace RegistryPreviewUILib { // mute the TextChanged handler to make for clean UI MonacoEditor.TextChanged -= MonacoEditor_TextChanged; + + // update file name _appFileName = storageFile.Path; UpdateToolBarAndUI(await OpenRegistryFile(_appFileName)); // disable the Save button as it's a new file - saveButton.IsEnabled = false; + UpdateUnsavedFileState(false); // Restore the event handler as we're loaded - MonacoEditor.TextChanged += MonacoEditor_TextChanged; + ButtonAction_RestoreTextChangedEvent(); } } @@ -153,7 +224,14 @@ namespace RegistryPreviewUILib /// private void SaveButton_Click(object sender, RoutedEventArgs e) { - SaveFile(); + if (!AskFileName(string.Empty)) + { + return; + } + + // save and update window title + // error handling and ui update happens in SaveFile() method + _ = SaveFile(); } /// @@ -161,47 +239,24 @@ namespace RegistryPreviewUILib /// private async void SaveAsButton_Click(object sender, RoutedEventArgs e) { - // Save out a new REG file and then open it - we have to use the direct Win32 method because FileOpenPicker crashes when it's - // called while running as admin - IntPtr windowHandle = WinRT.Interop.WindowNative.GetWindowHandle(_mainWindow); - string filename = SaveFilePicker.ShowDialog( - windowHandle, - resourceLoader.GetString("SuggestFileName"), - resourceLoader.GetString("FilterRegistryName") + '\0' + "*.reg" + '\0' + resourceLoader.GetString("FilterAllFiles") + '\0' + "*.*" + '\0' + '\0', - resourceLoader.GetString("SaveDialogTitle")); + // mute the TextChanged handler to make for clean UI + MonacoEditor.TextChanged -= MonacoEditor_TextChanged; - if (filename == string.Empty) + if (!AskFileName(_appFileName) || !SaveFile()) { return; } - _appFileName = filename; - SaveFile(); UpdateToolBarAndUI(await OpenRegistryFile(_appFileName)); + + // restore the TextChanged handler + ButtonAction_RestoreTextChangedEvent(); } /// /// Reloads the current REG file from storage /// private async void RefreshButton_Click(object sender, RoutedEventArgs e) - { - // mute the TextChanged handler to make for clean UI - MonacoEditor.TextChanged -= MonacoEditor_TextChanged; - - // reload the current Registry file and update the toolbar accordingly. - UpdateToolBarAndUI(await OpenRegistryFile(_appFileName), true, true); - - // disable the Save button as it's a new file - saveButton.IsEnabled = false; - - // restore the TextChanged handler - MonacoEditor.TextChanged += MonacoEditor_TextChanged; - } - - /// - /// Resets the editor content - /// - private async void NewButton_Click(object sender, RoutedEventArgs e) { // Check to see if the current file has been saved if (saveButton.IsEnabled) @@ -209,10 +264,9 @@ namespace RegistryPreviewUILib ContentDialog contentDialog = new ContentDialog() { Title = resourceLoader.GetString("YesNoCancelDialogTitle"), - Content = resourceLoader.GetString("YesNoCancelDialogContent"), - PrimaryButtonText = resourceLoader.GetString("YesNoCancelDialogPrimaryButtonText"), - SecondaryButtonText = resourceLoader.GetString("YesNoCancelDialogSecondaryButtonText"), - CloseButtonText = resourceLoader.GetString("YesNoCancelDialogCloseButtonText"), + Content = resourceLoader.GetString("ReloadDialogContent"), + PrimaryButtonText = resourceLoader.GetString("ReloadDialogPrimaryButtonText"), + CloseButtonText = resourceLoader.GetString("ReloadDialogCloseButtonText"), DefaultButton = ContentDialogButton.Primary, }; @@ -227,15 +281,10 @@ namespace RegistryPreviewUILib switch (contentDialogResult) { case ContentDialogResult.Primary: - // Save, then continue the file open - SaveFile(); - break; - case ContentDialogResult.Secondary: - // Don't save and continue the file open! - saveButton.IsEnabled = false; + // Don't save and continue the reload action! break; default: - // Don't open the new file! + // Don't continue the reload action! return; } } @@ -243,16 +292,14 @@ namespace RegistryPreviewUILib // mute the TextChanged handler to make for clean UI MonacoEditor.TextChanged -= MonacoEditor_TextChanged; - // reset editor, file info and ui. - _appFileName = string.Empty; - ResetEditorAndFile(); + // reload the current Registry file and update the toolbar accordingly. + UpdateToolBarAndUI(await OpenRegistryFile(_appFileName), true, true); + + // disable the Save button as it's a new file + UpdateUnsavedFileState(false); // restore the TextChanged handler - MonacoEditor.TextChanged += MonacoEditor_TextChanged; - - // disable buttons that do not make sense - saveButton.IsEnabled = false; - refreshButton.IsEnabled = false; + ButtonAction_RestoreTextChangedEvent(); } /// @@ -313,15 +360,20 @@ namespace RegistryPreviewUILib switch (contentDialogResult) { case ContentDialogResult.Primary: - // Save, then continue the file open - SaveFile(); + // Save, then continue the merge action + if (!AskFileName(string.Empty) || + !SaveFile()) + { + return; + } + break; case ContentDialogResult.Secondary: - // Don't save and continue the file open! - saveButton.IsEnabled = false; + // Don't save and continue the merge action! + UpdateUnsavedFileState(false); break; default: - // Don't open the new file! + // Don't merge the file! return; } } @@ -411,10 +463,29 @@ namespace RegistryPreviewUILib _dispatcherQueue.TryEnqueue(() => { RefreshRegistryFile(); - saveButton.IsEnabled = true; + if (!editorContentChangedScripted) + { + UpdateUnsavedFileState(true); + } + + editorContentChangedScripted = false; }); } + /// + /// Sets indicator for programatic text change and adds text changed handler + /// + /// + /// Use this always, if button actions temporary disable the text changed event + /// + private void ButtonAction_RestoreTextChangedEvent() + { + // Solves the problem that enabling the event handler fires it one time. + // These one time fired event would causes wrong unsaved changes state. + editorContentChangedScripted = true; + MonacoEditor.TextChanged += MonacoEditor_TextChanged; + } + // Commands to show data preview public void ButtonExtendedPreview_Click(object sender, RoutedEventArgs e) { diff --git a/src/modules/registrypreview/RegistryPreviewUILib/RegistryPreviewMainPage.Utilities.cs b/src/modules/registrypreview/RegistryPreviewUILib/RegistryPreviewMainPage.Utilities.cs index 57d27fdc12..2211113f2c 100644 --- a/src/modules/registrypreview/RegistryPreviewUILib/RegistryPreviewMainPage.Utilities.cs +++ b/src/modules/registrypreview/RegistryPreviewUILib/RegistryPreviewMainPage.Utilities.cs @@ -11,6 +11,7 @@ using System.IO; using System.Linq; using System.Reflection; using System.Text; +using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; using Microsoft.UI.Input; @@ -24,7 +25,10 @@ namespace RegistryPreviewUILib { private const string NEWFILEHEADER = "Windows Registry Editor Version 5.00\r\n\r\n"; + private static readonly string _unsavedFileIndicator = "* "; + private static readonly char[] _unsavedFileIndicatorChars = [' ', '*']; private static SemaphoreSlim _dialogSemaphore = new(1); + private string lastKeyPath; public delegate void UpdateWindowTitleFunction(string title); @@ -832,42 +836,66 @@ namespace RegistryPreviewUILib /// private async void HandleDirtyClosing(string title, string content, string primaryButtonText, string secondaryButtonText, string closeButtonText) { - ContentDialog contentDialog = new ContentDialog() + if (_dialogSemaphore.CurrentCount == 0) { - Title = title, - Content = content, - PrimaryButtonText = primaryButtonText, - SecondaryButtonText = secondaryButtonText, - CloseButtonText = closeButtonText, - DefaultButton = ContentDialogButton.Primary, - }; - - // Use this code to associate the dialog to the appropriate AppWindow by setting - // the dialog's XamlRoot to the same XamlRoot as an element that is already present in the AppWindow. - if (ApiInformation.IsApiContractPresent("Windows.Foundation.UniversalApiContract", 8)) - { - contentDialog.XamlRoot = this.Content.XamlRoot; + return; } - ContentDialogResult contentDialogResult = await contentDialog.ShowAsync(); - - switch (contentDialogResult) + try { - case ContentDialogResult.Primary: - // Save, then close - SaveFile(); - break; - case ContentDialogResult.Secondary: - // Don't save, and then close! - saveButton.IsEnabled = false; - break; - default: - // Cancel closing! - return; - } + await _dialogSemaphore.WaitAsync(); - // if we got here, we should try to close again - Application.Current.Exit(); + ContentDialog contentDialog = new ContentDialog() + { + Title = title, + Content = content, + PrimaryButtonText = primaryButtonText, + SecondaryButtonText = secondaryButtonText, + CloseButtonText = closeButtonText, + DefaultButton = ContentDialogButton.Primary, + }; + + // Use this code to associate the dialog to the appropriate AppWindow by setting + // the dialog's XamlRoot to the same XamlRoot as an element that is already present in the AppWindow. + if (ApiInformation.IsApiContractPresent("Windows.Foundation.UniversalApiContract", 8)) + { + contentDialog.XamlRoot = this.Content.XamlRoot; + } + + ContentDialogResult contentDialogResult = await contentDialog.ShowAsync(); + + switch (contentDialogResult) + { + case ContentDialogResult.Primary: + // Save, then close + if (!AskFileName(string.Empty) || + !SaveFile()) + { + return; + } + + break; + case ContentDialogResult.Secondary: + // Don't save, and then close! + UpdateUnsavedFileState(false); + break; + default: + // Cancel closing! + return; + } + + // if we got here, we should try to close again + Application.Current.Exit(); + } + catch + { + // Normally nothing to catch here. + // But for safety the try-catch ensures that we always release the content dialog lock and exit correctly. + } + finally + { + _dialogSemaphore.Release(); + } } /// @@ -927,11 +955,71 @@ namespace RegistryPreviewUILib type.InvokeMember("ProtectedCursor", BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.SetProperty | BindingFlags.Instance, null, uiElement, new object[] { cursor }, CultureInfo.InvariantCulture); } + public void UpdateUnsavedFileState(bool unsavedChanges) + { + // get, cut and analyze the current title + string currentTitle = Regex.Replace(_mainWindow.Title, APPNAME + @"$|\s-\s" + APPNAME + @"$", string.Empty); + bool titleContainsIndicator = currentTitle.StartsWith(_unsavedFileIndicator, StringComparison.CurrentCultureIgnoreCase); + + // update window title and save button state + if (unsavedChanges) + { + saveButton.IsEnabled = true; + + if (!titleContainsIndicator) + { + _updateWindowTitleFunction(_unsavedFileIndicator + currentTitle); + } + } + else + { + saveButton.IsEnabled = false; + + if (titleContainsIndicator) + { + _updateWindowTitleFunction(currentTitle.TrimStart(_unsavedFileIndicatorChars)); + } + } + } + + /// + /// Ask the user for the file path if it is unknown because of an unsaved file + /// + /// If not empty always ask for a file path and use the value as name. + /// Returns true if user selected a path, otherwise false + public bool AskFileName(string fileName) + { + if (string.IsNullOrEmpty(_appFileName) || !string.IsNullOrEmpty(fileName) ) + { + string fName = string.IsNullOrEmpty(fileName) ? resourceLoader.GetString("SuggestFileName") : fileName; + + // Save out a new REG file and then open it - we have to use the direct Win32 method because FileOpenPicker crashes when it's + // called while running as admin + IntPtr windowHandle = WinRT.Interop.WindowNative.GetWindowHandle(_mainWindow); + string filename = SaveFilePicker.ShowDialog( + windowHandle, + fName, + resourceLoader.GetString("FilterRegistryName") + '\0' + "*.reg" + '\0' + resourceLoader.GetString("FilterAllFiles") + '\0' + "*.*" + '\0' + '\0', + resourceLoader.GetString("SaveDialogTitle")); + + if (filename == string.Empty) + { + return false; + } + + _appFileName = filename; + } + + return true; + } + /// /// Wrapper method that saves the current file in place, using the current text in editor. /// - private void SaveFile() + private bool SaveFile() { + bool saveSuccess = true; + ChangeCursor(gridPreview, true); // set up the FileStream for all writing @@ -955,10 +1043,13 @@ namespace RegistryPreviewUILib streamWriter.Close(); // only change when the save is successful - saveButton.IsEnabled = false; + UpdateUnsavedFileState(false); + _updateWindowTitleFunction(_appFileName); } catch (UnauthorizedAccessException ex) { + saveSuccess = false; + // this exception is thrown if the file is there but marked as read only ShowMessageBox( resourceLoader.GetString("ErrorDialogTitle"), @@ -967,6 +1058,8 @@ namespace RegistryPreviewUILib } catch { + saveSuccess = false; + // this catch handles all other exceptions thrown when trying to write the file out ShowMessageBox( resourceLoader.GetString("ErrorDialogTitle"), @@ -984,6 +1077,8 @@ namespace RegistryPreviewUILib // restore the cursor ChangeCursor(gridPreview, false); + + return saveSuccess; } /// diff --git a/src/modules/registrypreview/RegistryPreviewUILib/Strings/en-US/Resources.resw b/src/modules/registrypreview/RegistryPreviewUILib/Strings/en-US/Resources.resw index 54ef71284e..b52dd4b511 100644 --- a/src/modules/registrypreview/RegistryPreviewUILib/Strings/en-US/Resources.resw +++ b/src/modules/registrypreview/RegistryPreviewUILib/Strings/en-US/Resources.resw @@ -258,12 +258,18 @@ Cancel + + No + Save changes? Save + + Yes + Don't save @@ -363,4 +369,7 @@ New + + You lose any unsaved changes. Reload anyway? + \ No newline at end of file