Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Basic Narg Functionality #71

Closed
wants to merge 7 commits into from
Closed

Basic Narg Functionality #71

wants to merge 7 commits into from

Conversation

alanxoc3
Copy link

I needed Narg functionality in my application, and I was able to achieve what I needed with a few minor, backwards compatible changes to the library. This doesn't have to be merged, but it might help bring some good ideas to the discussion (I noticed #20, #22, and #38).

Also, my application doesn't use sub commands, so that may be why this change works well for me.

@@ -27,6 +27,7 @@ type Arg interface {
GetOpts() *Options
GetSname() string
GetLname() string
IsFlag() bool
Copy link
Author

@alanxoc3 alanxoc3 Jun 21, 2020

Choose a reason for hiding this comment

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

This IsFlag line is unrelated to the topic of the pull request, but it was useful for me to get a nice looking help menu. Example:

Options:
  -h  --help      Print help information.
  -V  --version   Concards build information..
  -r  --review    Show cards available to be reviewed.
  -m  --memorize  Show cards available to be memorized.
  -d  --done      Show cards not available to be reviewed or memorized.
  -p  --print     Prints all cards, one line per card.
  -n  --number n  How many cards to review.
  -E  --editor E  Which editor to use. Defaults to "$EDITOR".
  -M  --meta M    Path to meta file. Defaults to "$CONCARDS_META" or "~/.concards-meta".

In my custom help menu function, if the argument is a flag, I'm not printing an argument name. And conversely, if the argument is not a flag, there is an argument name (same as the short name).

Copy link
Owner

Choose a reason for hiding this comment

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

That should be its own PR please. 1 PR == 1 functionality change.

Also that is not really common way for building help message. Arguably there are no standards of doing this per se. But there are recognized common formats for help messages and that wouldn't be it.

@@ -41,6 +42,10 @@ func (o arg) GetLname() string {
return o.lname
}

func (o arg) IsFlag() bool {
return o.size == 1
Copy link
Owner

Choose a reason for hiding this comment

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

o.size does not mean it is a flag. flag would be a type of the interface successfully cast to boolean.

@akamensky
Copy link
Owner

I am confused here, the top post mentions 3 issues. 2 of them talk about positional arguments, another about nargs. I should probably note -- positional arguments != nargs.

This change is missing any test cases (therefore failure of code coverage checks).

@alanxoc3 alanxoc3 marked this pull request as draft June 24, 2020 01:00
@stale
Copy link

stale bot commented Jan 15, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed soon if no further activity occurs. Thank you for your contributions. Feel free to comment or otherwise update to keep it alive.

@stale stale bot added the stale-issue The issue has been stale for some time. label Jan 15, 2021
@stale
Copy link

stale bot commented Jan 29, 2021

Closing due to old age. Feel free to re-open or ping maintainers.

@stale stale bot closed this Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale-issue The issue has been stale for some time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants