Conditionally trim trailing periods of argument and option descriptions

[TheTonttu]

fixes #1729

• I have read the Contribution Guidelines
• I have commented on the issue above and discussed the intended changes
• A maintainer has signed off on the changes and the issue was assigned to me
• All newly added code is adequately covered by tests
• All existing tests are still running without errors
• The documentation was modified to reflect the changes OR no documentation changes are required.

Changes

• HelpProvider.TrimTrailingPeriod property affects trailing period of argument and option descriptions in addition to command descriptions.
• Related test case output includes argument and option description in addition to command description.
  • The test case (Should_Not_Trim_Description_Trailing_Period) expectation text (Description_No_Trailing_Period) is a bit confusing/conflicting but I didn't touch it to keep changes minimal.
• Localized print help and print version descriptions include trailing period.
  • Default help output stays the same but when the trimming is disabled the period is added to the output.

---

Please upvote 👍 this pull request if you are interested in it.

[TheTonttu]

@microsoft-github-policy-service agree

[FrankRay78]

Thanks, I'll try to review this one over the weekend.

[FrankRay78]

Hello @TheTonttu, thank you for the contribution. My brain isn't fully functional today, however it looks like there is one more change required in this PR.

Take one of the expectations, say Default_Without_Args_Additional.Output_EN.verified.txt, and it has the following inconsistent line endings for the description:

I believe this line needs normalising too:

Can you please check and see if you concur? If so, make the change and repush.

[FrankRay78]

PS. @TheTonttu I plan to review/merge this #1717 soon, so if it's before your next push, you'll need to rebase and update accordingly.

[TheTonttu]

Good catch, I'll make an update soon™.

[TheTonttu]

The change affected quite many test cases.

[FrankRay78]

The change affected quite many test cases.

It looks ok to me, ICommandAppSettings does have the following description (in main, prior to your PR):

    /// <summary>
    /// Gets or sets a value indicating whether a trailing period of a command description is trimmed in the help text.
    /// </summary>
    bool TrimTrailingPeriod { get; set; }

Prior to your PR, the actual command description wouldn't ever be trimmed!

I just want to triple check with one of our other maintainers over the weekend first...

[FrankRay78]

Note to self: The more I think about the idea of TrimTrailingPeriod, the more I dislike it and the more I want to remove it.

The trim only ever happens on text entered by the Spectre.Console application developer, and it's compile time text rather than dynamic somehow (ie. it's known in advance) - so why on earth do we have a property to offer to trim their explicitly entered text? If they want it trimmed, then simply don't use a period at the end.

Furthermore, the trim is defaulting to true, so new users of Spectre.Console are faced with help displaying their own text but with trailing periods trimmed, which likely isn't obvious why and so seems strange default behaviour.

I think I'm in favour of marking the property as obsolete and also changing its default to false, which is hardly a breaking change. I need to give it a little more thought and look back through the git history.