Skip to content

Add the missing help messages for some switches - #9719

Merged
YuliiaKovalova merged 21 commits into
mainfrom
dev/jennybai/issue9710
Jun 24, 2024
Merged

Add the missing help messages for some switches#9719
YuliiaKovalova merged 21 commits into
mainfrom
dev/jennybai/issue9710

Conversation

@JaynieBai

@JaynieBai JaynieBai commented Feb 8, 2024

Copy link
Copy Markdown
Member

Fixes #9710

Context

Some of the supported command line switches are not mentioned in the help message

Changes Made

List the relevant string resource ID in s_parameterlessSwitchesMap / s_parameterizedSwitchesMap. And iterate the resource ids in the ShowHelpMessage

Testing

Before:
msbuildBefore.txt

dotnetmsbuildBefore.txt

After:

MSBuildAfter.txt

netMSBuildAfter.txt

Notes

Comment thread src/MSBuild/Resources/Strings.resx Outdated
Comment thread src/MSBuild/Resources/Strings.resx Outdated
Comment thread src/MSBuild/CommandLineSwitches.cs
Comment thread src/MSBuild/CommandLineSwitches.cs Outdated
Comment thread src/MSBuild/XMake.cs Outdated
@JaynieBai
JaynieBai marked this pull request as ready for review March 13, 2024 04:34

@JanKrivanek JanKrivanek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good.

I agree that getTargetResult should probably specify that the specified target will be executed.

Comment thread src/MSBuild/Resources/Strings.resx Outdated
Comment thread src/MSBuild/Resources/Strings.resx Outdated
Comment thread src/MSBuild/Resources/Strings.resx Outdated
@JaynieBai

Copy link
Copy Markdown
Member Author

Looks good.

I agree that getTargetResult should probably specify that the specified target will be executed.

Thanks, have updated the help message for this property.

@YuliiaKovalova
YuliiaKovalova requested a review from ladipro April 2, 2024 11:24
@YuliiaKovalova
YuliiaKovalova requested a review from Forgind April 2, 2024 11:32

@ladipro ladipro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you, I've left a few comments inline. For testing, can you please compare the help output before and after, and from both MSBuild.exe and dotnet build?

Comment thread src/MSBuild/CommandLineSwitches.cs Outdated
Comment thread src/MSBuild/CommandLineSwitches.cs Outdated
Comment thread src/MSBuild/CommandLineSwitches.cs Outdated
Comment thread src/MSBuild/XMake.cs Outdated
Comment thread src/MSBuild/XMake.cs Outdated
@JaynieBai

Copy link
Copy Markdown
Member Author

Thank you, I've left a few comments inline. For testing, can you please compare the help output before and after, and from both MSBuild.exe and dotnet build?

I upload the text based on test. There is a change in the order of the switches. Please have a look if that is acceptable. -help is earlier than other no parameters switches since the

{
//---------------------------------------------------------------------------------------------------------------------------------------------------
// Switch Names Switch Id Dup Error Light up key
//---------------------------------------------------------------------------------------------------------------------------------------------------
new ParameterlessSwitchInfo( new string[] { "help", "h", "?" }, ParameterlessSwitch.Help, null),
new ParameterlessSwitchInfo( new string[] { "version", "ver" }, ParameterlessSwitch.Version, null),
new ParameterlessSwitchInfo( new string[] { "nologo" }, ParameterlessSwitch.NoLogo, null),
new ParameterlessSwitchInfo( new string[] { "noautoresponse", "noautorsp" }, ParameterlessSwitch.NoAutoResponse, null),
new ParameterlessSwitchInfo( new string[] { "noconsolelogger", "noconlog" }, ParameterlessSwitch.NoConsoleLogger, null),
new ParameterlessSwitchInfo( new string[] { "filelogger", "fl" }, ParameterlessSwitch.FileLogger, null),
new ParameterlessSwitchInfo( new string[] { "filelogger1", "fl1" }, ParameterlessSwitch.FileLogger1, null),
new ParameterlessSwitchInfo( new string[] { "filelogger2", "fl2" }, ParameterlessSwitch.FileLogger2, null),
new ParameterlessSwitchInfo( new string[] { "filelogger3", "fl3" }, ParameterlessSwitch.FileLogger3, null),
new ParameterlessSwitchInfo( new string[] { "filelogger4", "fl4" }, ParameterlessSwitch.FileLogger4, null),
new ParameterlessSwitchInfo( new string[] { "filelogger5", "fl5" }, ParameterlessSwitch.FileLogger5, null),
new ParameterlessSwitchInfo( new string[] { "filelogger6", "fl6" }, ParameterlessSwitch.FileLogger6, null),
new ParameterlessSwitchInfo( new string[] { "filelogger7", "fl7" }, ParameterlessSwitch.FileLogger7, null),
new ParameterlessSwitchInfo( new string[] { "filelogger8", "fl8" }, ParameterlessSwitch.FileLogger8, null),
new ParameterlessSwitchInfo( new string[] { "filelogger9", "fl9" }, ParameterlessSwitch.FileLogger9, null),
new ParameterlessSwitchInfo( new string[] { "distributedfilelogger", "dfl" }, ParameterlessSwitch.DistributedFileLogger, null),
#if DEBUG
new ParameterlessSwitchInfo( new string[] { "waitfordebugger", "wfd" }, ParameterlessSwitch.WaitForDebugger, null),
#endif
};

image

@ladipro ladipro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I upload the text based on test. There is a change in the order of the switches. Please have a look if that is acceptable.

I can't imagine something would depend on the order. Reviewers, please chime in. I've left a few inline comments. Thank you!

Comment thread src/MSBuild/Resources/Strings.resx Outdated
Comment thread src/MSBuild/Resources/Strings.resx Outdated
Comment thread src/MSBuild/Resources/Strings.resx Outdated
Comment thread src/MSBuild/Resources/Strings.resx Outdated
Comment thread src/MSBuild/Resources/Strings.resx Outdated
Comment thread src/MSBuild/Resources/Strings.resx Outdated
Comment thread src/MSBuild/XMake.cs Outdated
Comment thread src/MSBuild/XMake.cs Outdated
Comment thread src/MSBuild/Resources/Strings.resx Outdated
Comment thread src/MSBuild/Resources/Strings.resx Outdated
Comment thread src/MSBuild/Resources/Strings.resx Outdated
Comment thread src/MSBuild/Resources/Strings.resx Outdated
Comment thread src/MSBuild/Resources/Strings.resx Outdated
Comment thread src/MSBuild/Resources/Strings.resx Outdated
Comment thread src/MSBuild/Resources/Strings.resx Outdated
@JaynieBai

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@JaynieBai

Copy link
Copy Markdown
Member Author

There are conflicts since the new analyzer switch in the commit f744eb5#diff-c0f68f9abf837e0ecb1e4de52b83e9e81d6934c5596bc213e4b1de45e591d7ce But there is no help message for this analyzer switch. @JanKrivanek , Could you give more info about this switch? Should we show the help message for this switch?

@JanKrivanek

Copy link
Copy Markdown
Member

There are conflicts since the new analyzer switch in the commit f744eb5#diff-c0f68f9abf837e0ecb1e4de52b83e9e81d6934c5596bc213e4b1de45e591d7ce But there is no help message for this analyzer switch. @JanKrivanek , Could you give more info about this switch? Should we show the help message for this switch?

Disccussed offline.
-analyze should not be included in help message now

@JaynieBai
JaynieBai requested a review from maridematte May 22, 2024 08:04
Comment thread src/MSBuild/Resources/Strings.resx Outdated
@YuliiaKovalova
YuliiaKovalova merged commit c028e44 into main Jun 24, 2024
@YuliiaKovalova
YuliiaKovalova deleted the dev/jennybai/issue9710 branch June 24, 2024 12:03
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.

[Bug]: Help message does not contain some switches (-getProperty, -getTargetResult)

8 participants