diff --git a/CHANGELOG.md b/CHANGELOG.md index 8aeb7d97e..20b3d9524 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,17 +16,21 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### Changed + - [SIL.Archiving] Upgraded to L10nSharp 7.0.0 - [SIL.Windows.Forms] Upgraded to L10nSharp 7.0.0 - [SIL.Windows.Forms.DblBundle] Upgraded to L10nSharp 7.0.0 - [SIL.Windows.Forms.Keyboarding] Upgraded to L10nSharp 7.0.0 - [SIL.Windows.Forms.WritingSystems] Upgraded to L10nSharp 7.0.0 +- [SIL.Core] `RaiseExceptionIfFailed` no longer throws an exception if user cancelled ## [13.0.1] - 2024-01-09 ### Fixed - [SIL.Core] Fixed bug in extension method GetLongestUsefulCommonSubstring when string ends with an Object replacement character +- [SIL.Core] LogBox: Checked for disposed log box or caller-requested cancel in SafeInvoke so we don't try to write messages or scroll. ## [13.0.0] - 2023-12-07 diff --git a/Palaso.sln.DotSettings b/Palaso.sln.DotSettings index 01cb9abbf..b55cbf111 100644 --- a/Palaso.sln.DotSettings +++ b/Palaso.sln.DotSettings @@ -31,6 +31,7 @@ True True True + True True True True diff --git a/SIL.Core/CommandLineProcessing/CommandLineRunner.cs b/SIL.Core/CommandLineProcessing/CommandLineRunner.cs index dc2231215..df02ee518 100644 --- a/SIL.Core/CommandLineProcessing/CommandLineRunner.cs +++ b/SIL.Core/CommandLineProcessing/CommandLineRunner.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2018 SIL International +// Copyright (c) 2024 SIL International // This software is licensed under the MIT License (http://opensource.org/licenses/MIT) using System; @@ -6,9 +6,11 @@ using System.Globalization; using System.IO; using System.Text; +using JetBrains.Annotations; using SIL.IO; using SIL.PlatformUtilities; using SIL.Progress; +using static System.String; namespace SIL.CommandLineProcessing { @@ -23,7 +25,7 @@ public class CommandLineRunner private Process _process; /// - /// This one doesn't attemtp to influence the encoding used + /// This one doesn't attempt to influence the encoding used /// public static ExecutionResult Run(string exePath, string arguments, string fromDirectory, int secondsBeforeTimeOut, IProgress progress) { @@ -102,9 +104,11 @@ public ExecutionResult Start(string exePath, string arguments, Encoding encoding exePath = "mono"; } progress.WriteVerbose("running '{0} {1}' from '{2}'", exePath, arguments, fromDirectory); - ExecutionResult result = new ExecutionResult(); - result.Arguments = arguments; - result.ExePath = exePath; + ExecutionResult result = new ExecutionResult + { + Arguments = arguments, + ExePath = exePath + }; using (_process = new Process()) { @@ -191,19 +195,24 @@ public ExecutionResult Start(string exePath, string arguments, Encoding encoding } /// - /// On Windows, We can't get unicode over the command-line barrier, so - /// instead create 8.3 filename, which, happily, will have no non-english characters - /// for any part of the path. This is safe to call from Linux, too + /// On Windows, We can't get Unicode over the command-line barrier, so + /// instead create 8.3 filename, which, happily, will have no non-English characters + /// for any part of the path. This is safe to call from Linux, too. /// + [PublicAPI] public static string MakePathToFileSafeFromEncodingProblems(string path) { if (Directory.Exists(path)) - throw new ArgumentException(string.Format("MakePathToFileSafeFromEncodingProblems() is only for files, but {0} is a directory.", path)); + { + throw new ArgumentException( + $"MakePathToFileSafeFromEncodingProblems() is only for files, but {path} is a directory."); + } var safe = ""; - //if the filename doesn't exist yet, we can't get the 8.3 name. So we make it, get the name, then delete it. - //NB: this will not yet deal with the problem of creating a directory + // If the filename doesn't exist yet, we can't get the 8.3 name. So we make it, get + // the name, then delete it. + // NB: this will not yet deal with the problem of creating a directory if (!File.Exists(path)) { File.WriteAllText(path, ""); @@ -223,11 +232,12 @@ public static string MakePathToFileSafeFromEncodingProblems(string path) /// instead create 8.3 filename, which, happily, will have no non-english characters /// for any part of the path. This is safe to call from Linux, too /// + [PublicAPI] public static string MakePathToDirectorySafeFromEncodingProblems(string path) { if (File.Exists(path)) throw new ArgumentException( - string.Format( + Format( "MakePathToDirectorySafeFromEncodingProblems() is only for directories, but {0} is a file.", path)); @@ -251,9 +261,10 @@ public static string MakePathToDirectorySafeFromEncodingProblems(string path) /// - /// Some command line applications (e.g. exiftool) can handle html-encoded characters in their command arguments + /// Some command line applications (e.g. exiftool) can handle HTML-encoded characters in their command arguments /// /// From Rick Strahl at http://www.west-wind.com/weblog/posts/2009/Feb/05/Html-and-Uri-String-Encoding-without-SystemWeb + [PublicAPI] public static string HtmlEncodeNonAsciiCharacters(string text) { if (text == null) @@ -289,17 +300,23 @@ public class ExecutionResult public string StandardOutput; public string ExePath; public string Arguments; - public bool DidTimeOut { get { return ExitCode == kTimedOut; } } - public bool UserCancelled { get { return ExitCode == kCancelled; } } + public bool DidTimeOut => ExitCode == kTimedOut; + public bool UserCancelled => ExitCode == kCancelled; + [PublicAPI] public void RaiseExceptionIfFailed(string contextDescription, params object[] originalPath) { - if (ExitCode == 0) + // REVIEW: This method used to raise a regular ApplicationException in the case of + // UserCancelled. If anything, I assume we would want it to raise a + // UserCancelledException, but since the method name says "IfFailed" and the caller + // should always already know if the user cancelled, not sure there is any need to + // raise an exception in this case. + if (ExitCode == 0 || UserCancelled) return; var builder = new StringBuilder(); - builder.AppendLine(string.Format("{0} {1} failed.", ExePath, Arguments)); - builder.AppendLine("In the context of " + string.Format(contextDescription, originalPath)); + builder.AppendLine($"{ExePath} {Arguments} failed."); + builder.AppendLine("In the context of " + Format(contextDescription, originalPath)); builder.AppendLine("StandardError: " + StandardError); builder.AppendLine("StandardOutput: " + StandardOutput); if (DidTimeOut) @@ -308,7 +325,7 @@ public void RaiseExceptionIfFailed(string contextDescription, params object[] or } } - + [PublicAPI] public class UserCancelledException : ApplicationException { public UserCancelledException() diff --git a/SIL.Core/IO/PathHelper.cs b/SIL.Core/IO/PathHelper.cs index e5c8eb310..2c800e8ff 100644 --- a/SIL.Core/IO/PathHelper.cs +++ b/SIL.Core/IO/PathHelper.cs @@ -122,8 +122,10 @@ static extern uint GetShortPathName( uint cchBuffer); /// - /// When calling external exe's on Windows any non-ascii characters can get converted to '?'. This - /// will convert them to 8.3 format which is all ascii (and do nothing on Linux). + /// When calling external executables on Windows any non-ASCII characters can get converted to '?'. This + /// will convert them to 8.3 format which is all ascii (and do nothing on Linux). Note that this + /// is dependent on the drive being able to generate 8.3 filenames, so this is not guaranteed to produce + /// a short name. /// public static string MakePathSafeFromEncodingProblems(string path) { diff --git a/SIL.Windows.Forms.Tests/Progress/LogBox/LogBoxFormForTest.cs b/SIL.Windows.Forms.Tests/Progress/LogBox/LogBoxFormForTest.cs index e8dc1b009..cdda0157a 100644 --- a/SIL.Windows.Forms.Tests/Progress/LogBox/LogBoxFormForTest.cs +++ b/SIL.Windows.Forms.Tests/Progress/LogBox/LogBoxFormForTest.cs @@ -6,25 +6,19 @@ namespace SIL.Windows.Forms.Tests.Progress.LogBox public class LogBoxFormForTest : IDisposable { private Form _window; - private Windows.Forms.Progress.LogBox _box; + private Forms.Progress.LogBox _box; public LogBoxFormForTest () { CreateWindow(); } - public Windows.Forms.Progress.LogBox progress - { - get - { - return _box; - } - } + public Forms.Progress.LogBox progress => _box; private void CreateWindow() { _window = new Form(); - _box = new Windows.Forms.Progress.LogBox(); + _box = new Forms.Progress.LogBox(); _box.Dock = DockStyle.Fill; _window.Controls.Add(_box); diff --git a/SIL.Windows.Forms.Tests/Progress/LogBox/LogBoxTests.cs b/SIL.Windows.Forms.Tests/Progress/LogBox/LogBoxTests.cs index 5cd4baa01..9b392717a 100644 --- a/SIL.Windows.Forms.Tests/Progress/LogBox/LogBoxTests.cs +++ b/SIL.Windows.Forms.Tests/Progress/LogBox/LogBoxTests.cs @@ -31,6 +31,57 @@ public void ShowLogBox() } } + [Test] + [Category("SkipOnTeamCity")] + public void WriteError_Disposed_IgnoreRequestsToWriteMessages() + { + Console.WriteLine("Showing LogBox"); + using (var e = new LogBoxFormForTest()) + { + progress = e.progress; + progress.WriteMessage("Hi, Mom!"); + progress.Dispose(); + progress.WriteError("Ignore this."); + Assert.IsTrue(progress.ErrorEncountered); + Assert.IsFalse(progress.Rtf.Contains("Ignore this.")); + Assert.IsTrue(progress.Rtf.Contains("Hi, Mom!")); + } + } + + [Test] + [Category("SkipOnTeamCity")] + public void WriteMessage_CancelRequested_HandleRequestsToWriteMessages() + { + Console.WriteLine("Showing LogBox"); + using (var e = new LogBoxFormForTest()) + { + progress = e.progress; + progress.WriteMessage("Hi, Mom!"); + progress.CancelRequested = true; + progress.WriteMessage("Do not ignore this."); + Assert.IsFalse(progress.ErrorEncountered); + Assert.IsTrue(progress.Rtf.Contains("Do not ignore this.")); + Assert.IsTrue(progress.Rtf.Contains("Hi, Mom!")); + } + } + + [Test] + [Category("SkipOnTeamCity")] + public void WriteError_CancelRequested_HandleRequestsToWriteMessages() + { + Console.WriteLine("Showing LogBox"); + using (var e = new LogBoxFormForTest()) + { + progress = e.progress; + progress.WriteMessage("Hi, Mom!"); + progress.CancelRequested = true; + progress.WriteError("User cancelled operation."); + Assert.IsTrue(progress.ErrorEncountered); + Assert.IsTrue(progress.Rtf.Contains("Hi, Mom!")); + Assert.IsTrue(progress.Rtf.Contains("User cancelled operation.")); + } + } + [Test] [Explicit] [Category("LongRunning")] diff --git a/SIL.Windows.Forms/Extensions/ControlExtensions.cs b/SIL.Windows.Forms/Extensions/ControlExtensions.cs index b236ca00b..08658a759 100644 --- a/SIL.Windows.Forms/Extensions/ControlExtensions.cs +++ b/SIL.Windows.Forms/Extensions/ControlExtensions.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.ComponentModel; using System.Runtime.InteropServices; using System.Windows.Forms; @@ -129,7 +129,7 @@ public static IAsyncResult SafeInvoke(this Control control, Action action, strin if (!control.InvokeRequired) { - if (control.IsDisposed) + if (control.IsDisposed || control.Disposing) { if (errorHandling == ErrorHandlingAction.Throw) throw new ObjectDisposedException("SafeInvoke called after the control was disposed. (" + nameForErrorReporting + ")"); diff --git a/SIL.Windows.Forms/Progress/LogBox.cs b/SIL.Windows.Forms/Progress/LogBox.cs index a373eba7d..dee4fc29e 100644 --- a/SIL.Windows.Forms/Progress/LogBox.cs +++ b/SIL.Windows.Forms/Progress/LogBox.cs @@ -418,15 +418,11 @@ private void WriteErrorInternal(string message, params object[] args) { Write(Color.Red, _box.Font.Style, message, args); - // There is no Invoke method on ToolStripItems (which the _reportLink is) - // and setting it to visible from another thread seems to work okay. - _reportLink.Visible = true; - - menuStrip1.Invoke(new Action(() => + SafeInvoke(menuStrip1, () => { + _reportLink.Visible = true; menuStrip1.Visible = true; - })); - + }); ErrorEncountered = true; }