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

doc: Added information on --indent to --help #2504

Closed
wants to merge 1 commit into from

Conversation

EvanCarroll
Copy link

Simple doc patch when I noticed functionality I already need was there just undocumented.

src/main.c Outdated Show resolved Hide resolved
@pkoppstein
Copy link
Contributor

Since --indent requires an argument, an argument should be mentioned in usage(). Following the manual, --indent n would be appropriate.

The larger point here is that documenting all the options in usage() in this manner would increase the size of the executable quite a lot, and goes against the idea of keeping the executable small by providing details online.

I would propose that usage() simply provide a synopsis of all the options (in the usual man page style), leaving the descriptive details of the options to the online documentation.

@EvanCarroll
Copy link
Author

@pkoppstein the patch was already amended 12 hours before your comment to include n. And you're talking about a couple of bytes of text. Hardly something that will "increase the size of the executable quite a lot."

@pkoppstein
Copy link
Contributor

@EvanCarroll - You apparently overlooked the phrase “ … documenting all the options in usage() …”.

(The screen I was looking at only showed the line without the “n”.)

@wader
Copy link
Member

wader commented Mar 8, 2023

I would not worry about binary size either, possibly that the text is getting long, but maybe then we should cut down on some other usage text instead? maybe the stuff about IEEE754 numbers etc

@itchyny itchyny added the docs label Jun 3, 2023
@itchyny itchyny added this to the 1.7 release milestone Jun 25, 2023
Copy link
Contributor

@itchyny itchyny left a comment

Choose a reason for hiding this comment

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

I'm not worried about the executable size due to this help, we should list all the option #2284 (we should rather consider replace the builtins by C-coded internals). The only concern about this PR is about the newline in the description; can Bash's _parse_help parse this form?

@itchyny
Copy link
Contributor

itchyny commented Jun 29, 2023

Looks OK.

 $ _parse_help jq -h | xargs
-c -n -e -s -r -R -C -M -S --tab --arg --argjson --slurpfile --rawfile --args --jsonargs --
 $ _parse_help ./jq -h | xargs
-c -n -e -s -r -R -C -M -S --tab --indent --arg --argjson --slurpfile --rawfile --args --jsonargs --

@@ -81,6 +81,8 @@ static void usage(int code, int keep_it_short) {
" -M monochrome (don't colorize JSON);\n"
" -S sort keys of objects on output;\n"
" --tab use tabs for indentation;\n"
" --indent n use $n spaces for indentation.\n"
" between -1 (default of 4 spaces) and 7;\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, -1 is not 4 spaces, but the tab character.

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

Successfully merging this pull request may close these issues.

4 participants