From baf09c19c5a0f0d3f9533f9084f094066c1be7d9 Mon Sep 17 00:00:00 2001 From: Rob Mensching Date: Sat, 5 Mar 2022 11:59:14 -0800 Subject: Reduce duplicative messaging in converter Fixes 6681 --- src/wix/WixToolset.Converters/WixConverter.cs | 55 ++++++++-------------- .../WixToolsetTest.Converters/ExePackageFixture.cs | 7 +++ 2 files changed, 27 insertions(+), 35 deletions(-) diff --git a/src/wix/WixToolset.Converters/WixConverter.cs b/src/wix/WixToolset.Converters/WixConverter.cs index 183a7394..c009042d 100644 --- a/src/wix/WixToolset.Converters/WixConverter.cs +++ b/src/wix/WixToolset.Converters/WixConverter.cs @@ -15,7 +15,6 @@ namespace WixToolset.Converters using WixToolset.Data.WindowsInstaller; using WixToolset.Extensibility.Services; -#pragma warning disable 1591 // TODO: add documentation /// /// How to convert CustomTable elements. /// @@ -295,7 +294,7 @@ namespace WixToolset.Converters private int SourceVersion { get; set; } - public XElement XRoot { get; private set; } + private XElement XRoot { get; set; } /// /// Convert a file. @@ -2085,31 +2084,7 @@ namespace WixToolset.Converters /// Returns true indicating that action should be taken on this error, and false if it should be ignored. private bool OnError(ConverterTestType converterTestType, XObject node, string message, params object[] args) { - // Ignore the error if explicitly ignored or outside the range of the current operation. - if (this.IgnoreErrors.Contains(converterTestType) || - (this.Operation == ConvertOperation.Convert && converterTestType < ConverterTestType.EndIgnoreInConvert) || - (this.Operation == ConvertOperation.Format && converterTestType > ConverterTestType.BeginIgnoreInFormat)) - { - return false; - } - - // Increase the message count. - this.Messages++; - - var sourceLine = (null == node) ? new SourceLineNumber(this.SourceFile ?? "wix.exe") : new SourceLineNumber(this.SourceFile, ((IXmlLineInfo)node).LineNumber); - var warning = this.ErrorsAsWarnings.Contains(converterTestType); - - var msg = new Message( - sourceLine, - warning ? MessageLevel.Warning : MessageLevel.Error, - (int)converterTestType, - "{0} ({1})", - String.Format(CultureInfo.CurrentCulture, message, args), - converterTestType.ToString()); - - this.Messaging.Write(msg); - - return true; + return this.OnMessage(MessageLevel.Error, converterTestType, node, message, args); } /// @@ -2119,8 +2094,13 @@ namespace WixToolset.Converters /// The node that caused the error. /// Detailed error message. /// Additional formatted string arguments. - /// Returns true indicating that action should be taken on this error, and false if it should be ignored. + /// Returns true indicating that action should be taken on this message, and false if it should be ignored. private bool OnInformation(ConverterTestType converterTestType, XObject node, string message, params object[] args) + { + return this.OnMessage(MessageLevel.Information, converterTestType, node, message, args); + } + + private bool OnMessage(MessageLevel level, ConverterTestType converterTestType, XObject node, string message, params object[] args) { // Ignore the error if explicitly ignored or outside the range of the current operation. if (this.IgnoreErrors.Contains(converterTestType) || @@ -2134,14 +2114,19 @@ namespace WixToolset.Converters this.Messages++; var sourceLine = (null == node) ? new SourceLineNumber(this.SourceFile ?? "wix.exe") : new SourceLineNumber(this.SourceFile, ((IXmlLineInfo)node).LineNumber); + var prefix = String.Empty; + if (level == MessageLevel.Information) + { + prefix = "[Converted] "; + } + else if (level == MessageLevel.Error && this.ErrorsAsWarnings.Contains(converterTestType)) + { + level = MessageLevel.Warning; + } + + var format = prefix + message + $" ({converterTestType})"; - var msg = new Message( - sourceLine, - MessageLevel.Information, - (int)converterTestType, - "[Converted] {0} ({1})", - String.Format(CultureInfo.CurrentCulture, message, args), - converterTestType.ToString()); + var msg = new Message(sourceLine, level, (int)converterTestType, format, args); this.Messaging.Write(msg); diff --git a/src/wix/test/WixToolsetTest.Converters/ExePackageFixture.cs b/src/wix/test/WixToolsetTest.Converters/ExePackageFixture.cs index 0ee8d065..68d234ad 100644 --- a/src/wix/test/WixToolsetTest.Converters/ExePackageFixture.cs +++ b/src/wix/test/WixToolsetTest.Converters/ExePackageFixture.cs @@ -3,6 +3,7 @@ namespace WixToolsetTest.Converters { using System; + using System.Linq; using System.Xml.Linq; using WixBuildTools.TestSupport; using WixToolset.Converters; @@ -43,6 +44,12 @@ namespace WixToolsetTest.Converters Assert.Equal(3, errors); var actualLines = UnformattedDocumentLines(document); + WixAssert.CompareLineByLine(new[] + { + "[Converted] The ExePackage element InstallCommand attribute has been renamed InstallArguments. (RenameExePackageCommandToArguments)", + "[Converted] The ExePackage element RepairCommand attribute has been renamed RepairArguments. (RenameExePackageCommandToArguments)", + "[Converted] The ExePackage element UninstallCommand attribute has been renamed UninstallArguments. (RenameExePackageCommandToArguments)", + }, messaging.Messages.Select(m => m.ToString()).ToArray()); WixAssert.CompareLineByLine(expected, actualLines); } } -- cgit v1.2.3-55-g6feb