@joebigelow / wix-1 / commits / baf09c19

Reduce duplicative messaging in converter

Fixes 6681

Rob Mensching committed Mar 5, 2022 at 11:59 UTC baf09c19c5a0f0d3f9533f9084f094066c1be7d9
2 files changed +27 -35
src/wix/WixToolset.Converters/WixConverter.cs
+20 -35
@@ -15,7 +15,6 @@ namespace WixToolset.Converters
15 using WixToolset.Data.WindowsInstaller;
16 using WixToolset.Extensibility.Services;
17
18 -#pragma warning disable 1591 // TODO: add documentation
18 /// <summary>
19 /// How to convert CustomTable elements.
20 /// </summary>
@@ -295,7 +294,7 @@ namespace WixToolset.Converters
294
295 private int SourceVersion { get; set; }
296
298 - public XElement XRoot { get; private set; }
297 + private XElement XRoot { get; set; }
298
299 /// <summary>
300 /// Convert a file.
@@ -2085,31 +2084,7 @@ namespace WixToolset.Converters
2084 /// <returns>Returns true indicating that action should be taken on this error, and false if it should be ignored.</returns>
2085 private bool OnError(ConverterTestType converterTestType, XObject node, string message, params object[] args)
2086 {
2088 - // Ignore the error if explicitly ignored or outside the range of the current operation.
2089 - if (this.IgnoreErrors.Contains(converterTestType) ||
2090 - (this.Operation == ConvertOperation.Convert && converterTestType < ConverterTestType.EndIgnoreInConvert) ||
2091 - (this.Operation == ConvertOperation.Format && converterTestType > ConverterTestType.BeginIgnoreInFormat))
2092 - {
2093 - return false;
2094 - }
2095 -
2096 - // Increase the message count.
2097 - this.Messages++;
2098 -
2099 - var sourceLine = (null == node) ? new SourceLineNumber(this.SourceFile ?? "wix.exe") : new SourceLineNumber(this.SourceFile, ((IXmlLineInfo)node).LineNumber);
2100 - var warning = this.ErrorsAsWarnings.Contains(converterTestType);
2101 -
2102 - var msg = new Message(
2103 - sourceLine,
2104 - warning ? MessageLevel.Warning : MessageLevel.Error,
2105 - (int)converterTestType,
2106 - "{0} ({1})",
2107 - String.Format(CultureInfo.CurrentCulture, message, args),
2108 - converterTestType.ToString());
2109 -
2110 - this.Messaging.Write(msg);
2111 -
2112 - return true;
2087 + return this.OnMessage(MessageLevel.Error, converterTestType, node, message, args);
2088 }
2089
2090 /// <summary>
@@ -2119,8 +2094,13 @@ namespace WixToolset.Converters
2094 /// <param name="node">The node that caused the error.</param>
2095 /// <param name="message">Detailed error message.</param>
2096 /// <param name="args">Additional formatted string arguments.</param>
2122 - /// <returns>Returns true indicating that action should be taken on this error, and false if it should be ignored.</returns>
2097 + /// <returns>Returns true indicating that action should be taken on this message, and false if it should be ignored.</returns>
2098 private bool OnInformation(ConverterTestType converterTestType, XObject node, string message, params object[] args)
2099 + {
2100 + return this.OnMessage(MessageLevel.Information, converterTestType, node, message, args);
2101 + }
2102 +
2103 + private bool OnMessage(MessageLevel level, ConverterTestType converterTestType, XObject node, string message, params object[] args)
2104 {
2105 // Ignore the error if explicitly ignored or outside the range of the current operation.
2106 if (this.IgnoreErrors.Contains(converterTestType) ||
@@ -2134,14 +2114,19 @@ namespace WixToolset.Converters
2114 this.Messages++;
2115
2116 var sourceLine = (null == node) ? new SourceLineNumber(this.SourceFile ?? "wix.exe") : new SourceLineNumber(this.SourceFile, ((IXmlLineInfo)node).LineNumber);
2117 + var prefix = String.Empty;
2118 + if (level == MessageLevel.Information)
2119 + {
2120 + prefix = "[Converted] ";
2121 + }
2122 + else if (level == MessageLevel.Error && this.ErrorsAsWarnings.Contains(converterTestType))
2123 + {
2124 + level = MessageLevel.Warning;
2125 + }
2126 +
2127 + var format = prefix + message + $" ({converterTestType})";
2128
2138 - var msg = new Message(
2139 - sourceLine,
2140 - MessageLevel.Information,
2141 - (int)converterTestType,
2142 - "[Converted] {0} ({1})",
2143 - String.Format(CultureInfo.CurrentCulture, message, args),
2144 - converterTestType.ToString());
2129 + var msg = new Message(sourceLine, level, (int)converterTestType, format, args);
2130
2131 this.Messaging.Write(msg);
2132
src/wix/test/WixToolsetTest.Converters/ExePackageFixture.cs
+7
@@ -3,6 +3,7 @@
3 namespace WixToolsetTest.Converters
4 {
5 using System;
6 + using System.Linq;
7 using System.Xml.Linq;
8 using WixBuildTools.TestSupport;
9 using WixToolset.Converters;
@@ -43,6 +44,12 @@ namespace WixToolsetTest.Converters
44 Assert.Equal(3, errors);
45
46 var actualLines = UnformattedDocumentLines(document);
47 + WixAssert.CompareLineByLine(new[]
48 + {
49 + "[Converted] The ExePackage element InstallCommand attribute has been renamed InstallArguments. (RenameExePackageCommandToArguments)",
50 + "[Converted] The ExePackage element RepairCommand attribute has been renamed RepairArguments. (RenameExePackageCommandToArguments)",
51 + "[Converted] The ExePackage element UninstallCommand attribute has been renamed UninstallArguments. (RenameExePackageCommandToArguments)",
52 + }, messaging.Messages.Select(m => m.ToString()).ToArray());
53 WixAssert.CompareLineByLine(expected, actualLines);
54 }
55 }