Skip to content

Conversation

@YoshiMan
Copy link

I used shellcheck to validate the script.
see the commits

added new countries

used shellcheck to find issues.
https://github.com/koalaman/shellcheck
fixed SC2128: Expanding an array without an index only gives the first element.
https://github.com/koalaman/shellcheck/wiki/SC2128
https://github.com/koalaman/shellcheck
fixed SC2116: Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'
https://github.com/koalaman/shellcheck/wiki/SC2116

query, code and country variable to an array

changed case statement
shellcheck
fixed SC2162: read without -r will mangle backslashes.
https://github.com/koalaman/shellcheck/wiki/SC2162
shellcheck fixed 
SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
https://github.com/koalaman/shellcheck/wiki/SC2181
added support for 'protocol' parameter
added support for 'ip_version' parameter
added support for 'use_mirror_status' parameter
@YoshiMan YoshiMan changed the title used shellcheck, some fixes, added new countries used shellcheck, some fixes, added new countries, added new support for url query parameter Nov 27, 2017
@deadhead420
Copy link
Owner

Looks good! Only thing is running default with no options does not work. I'll have to look at it more soon

@deadhead420
Copy link
Owner

Can you verify this? When running fetchmirrors without any options it shows usage info and not country select list.

@deadhead420
Copy link
Owner

The search function is broken

@deadhead420
Copy link
Owner

I will be more than happy to merge this and update on the AUR but the search function needs to be working. One of the problems is the line with: [ -z "$1" ] causes the program not to even use the search section, this line needs to be removed and only the line checking for root left. Beyond that when you enter the search function it fails when you select a country number.

@YoshiMan
Copy link
Author

When I invoke fetchmirrors without sudo and without any arguments it shows the (modified) usage info.
Or did I get you wrong?
my output from the "your" fetchmirror:


 Usage: fetchmirrors <opts> <args> Ex: fetchmirrors -q -s 4 -c US

 Options:
   -c --country
   Specify your country code(s): fetchmirrors -c US AU
   -d --nocolor
   Disable color prompts
   -h --help
   Display this help message
   -l --list
   Display list of country codes
   -q --noconfirm
   Disable confirmation messages
   -s --servers
   Number of servers to rank (default  6)
   -v --verbose
   Verbose output

 Use fetchmirrors without any option to prompt for country code(s)

my output with "my" fetchmirrors:


 Usage: fetchmirrors.sh <opts> <args> Ex: fetchmirrors.sh -q -s 4 -c US

 Options:
   -c --country
   Specify your country code(s): fetchmirrors.sh -c US AU
   -d --nocolor
   Disable color prompts
   -h --help
   Display this help message
   -l --list
   Display list of country codes
   -q --noconfirm
   Disable confirmation messages
   -s --servers
   Number of servers to rank (default  6)
   -v --verbose
   Verbose output
   -p --protocol
   the protocol to use either http or https, default is empty (both)
   -i --ip-version
   the ip version to use either 4 or 6, default is empty (both)
   -u --use-mirror-status
   use mirror status

 Use fetchmirrors.sh without any option to prompt for country code(s)

@YoshiMan
Copy link
Author

I dont understand what you mean by "The search function is broken", could you provide an sample search term or any further information. ;)

I have recorded my terminal via https://asciinema.org.

the parameterless call. https://asciinema.org/a/5ugAH0aTnlKZdpavY65Z4hdvi
the search function. https://asciinema.org/a/YWaPSpCbBhaqsHdg8yhHDjQwZ

Maybe you could do this to, so i can understand what you mean. :)

moved parameters to get_list function, because they dont change per request
@YoshiMan
Copy link
Author

YoshiMan commented Dec 5, 2017

@deadhead420 could you provide some Information?
thank you.

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.

2 participants