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

Fix print_usage and man page mistakes #103

Merged
merged 25 commits into from Nov 23, 2023
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
2c4b65e
Fix print_usage mistakes
Jun 26, 2023
4195d4e
Fix the manual page as well
Jun 26, 2023
d5fac81
Remove redundancy and fix man page URLs
Jun 26, 2023
5cf3ff9
Add a period ad the end of the urls
Jun 26, 2023
053757b
Fix man page mistakes and update usage
Jun 27, 2023
1da5cb3
Fix capitalization
Jun 27, 2023
17fac4a
Reorder options in a more logical manner
Jun 27, 2023
774d9ea
Remove unused variable in deps.sh
Jun 28, 2023
08ffe3e
remove non-existing options and add other options to man page
Jul 23, 2023
2754c7b
Remove obsolete padding code
Jul 24, 2023
0900ecb
Replace "[PAGE]" with "PAGE" to comply with usual conventions
Jul 24, 2023
32c7172
Prevent -v from printing version if no additional arguments are provi…
Jul 24, 2023
9383391
Use literal URLs in the man page to increase compatiability between m…
Jul 24, 2023
7747857
Comply with padding of 80 colums
Jul 24, 2023
6985894
Client specification compliant does the opposite of commit 32c7172
Jul 24, 2023
56749be
Fix exit codes and more descriptive usage info for --verbose
Jul 24, 2023
9a5515d
More valid --verbose description
Jul 24, 2023
7f2d816
Revert to old usage, but use the long option instead
Jul 27, 2023
5402f04
Revert to old usage, but use the long option instead
Jul 27, 2023
d3dab8f
Revert accidental changes
Jul 27, 2023
fc12f84
Merge branch 'main' into main
kbdharun Oct 31, 2023
d36ea25
tldr.1: update manpage
kbdharun Oct 31, 2023
fbeca5d
tldr.c: fix EOL
kbdharun Oct 31, 2023
ae82b4f
Properly align print_usage
Nov 18, 2023
9bf6e74
Fix whitespace and argument phrasing
Nov 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion deps.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#!/bin/sh

UNAME=$(uname -s)
ARCH=$(uname -m)

if [ "$UNAME" = "Darwin" ]; then
HAS_BREW=$(command -v brew > /dev/null 2>&1 && echo 1 || echo 0)
Expand Down
36 changes: 24 additions & 12 deletions man/tldr.1
Original file line number Diff line number Diff line change
@@ -1,22 +1,18 @@
.\" Manpage for tldr.
.\" Contact ag@arvid.io to correct errors or typos.
.mso www.tmac
.TH TLDR 1
.SH NAME
tldr \- A collection of simplified and community-driven man pages.
.SH SYNOPSIS
.B tldr
[\fB\-v\fR]
[\fIOPTION\fR]... \fISEARCH\fR
[\fI\-\-verbose\fR] [\fIOPTION\fR]... \fIPAGE\fR
.SH DESCRIPTION
tldr is a collection of simplified and community-driven man pages, for commonly
used commandline tools.
.SH OPTIONS
.TP
.BR \-v
verbose output
.TP
.BR \-\-version
.BR \-v ", " \-\-version
output version information and exit
.TP
.BR \-h ", " \-\-help
Expand All @@ -28,19 +24,35 @@ update local database and exit
.BR \-c ", " \-\-clear-cache
remove local database and exit
.TP
.BR \-p ", " \-\-platform=\fIPLATFORM\fR
select platform, supported are \fIlinux\fR / \fIosx\fR / \fIsunos\fR / \fIwindows\fR / \fIcommon\fR
\fB\-p\fR, \fB\-\-platform\fR=\fIPLATFORM\fR
select platform, supported are \fIlinux\fR / \fIosx\fR / \fIsunos\fR /
\fIwindows\fR / \fIcommon\fR
.TP
.BR \-r ", " \-\-render=\fIPATH\fR
\fB\-r\fR, \fB\-\-render\fR=\fIPATH\fR
render a local page for testing purposes
.TP
.B \-\-verbose
print verbose output (when clearing or updating database)
.TP
.B \-\-list
list all entries in the local database
.TP
.B \-\-linux
show command page for Linux
.TP
.B \-\-osx
show command page for OSX
.TP
.B \-\-sunos
show command page for SunOS
.SH EXIT STATUS
0 on success, any other positive value otherwise
.SH SEE ALSO
The source is available at:
.URL "https://github.com/tldr-pages/tldr-cpp-client" "" ""
<https://github.com/tldr-pages/tldr-cpp-client>.
.SH REPORTING BUGS
Report bugs through the Github repository:
.URL "https://github.com/tldr-pages/tldr-cpp-client" "" "."
Report bugs through the GitHub repository:
<https://github.com/tldr-pages/tldr-cpp-client>.
.SH COPYRIGHT
The MIT License (MIT)

Expand Down
37 changes: 21 additions & 16 deletions src/tldr.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,13 @@ main(int argc, char **argv)
break;

case 'v':
verbose_flag = 1;
version_flag = 1;
break;

case '?':
/* do not set help flag, only show getopt error */
/* help_flag = 1; */
return EXIT_FAILURE;
Copy link
Member

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 of 1 being preferable when printing help.

break;

case 'p': {
Expand Down Expand Up @@ -119,10 +120,14 @@ main(int argc, char **argv)

/* show help, if 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;
Expand All @@ -138,8 +143,8 @@ 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;
Copy link
Member

Choose a reason for hiding this comment

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

I assume this exits with code e.g. 1 instead of 0 when printing help? Other commands e.g. ls, tar, etc exit with code 0 when printing help, so we should do the same.

Unless I misunderstood this?

Copy link
Member

@kbdharun kbdharun Nov 12, 2023

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Author

Choose a reason for hiding this comment

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

I assume this exits with code e.g. 1 instead of 0 when printing help? Other commands e.g. ls, tar, etc exit with code 0 when printing help, so we should do the same.

Unless I misunderstood this?

This exits with 1 on a getopt error.

For example:

  • tldr -a, which is an unknown argument, exits with 1 and displays a getopt error.
  • tldr -v exits with 0 and displays the version.

Copy link
Author

Choose a reason for hiding this comment

The 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

Copy link
Member

Choose a reason for hiding this comment

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

Ah okay. So can you confirm that when you call e.g. tldr --help it exits with code 0, @4G3NT?

Copy link
Author

Choose a reason for hiding this comment

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

Ah okay. So can you confirm that when you call e.g. tldr --help it exits with code 0, @4G3NT?

Yes, it exits with code 0 because of this code:

if (help_flag) {
    print_usage(argv[0]);
    return EXIT_SUCCESS;
}

Copy link
Member

Choose a reason for hiding this comment

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

Very cool, many thanks!

}
if (list_flag) {
if (!has_localdb())
Expand Down Expand Up @@ -204,24 +209,24 @@ print_version(char const *arg)
void
print_usage(char const *arg)
{
char const *out = "usage: %s [-v] [OPTION]... SEARCH\n\n";
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", "-v, --version", "print version and exit");
fprintf(stdout, " %-23s %s\n", "-h, --help", "print this help and exit");
fprintf(stdout, " %-23s %s\n", "-u, --update", "update local database");
fprintf(stdout, " %-23s %s\n", "-c, --clear-cache", "clear local database");
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, " %-23s %s\n", "--verbose", "verbose output (when clearing or updating database)");
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* */
}