-
-
Notifications
You must be signed in to change notification settings - Fork 49
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
Fix print_usage and man page mistakes #103
Changes from all commits
2c4b65e
4195d4e
d5fac81
5cf3ff9
053757b
1da5cb3
17fac4a
774d9ea
08ffe3e
2754c7b
0900ecb
32c7172
9383391
7747857
6985894
56749be
9a5515d
7f2d816
5402f04
d3dab8f
fc12f84
d36ea25
fbeca5d
ae82b4f
9bf6e74
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,7 +70,7 @@ int main(int argc, char **argv) { | |
|
||
while (1) { | ||
option_index = 0; | ||
c = getopt_long_only(argc, argv, "v", long_options, &option_index); | ||
c = getopt_long_only(argc, argv, "vp:r:C", long_options, &option_index); | ||
|
||
/* reached the end, bail out */ | ||
if (c == -1) { | ||
|
@@ -82,12 +82,13 @@ int main(int argc, char **argv) { | |
break; | ||
|
||
case 'v': | ||
verbose_flag = 1; | ||
version_flag = 1; | ||
break; | ||
|
||
case '?': | ||
/* do not set the help flag, only show getopt error */ | ||
/* help_flag = 1; */ | ||
return EXIT_FAILURE; | ||
break; | ||
|
||
case 'p': { | ||
|
@@ -130,10 +131,14 @@ int main(int argc, char **argv) { | |
|
||
/* show help, if the platform was supplied, but no further argument */ | ||
missing_arg = (platform_flag && !list_flag && (optind == argc)); | ||
if (help_flag || missing_arg) { | ||
if (help_flag) { | ||
print_usage(argv[0]); | ||
return EXIT_SUCCESS; | ||
} | ||
if (missing_arg) { | ||
print_usage(argv[0]); | ||
return EXIT_FAILURE; | ||
} | ||
if (version_flag) { | ||
print_version(argv[0]); | ||
return EXIT_SUCCESS; | ||
|
@@ -149,8 +154,8 @@ int main(int argc, char **argv) { | |
return EXIT_SUCCESS; | ||
} | ||
if (verbose_flag && optind >= argc) { | ||
print_version(argv[0]); | ||
return EXIT_SUCCESS; | ||
print_usage(argv[0]); | ||
return EXIT_FAILURE; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume this exits with code e.g. Unless I misunderstood this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Our client specification v1.4 suggests to use a non-zero error code for finding pages but doesn't cover the usage part. (tldr-pages/tldr#4246) Feel free to update the exit code in this branch. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. * Specifically, a client MUST exit with a non-zero exit code if the page could not be found anywhere. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This exits with For example:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can even test this by running these commands: $ tldr -a 2> /dev/null; echo $?
1
$ tldr -v > /dev/null; echo $?
0 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah okay. So can you confirm that when you call e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, it exits with code if (help_flag) {
print_usage(argv[0]);
return EXIT_SUCCESS;
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Very cool, many thanks! |
||
} | ||
if (list_flag) { | ||
if (!has_localdb()) | ||
|
@@ -210,25 +215,25 @@ void print_version(char const *arg) { | |
/* *INDENT-ON* */ | ||
} | ||
|
||
void print_usage(char const *arg) { | ||
char const *out = "usage: %s [-v] [OPTION]... SEARCH\n\n"; | ||
void print_usage(char const *arg){ | ||
char const *out = "usage: %s [--verbose] [OPTION]... [PAGE]\n\n"; | ||
|
||
/* *INDENT-OFF* */ | ||
fprintf(stdout, out, arg); | ||
fprintf(stdout, "available commands:\n"); | ||
fprintf(stdout, " %-20s %-30s\n", "-v", "print verbose output"); | ||
fprintf(stdout, " %-20s %-30s\n", "--version", "print version and exit"); | ||
fprintf(stdout, " %-20s %-30s\n", "-h, --help", "print this help and exit"); | ||
fprintf(stdout, " %-20s %-30s\n", "-u, --update", "update local database"); | ||
fprintf(stdout, " %-20s %-30s\n", "-c, --clear-cache", "clear local database"); | ||
fprintf(stdout, " %-20s %-30s\n", "-l, --list", "list all entries in the local database"); | ||
fprintf(stdout, " %-20s %-30s\n", "-p, --platform=PLATFORM", | ||
fprintf(stdout, " %-23s %s\n", "-C, --color", "force color display"); | ||
fprintf(stdout, " %-23s %s\n", "-h, --help", "print this help and exit"); | ||
fprintf(stdout, " %-23s %s\n", "-p, --platform=PLATFORM", | ||
"select platform, supported are linux / osx / sunos / windows / common"); | ||
fprintf(stdout, " %-20s %-30s\n", "--linux", "show command page for Linux"); | ||
fprintf(stdout, " %-20s %-30s\n", "--osx", "show command page for OSX"); | ||
fprintf(stdout, " %-20s %-30s\n", "--sunos", "show command page for SunOS"); | ||
fprintf(stdout, " %-20s %-30s\n", "-r, --render=PATH", | ||
fprintf(stdout, " %-23s %s\n", "-r, --render=PATH", | ||
"render a local page for testing purposes"); | ||
fprintf(stdout, " %-20s %-30s\n", "-C, --color", "force color display"); | ||
fprintf(stdout, " %-23s %s\n", "-u, --update", "update local database"); | ||
fprintf(stdout, " %-23s %s\n", "-v, --version", "print version and exit"); | ||
fprintf(stdout, " %-23s %s\n", "--clear-cache", "clear local database"); | ||
fprintf(stdout, " %-23s %s\n", "--verbose", "display verbose output (when used with --clear-cache or --update)"); | ||
fprintf(stdout, " %-23s %s\n", "--list", "list all entries in the local database"); | ||
fprintf(stdout, " %-23s %s\n", "--linux", "show command page for Linux"); | ||
fprintf(stdout, " %-23s %s\n", "--osx", "show command page for OSX"); | ||
fprintf(stdout, " %-23s %s\n", "--sunos", "show command page for SunOS"); | ||
/* *INDENT-ON* */ | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See below comment about exiting with code
0
instead of1
being preferable when printing help.