Skip to content

Conversation

rmanosuthi
Copy link

Hello,

This commit makes using a Pager to display help text optional.

HelpUsePager is initialized to true as to not break default behaviour.

Re: Test - I'm not sure how to write a test for this since the value is always initialized and ShowHelp then refers to that value. Assert.True(app.HelpUsePager)?

Copy link
Owner

@natemcmaster natemcmaster left a comment

Choose a reason for hiding this comment

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

Suggested slightly different naming. Don't worry about adding tests, this seems straightforward.

Also changed ShowHelp() to refer to UsePagerForHelpText instead

Co-Authored-By: mpipo <[email protected]>
@natemcmaster natemcmaster merged commit 5442748 into natemcmaster:master Feb 22, 2019
@natemcmaster natemcmaster added this to the 2.3.3 milestone Feb 22, 2019
@natemcmaster
Copy link
Owner

Once this build completes https://dev.azure.com/natemcmaster/github/_build/results?buildId=160, there should be a daily build of this package published here: https://myget.org/feed/natemcmaster/package/nuget/McMaster.Extensions.CommandLineUtils

I'm going to wait 2 weeks to see if any other PRs come in for patch changes before releasing this to nuget.org

@simonech
Copy link

simonech commented Mar 3, 2019

Nice... I was getting annoyed of that pager popping up all time for help. Why not making it false by default? It's the first time I see such behaviour in a cli.

@rmanosuthi
Copy link
Author

You're right, not many programs have a similar feature, but I didn't want to change the default behaviour (of using the pager) or make assumptions about what people use/like.

The option's there now though 😃

@simonech
Copy link

simonech commented Mar 5, 2019

@mpipo indeed... well.. technically not there yet... until new version is released 😄

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.

3 participants