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

everything #1

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

everything #1

wants to merge 13 commits into from

Conversation

davidchambers
Copy link
Member

This pull request has been some time coming. I wrote the code two years ago and submitted davidchambers/transcribe#28, then decided that the code should live in its own package. This is that package. :)

Copy link
Member

@Avaq Avaq left a comment

Choose a reason for hiding this comment

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

Cool design! I'd love to use this in my next CLI. A few thoughts:

index.js Outdated
Comment on lines 22 to 29
//. cut -s -d : -f 1 -- file1 file2 file3 = command
//. -s -d : -f 1 -- file1 file2 file3 = arguments
//. -s = flags = flag names
//. -d : -f 1 = options
//. -d -f = option names
//. : 1 = option values
//. -- = separator
//. file1 file2 file3 = positional arguments
Copy link
Member

@Avaq Avaq Sep 7, 2021

Choose a reason for hiding this comment

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

It took me a while to understand what's going on here. What am I looking at? What the significance of the cut command in relation to terminology? How do I read the diagram being painted here?

I finally got that you're dissecting an invocation of the cut command for the purposes of showing how you'd refer to each segment, with each line kind of being an entry in the dictionary but with the definition on the left.

Maybe a short explanation prior to this dictionary might help readers.

Perhaps more space before the = sign might have also helped me parse it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated this section. Please have another look. :)

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
test/index.js Outdated Show resolved Hide resolved
README.md Outdated
Comment on lines 83 to 84
- The argument is a (short/long) flag/option name. It is looked up in the
specification. There are three possibilities:
Copy link
Member Author

Choose a reason for hiding this comment

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

This is no longer accurate now that we permit -xyz. I will update this section.

Copy link
Member

Choose a reason for hiding this comment

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

Have you updated that section?

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@dotnetCarpenter
Copy link

dotnetCarpenter commented Sep 10, 2021

@davidchambers really looking forward to ditching prompt and yargs for this! I should clarify, that I don't expect this to substitute prompt but hopefully it should be fairly easy to implement our own prompt based on sanctuary-argv.

@davidchambers davidchambers requested a review from Avaq February 22, 2023 15:56
@davidchambers davidchambers force-pushed the davidchambers/everything branch from b49bea5 to 1696727 Compare February 24, 2023 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants