tvm icon indicating copy to clipboard operation
tvm copied to clipboard

Fix command line input process issue.

Open Liliyaw opened this issue 3 years ago • 6 comments
trafficstars

I think previously by " if [ "$#" -lt 1 -o "$1" == "--help" -o "$1" == "-h" ]; then xxx" we are trying to realize when the user input non-para, we will show help_info too? So if use previous one, it may shows unbound variable $1, may not what we hope to do.

Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers by @ them in the pull request thread.

Liliyaw avatar May 22 '22 19:05 Liliyaw

@Mousius @ashutosh-arm for visibility.

grant-arm avatar May 23 '22 09:05 grant-arm

While the new syntax looks better, tbh, I don't fully understand the difference between the two. Cursory googling shows that -o / -a <OPTIONNAME> are going to be obsolete in standard, but couldn't find authentic source for it. Maybe @leandron @Mousius know more about it.

ashutosh-arm avatar May 23 '22 09:05 ashutosh-arm

While the new syntax looks better, tbh, I don't fully understand the difference between the two. Cursory googling shows that -o / -a are going to be obsolete in standard, but couldn't find authentic source for it. Maybe @leandron @Mousius know more about it.

Hi IMO, the difference between -o and || is that, command1 -o command2, the two commands will operate at the same time to judge the result. But for command1 || command 2, command 2 only operate when command 1 is not right. So in this use case, I guess previous you hope when CLI parameter <1( means you will not input any parameter), you will see the show_usage() information too right? But in the code above, if a user don't input any parameter, it will means $1 will not exists and then it will report an unbound variable error.

here, in this script, it's used for docker file and you can find in that file, it specify a parameter, which means $1 will never appear not existing status. The grammar doesn't report have issue but if you only use the script itself, you can find that if you don't specify any CLI para, it will not print show_usage() information. Instead with an error.

Liliyaw avatar May 23 '22 15:05 Liliyaw

Hi. I think both syntaxes are used and equivalent. Can you show the error message you're seeing? I tried both on my terminal and they seem to work the same way, and I'm not seeing any unbound variable error.

I tried it on MacOS and Ubuntu 20.

Have you tried run it like this? don't pass an path para to install script. image IMO, the first command if "$#" -lt 1 means that when you don't pass any para to the script, it will print the show_usage() information automatically.

Liliyaw avatar May 25 '22 08:05 Liliyaw

@tvm-bot rerun

gromero avatar Aug 03 '22 15:08 gromero

@tvm-bot rerun

gromero avatar Aug 04 '22 20:08 gromero