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

Add option to select default choices #42

Merged
merged 6 commits into from
Apr 5, 2018

Conversation

fnky
Copy link
Contributor

@fnky fnky commented Apr 3, 2018

This PR adds a default option to allow certain items in a list to be selected by default.

For a single-choice list, it will select the given item defined in default. For a multi-choice list, it will select the given choices separated by space ( ) within quotations.

This is especially helpful in for example selecting the current branch in a list of branches:

$ git branch -a | ipt -D "$(git rev-parse --abbrev-ref HEAD)"

or for example auto-selecting already merged branches:

$ git branch -a | ipt -m -D "$(git branch -a --merged ${1-master} | xargs)"

src/index.js Outdated

if (promptType.type === 'checkbox') {
return options.default
.split(' ')
Copy link
Owner

Choose a reason for hiding this comment

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

-				.split(' ')
+				.split(options['default-separator'] || options.separator)

I'd much rather have a default-separator option for people to be able to customize and default to separator

it would also remove the need for the extra | xargs from your example 😊

Copy link
Owner

Choose a reason for hiding this comment

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

PS: it would also mean we would need to enforce a default value for options.separator in src/cli.js

right now it can be undefined

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, I'll make some changes and add this extra option :)

Copy link
Contributor Author

@fnky fnky Apr 5, 2018

Choose a reason for hiding this comment

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

I think it would make more sense if the default value of the separator is similar to this line using os.EOL (e.g. \n in Unix).

This way you can easily pass file contents, separated by new lines and won't require you to always specify default-separator in such occasions.

Copy link
Contributor Author

@fnky fnky Apr 5, 2018

Choose a reason for hiding this comment

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

Also, do you think it is necessary with an alias? There's no letters that matches with the naming default-separator. I tried using P, but it seemed too unclear.

@ruyadorno
Copy link
Owner

hi @fnky many thanks for taking the time! Really impressive contribution! I didn't knew how much I want to have a default option until I saw this 😂

One consideration, I actually checked out the code to try it out locally and for me the first example wasn't working at first since git branch -a preppends a * to the current branch name (I wonder if that kind of mismatching can be annoying when using the default option? I think a possible solution would be to sanitize the input first 🤔 ) , anyways for me this quick change made it work:

-$ git branch -a | ipt -D "$(git rev-parse --abbrev-ref HEAD)"
+$ git branch -a | ipt -D "* $(git rev-parse --abbrev-ref HEAD)"

@ruyadorno
Copy link
Owner

I couldn't get the multiple options example to work though 😞 I wonder if we have mismatching git versions 🤔 I have:

git --version
git version 2.15.0

here, what about you?

@fnky
Copy link
Contributor Author

fnky commented Apr 5, 2018

first example wasn't working at first since git branch -a preppends a * to the current branch name

Ah, I thought it was something to do with my git config. But you can use --format "%(refname:short)" to avoid the default formatting. A bit cumbersome, but with aliases this could be make it less verbose.

This is what I tested with originally:

Select a single choice:

git branch -a --format "%(refname:short)" | ipt -m -D "master"

Select multiple choices:

git branch -a --format "%(refname:short)" | ipt -m -D "$(git branch -a --merged ${1-master} --format "%(refname:short)" | xargs)"

I am also using git 2.15.0

git --version
git version 2.15.0

@fnky
Copy link
Contributor Author

fnky commented Apr 5, 2018

I have added the default-separator option, which falls back to separator, otherwise to os.EOL. This allows you to easily input files, delimited by new lines:

$ echo "foo\nbar\nbaz" | ipt -m -D "$(echo "foo\nbaz")"

Note that if you pass newlines directly, it won't work:

$ echo "foo\nbar\nbaz" | ipt -m -D "foo\nbaz"

To use newline separator directly, use default-separator:

$ echo "foo\nbar\nbaz" | ipt -m --default-separator="\n" -D "foo\nbaz"

@@ -31,6 +31,13 @@ const { argv } = yargs
.describe("c", "Copy selected item(s) to clipboard")
.alias("d", "debug")
.describe("d", "Prints to stderr any internal error")
.alias("D", "default")
.describe("D", "Select a default choices by their name")
.alias("P", "default-separator")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know whether it makes sense to have an alias for default-separator.


if (promptType.type === "checkbox") {
return options.default.split(
options["default-separator"] || options.separator || os.EOL
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This falls back to separator, otherwise os.EOL, similar to this line.

@ruyadorno
Copy link
Owner

oh yeah!!! works like a charm now 😍

MANY MANY THANKS! this is the best contribution I ever received in an OSS project 😊

@ruyadorno ruyadorno merged commit 90647fd into ruyadorno:master Apr 5, 2018
@ruyadorno
Copy link
Owner

@fnky I would also suggest you to share reusable workflows as npm packages for easy-sharing across the community 😊

Here are some simple examples I put together, feel free to use them as templates:

@fnky
Copy link
Contributor Author

fnky commented Apr 5, 2018

My pleasure, love to help out! Thanks for the workflow suggestions, I’ll take a look when I have the time!

Cheers 🍺

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.

None yet

2 participants