update formatting help #6359

mntlty · 2022-09-28T03:37:24Z

Adds more details to gh formatting -h

mntlty · 2022-10-03T10:29:36Z

pkg/cmd/root/help_topic.go

-			# format a pull request using multiple tables with headers
+			# default output format
+			$ gh pr list
+			Showing 23 of 23 open pull requests in cli/cli


I included example output from each of the example commands so users can view them and compare without needing to run the commands, which could be helpful in the web view. The downside is it's not consistent with other examples, and a lot more verbose which may be distracting, and risks getting out of date at some point in the future.

Leaving it in for discussion but it can definitely be removed

pkg/cmd/root/help_topic.go

mntlty · 2022-10-03T10:31:52Z

pkg/cmd/root/help_topic.go

+			]
+
+
+			# adding the --jq flag and filtering the output to select the second result


not sure if it makes sense to add a more complex jq example as well, or if that would be more confusing than it's worth

Let's change an example to something that prints raw values of leaf nodes, rather than printing a nested JSON structure. For example: .[].author.login prints

monalisa codercat cli-maintainer

The result of such jq expression can then be piped into a line-based script such as shell loop, which makes it more useful in my mind than outputting JSON.

mntlty · 2022-10-03T10:33:22Z

I wasn't 100 percent sure what should have backticks around it and what shouldn't, I added them around command names but not flag names.

mislav

This looks great!

General suggestions:

I would enclose --flag names in backticks for consistency and for readability, since encountering -- in prose can be disorienting, and since inline code tags will look better both in generated HTML and in generated man pages;
Note that we hard wrap our help text to 80 columns since there is nothing that will automatically wrap our help output for us. We aren't consistent with this, but some wrapping is better than completely unwrapped since some people can have ridiculously wide terminal windows but probably would not like to read paragraphs stretched across a whole ultrawide monitor.

mislav · 2022-10-03T13:19:52Z

pkg/cmd/root/help_topic.go

-			- %[1]struncate <length> <input>%[1]s: ensures input fits within length
+		By default, the result of %[1]sgh%[1]s commands are output in line-based plain text format. Some commands support passing the --json flag, which converts the output to JSON format. Once in JSON, the output can be further formatted according to a required formatting string by adding either the --jq or --template flag. This is useful for selecting a subset of data, creating new data structures, displaying the data in a different format, or as input to another command line script.
+
+		The --json flag requires a comma separated list of fields which match the JSON field names from an API response. For a list of available fields, see: <https://docs.github.com/en/graphql/reference/>. Note that you must pass the --json flag and field names to use the --jq or --template flags.


"For a list of available fields, see GraphQL reference" — this isn't correct, since our json fields do not necessarily have 1-1 mapping to our GraphQL schema (sometimes they are just made up). The previous documentation which you removed was correct:

Use the flag without a value to get the list of available fields.

If that sentence wasn't clear, perhaps you can modify it to make it clearer?

thanks for clarifying, I wasn't sure about that and should have called it out in the PR. I think this captures it, let me know what you think!

mislav · 2022-10-03T13:22:52Z

pkg/cmd/root/help_topic.go

+
+		The --json flag requires a comma separated list of fields which match the JSON field names from an API response. For a list of available fields, see: <https://docs.github.com/en/graphql/reference/>. Note that you must pass the --json flag and field names to use the --jq or --template flags.
+
+		The --jq flag requires a string argument in %[1]sjq%[1]s query syntax, and will only print those JSON values which match the query. %[1]sjq%[1]s queries can be used to select elements from an array, fields from an object, create a new array, and more. The %[1]sjq%[1]s utility does not need to be installed on the system to use this formatting directive. To learn about %[1]sjq%[1]s query syntax, see: <https://stedolan.github.io/jq/manual/v1.6/>


In this paragraph, "jq" is a proper noun of not just the command-line utility but also of the query language, and since we're mostly referencing the query language here, jq needn't be quoted in backticks.

This is equivalent to piping the output to jq -r,

I thought this bit of the old documentation was useful to draw a parallel between using --jq gh flag and the jq utility directly. Do you think it was too noisy to keep in?

I wasn't quite sure what the -r flag was for, and had to look it up in the jq documentation. To be honest, I'm still not sure what it is meant for. My feeling was that it probably makes sense to people who are very familiar with jq and confusing to anyone who isn't. If you think it's important of course it should be added back in

I think was a useful distinction to point out that using --jq is not equivalent to piping the --json output to jq using the same expression.

gh pr view --json author --jq .author.login #=> mntlty gh pr view --json author | jq .author.login #=> "mntlty"

pkg/cmd/root/help_topic.go

mislav · 2022-10-03T13:32:12Z

pkg/cmd/root/help_topic.go

+			]
+
+
+			# adding the --jq flag and filtering the output to select the second result


Let's change an example to something that prints raw values of leaf nodes, rather than printing a nested JSON structure. For example: .[].author.login prints

monalisa codercat cli-maintainer

The result of such jq expression can then be piped into a line-based script such as shell loop, which makes it more useful in my mind than outputting JSON.

mislav · 2022-10-03T13:34:12Z

pkg/cmd/root/help_topic.go

+			$ gh pr list --json number,title,headRefName,updatedAt --template \
+				'{{range .}}{{tablerow (printf "#%v" .number | autocolor "green") .title .headRefName (timeago .updatedAt)}}{{end}}'


This --template example outputs the exact format of regular gh pr list, which isn't very compelling as an example for using --template. How about changing something about this expression so that the final output is somehow different (and potentially more useful to some people) than regular gh pr list?

I wondered about that. I think this is more helpful because it's already known, and you can see how to sort of reverse engineer it using templates. I personally find a small tweak or revealing how something is done is easier for me to pattern match against. What do you think?

Well, it can stay like this if the goal was to demonstrate how one can reverse-engineer the default output. But, from the user's perspective, they might be scanning these examples and seeing how one simple gh invocation and one seemingly very complicated gh invocation both produce the same result.

I'll leave the final decision up to you. The example itself is indeed useful as a starting point for editing one's template.

pkg/cmd/root/help_topic.go

mislav · 2022-10-03T13:40:30Z

pkg/cmd/root/help_topic.go

-			- %[1]stimeago <time>%[1]s: renders a timestamp as relative to now
-			- %[1]stimefmt <format> <time>%[1]s: formats a timestamp using Go's Time.Format function
-			- %[1]struncate <length> <input>%[1]s: ensures input fits within length
+		By default, the result of %[1]sgh%[1]s commands are output in line-based plain text format. Some commands support passing the --json flag, which converts the output to JSON format. Once in JSON, the output can be further formatted according to a required formatting string by adding either the --jq or --template flag. This is useful for selecting a subset of data, creating new data structures, displaying the data in a different format, or as input to another command line script.


"The result … is" instead of "The result … are" sounds better to me

are because of the plural of commands, not the singular of result, sounds correct to me when reading the whole sentence 🤷 I honestly am not sure about this one

mntlty · 2022-10-03T16:18:12Z

This looks great!

General suggestions:

I would enclose --flag names in backticks for consistency and for readability, since encountering -- in prose can be disorienting, and since inline code tags will look better both in generated HTML and in generated man pages;

Note that we hard wrap our help text to 80 columns since there is nothing that will automatically wrap our help output for us. We aren't consistent with this, but some wrapping is better than completely unwrapped since some people can have ridiculously wide terminal windows but probably would not like to read paragraphs stretched across a whole ultrawide monitor.

done! some of the lines look longer in the go code but with the string substitution the backtick makes them shorter when they are rendered. they're now almost all the same length in the terminal, with a bit of wiggle room if it made the sentence a bit easier to parse based on where I added the line break

mntlty · 2022-10-03T16:19:32Z

@mislav I think I addressed all the feedback, it's hard to do long form text in a PR. please update anything that doesn't look right or let me know if you'd like to discuss further 🙏

mislav

Looks great. Thank you!

mislav · 2022-10-04T15:17:41Z

pkg/cmd/root/help_topic.go

-		To learn more about Go templates, see: <https://golang.org/pkg/text/template/>.
+			By default, the result of %[1]sgh%[1]s commands are output in line-based plain text format.
+			Some commands support passing the %[1]s--json%[1]s flag, which converts the output to JSON format.
+			Once in JSON, the output can be further formatted according to a required formatting string by


I've missed this in my original review: “according to a required formatting string” doesn't sound right to me. When using --json, passing a formatting string isn't required at all. Also, “formatting according to … formatting string” sound repetitive. How about:

Once in JSON, the output can be further formatted using either --jq or --template flag.

mislav · 2022-10-04T15:22:50Z

pkg/cmd/root/help_topic.go

+			creating new data structures, displaying the data in a different format, or as input to another
+			command line script.
+
+			The %[1]s--json%[1]s flag requires a comma separated list of fields to fetch. To view the possible JSON


Ah, I think I understand now. You have been using the pattern “the --foo flag requires so and so value”, which reads odd to me because I haven't seen this phrasing before, but it is technically correct that all of these flags require a value. Only boolean flags do not need a value.

Elsewhere, I think we've been using “the --foo flag accepts so and so value”, which reads a bit softer than “requires”, but ultimately there are no hard and fast rules around this.

update formatting help command

6785e9f

mntlty force-pushed the mntlty/formatting_help branch from c2f2fb5 to 6785e9f Compare October 3, 2022 10:25

mntlty marked this pull request as ready for review October 3, 2022 10:25

mntlty requested a review from a team as a code owner October 3, 2022 10:25

mntlty requested review from samcoe and removed request for a team October 3, 2022 10:25

cliAutomation added the external pull request originating outside of the CLI core team label Oct 3, 2022

cliAutomation added this to Needs review 🤔 in The GitHub CLI Oct 3, 2022

mntlty changed the title ~~mntlty/formatting help~~ update formatting help Oct 3, 2022

mntlty commented Oct 3, 2022

View reviewed changes

pkg/cmd/root/help_topic.go Outdated Show resolved Hide resolved

mntlty commented Oct 3, 2022

View reviewed changes

mislav suggested changes Oct 3, 2022

View reviewed changes

code review feedback

7a55496

mislav approved these changes Oct 4, 2022

View reviewed changes

The GitHub CLI automation moved this from Needs review 🤔 to Needs to be merged 🎉 Oct 4, 2022

vilmibm merged commit f82c6a6 into trunk Oct 4, 2022

The GitHub CLI automation moved this from Needs to be merged 🎉 to Pending Release 🥚 Oct 4, 2022

vilmibm deleted the mntlty/formatting_help branch October 4, 2022 19:04

github-actions bot moved this from Pending Release 🥚 to Done 💤 in The GitHub CLI Oct 4, 2022

Lil84 referenced this pull request Nov 1, 2022

Update install_linux.md

5372f92

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update formatting help #6359

update formatting help #6359

mntlty commented Sep 28, 2022 •

edited

mntlty Oct 3, 2022

mntlty Oct 3, 2022

mislav Oct 3, 2022 •

edited

mntlty commented Oct 3, 2022

mislav left a comment

mislav Oct 3, 2022

mntlty Oct 3, 2022

mislav Oct 3, 2022

mntlty Oct 3, 2022

mislav Oct 4, 2022

mislav Oct 3, 2022 •

edited

mislav Oct 3, 2022

mntlty Oct 3, 2022

mislav Oct 4, 2022

mislav Oct 3, 2022

mntlty Oct 3, 2022

mntlty commented Oct 3, 2022

mntlty commented Oct 3, 2022

mislav left a comment

mislav Oct 4, 2022

mislav Oct 4, 2022 •

edited

		]


		# adding the --jq flag and filtering the output to select the second result


		The --json flag requires a comma separated list of fields which match the JSON field names from an API response. For a list of available fields, see: <https://docs.github.com/en/graphql/reference/>. Note that you must pass the --json flag and field names to use the --jq or --template flags.

		The --jq flag requires a string argument in %[1]sjq%[1]s query syntax, and will only print those JSON values which match the query. %[1]sjq%[1]s queries can be used to select elements from an array, fields from an object, create a new array, and more. The %[1]sjq%[1]s utility does not need to be installed on the system to use this formatting directive. To learn about %[1]sjq%[1]s query syntax, see: <https://stedolan.github.io/jq/manual/v1.6/>

		$ gh pr list --json number,title,headRefName,updatedAt --template \
		'{{range .}}{{tablerow (printf "#%v" .number \| autocolor "green") .title .headRefName (timeago .updatedAt)}}{{end}}'

update formatting help #6359

update formatting help #6359

Conversation

mntlty commented Sep 28, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mislav Oct 3, 2022 • edited

Choose a reason for hiding this comment

mntlty commented Oct 3, 2022

mislav left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mislav Oct 3, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mntlty commented Oct 3, 2022

mntlty commented Oct 3, 2022

mislav left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mislav Oct 4, 2022 • edited

Choose a reason for hiding this comment

mntlty commented Sep 28, 2022 •

edited

mislav Oct 3, 2022 •

edited

mislav Oct 3, 2022 •

edited

mislav Oct 4, 2022 •

edited