Skip to content
This repository has been archived by the owner on Jun 6, 2022. It is now read-only.

Support for installing addons from search results #58

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

oreomanfan
Copy link

Simple user prompt

image

@antiwinter
Copy link
Owner

If you like this feature, I'd like to suggest adding a switch to allow this prompt. (because wowa was designed to behave like the no-prompt style)

switch, indicates interactive mode

wowa search -i bagon

And also I suggest changing the prompt sentence

from

Install one or more of these?
Enter ....
1 2 4

to

Quick install by inputting indexes (separate by ',' or space):
1,2,4

Here you must allow user to Ctrl-C to exit without any error prompt

@antiwinter
Copy link
Owner

This is possibly caused by the line feed character. The original was LF, you may have accidentally changed it to CRLF.

One more thing is core.search() is not only invoked by index.js but also invoked by test.js. You will want to change that file too.

@antiwinter
Copy link
Owner

This is possibly caused by the line feed character. The original was LF, you may have accidentally changed it to CRLF.

If you use vscode, check the bottom right corner.

@oreomanfan
Copy link
Author

One more thing is core.search() is not only invoked by index.js but also invoked by test.js. You will want to change that file too.

I'm not sure why, but changing the condition that runs the new install code from search from if (i) to if (i === true) fixed the tests hanging. I don't know why they did that in the first place, because none of the tests should trigger the new install code because they don't use the -i option. Clearly I'm too junior to be trying this lol.

@antiwinter
Copy link
Owner

It's not recommended to use i as a function argument, as i is more likely to be used as a local variable. And you still need to change the line feed of index.js from CRLF back to LF, or else the code cannot be reviewed.

You can manually run the test by yarn run test before committing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants