gh gist create by vilmibm · Pull Request #981 · cli/cli

vilmibm · 2020-05-20T22:18:00Z

Fixes #397

Based on #543; moved over to new PR for CI reasons. Big ups to @wingkwong for getting it started.

This PR adds support for creating gists. You can invoke it in a few ways:

gh gist create hello.py                   # turn file hello.py into a gist
gh gist create --public hello.py          # turn file hello.py into a public gist
gh gist create -d"a file!" hello.py       # turn file hello.py into a gist, with description
gh gist create hello.py world.py cool.txt # make a gist out of several files
gh gist create -                          # read from STDIN to create a gist
cat cool.txt | gh gist create             # read the output of another command and make a gist out of it

It looks like this:

Considerations

Currently we don't request the gist scope when creating auth tokens. We didn't want another "stop every user after a release to reauth" since not every user necessarily wants gist support. This PR adds an imperfect helper, ensureScopes that lets us prompt for re-auth in a given command.

In the future the best solution would be the ability to retry failed requests when we determine they failed for valid scope reasons, but that was too much effort for now.

I'd like to add some more tests but otherwise it's ready for review.

Our code had an unspoken assumption that only one apiClient is created during the course of a command. Violating this assumption is fine in almost all cases, but not when we need to do a re-auth to add a new oauth scope to a user's token. There is likely a more elegant solution to the problem but until then this changes determineBaseRepo to use an existing apiClient.

This reverts commit 08e9cda.

tierninho · 2020-05-21T00:18:47Z

I take it the -R flag does not work here as you must be in the directory from which you run to command?

I created the hellp.py file in my repo and used the flag bin/gh gist create hello.py -R tierninho/Blah, but got:

failed to collect files for posting: failed to read file hello.py: open hello.py: no such file or directory

vilmibm · 2020-05-21T01:12:39Z

the -R flag is not relevant. its presence in docs for commands like this is a bug as far as i remember?

either way: the arguments are file paths and relative only to the directory you're in when you run the command and not to any repositories.

ampinsk

Looks awesome from my perspective! I'm curious if there would be any value to a default behavior when there are no arguments (maybe make a gist out of whatever is in the directory? though I can see that being unexpected/destructive), but I'm totally comfortable thinking about that as a follow up!

mislav

Good job getting this over the finish line! 🎉

mislav · 2020-05-22T17:58:50Z

command/gist.go

+}
+
+var gistCreateCmd = &cobra.Command{
+	Use:   `create {<filename>|-}...`,


Nit: since - technically can't be repeated, a more precise usage expression would be:

create {<filename>... | -}

but I would be fine with simply omitting the information about - from the usage line and make a note of it in the docs where we talk about stdin.

mislav · 2020-05-22T18:17:28Z

command/gist.go

+	RootCmd.AddCommand(gistCmd)
+	gistCmd.AddCommand(gistCreateCmd)
+	gistCreateCmd.Flags().StringP("desc", "d", "", "A description for this gist")
+	gistCreateCmd.Flags().BoolP("public", "p", false, "When true, the gist will be public and available for anyone to see (default: private)")


Nit: to simplify the language of this flag description, might we use the same language as in gh repo create?

--public Make the new repository public (default: private)

The key would be to eliminate the When true prefix since "when true" is basically implied for all boolean flags.

mislav · 2020-05-22T18:22:46Z

command/gist.go

+var gistCreateCmd = &cobra.Command{
+	Use:   `create {<filename>|-}...`,
+	Short: "Create a new gist",
+	Long: `gh gist create: create gists


As the first sentence of Long description, we usually have a simple description of what the command does, e.g.

$ gh help pr list List and filter pull requests in this repository ...

This can be just "Create a GitHub gist", I feel. We don't need to hardcode gh gist create since that part the user already entered in their prompt.

mislav · 2020-05-22T18:23:30Z

command/gist.go

+
+Gists can be created from one or many files. This command can also read from STDIN. By default, gists are private; use --public to change this.
+
+Examples


I've found this out by accident the other day: Cobra has a dedicated Example field! spf13/cobra@bf480fe

mislav · 2020-05-22T18:24:54Z

command/gist.go

+	// In the future we'd rather have the ability to detect a "reauth needed" scenario and replay
+	// failed requests but some short spikes indicated that that would be a fair bit of work.


props to explaining all this 👏

mislav · 2020-05-22T18:30:20Z

command/gist.go

+	if err != nil {
+		return fmt.Errorf("failed to check STDIN: %w", err)
+	}
+	if (info.Mode() & os.ModeCharDevice) != os.ModeCharDevice {


TIL this way of inspecting stdin 🌟 Does this work on Windows?

In my experience, the easiest way to discern whether something was piped to a command (vs a command being called on its own) is to check whether stdin is not connected to a terminal.

mislav · 2020-05-22T18:30:47Z

command/gist.go

+		return fmt.Errorf("did not understand arguments: %w", err)
+	}
+
+	info, err := os.Stdin.Stat()


Should we always stat stdin unconditionally, or just when the file list is empty?

mislav · 2020-05-22T18:35:00Z

command/gist.go

+		var err error
+		if f == "-" {
+			filename = fmt.Sprintf("gistfile%d.txt", i)
+			content, err = ioutil.ReadAll(stdin)


Minor: it's not strictly necessary, but I think that closing stdin after having exhausted it would be the right thing to do, especially because it would lead to a clearer error message if someone tried to pass - multiple times.

wingkwong · 2020-05-28T06:25:53Z

@mislav @vilmibm Now we got gist create. May I know if gist list/edit/delete are in cli roadmap? If so, I may work on them.

dardo82 · 2020-06-23T09:37:26Z

@wingwong I'd like to be able to edit gists from the cli too,can you please implement the feature?

wingwong · 2020-06-23T13:10:01Z

I believe the message for @wingkwong instead. From: Michele Venturi <notifications@github.com> Sent: Tuesday, June 23, 2020 5:38 AM To: cli/cli <cli@noreply.github.com> Cc: Wong, Wing <wing.wong@sap.com>; Mention <mention@noreply.github.com> Subject: Re: [cli/cli] gh gist create (#981) @wingwong<https://github.com/wingwong> I'd like to be able to edit gists from the cli too,can you please implement the feature? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#981 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABQ5NM3CBNUEABZ5WW552RTRYBZWLANCNFSM4NGLKEZQ>.

wingkwong · 2020-06-23T13:30:15Z

@dardo82 Sure. I'll implement it when I'm free.
@wingwong It happens sometimes. Sorry about it.

dardo82 · 2020-06-23T14:24:43Z

@wingkwong thanks,how can I help you? 🤓

mislav · 2020-06-23T15:51:07Z

Hi, for discussing new feature ideas, such as the ability to list or edit gists, please first open a new issue for it so we can have a discussion there. I feel that this PR should only be a place for discussing the pr create functionality that got merged. Thank you!

wingkwong and others added 24 commits February 25, 2020 01:25

feat: add gist to scope #397

fc330de

feat: add gist queries #397

a2109c4

feat: add gist create command#397

386caef

feat: add gist create command test case #397

81462d1

feat: add gist create command test file #397

5dc06f8

Merge branch 'master' of https://github.com/cli/cli

22b9beb

Merge branch 'master' of https://github.com/cli/cli

64f4622

fix: add missing context to gist create command test case

3e54a0e

Merge remote-tracking branch 'origin/master' into wingkwong/master

ed1a3a6

tweak reauth code and request a new scope

3a7f564

stray debug

a62f298

notes

3b62d79

small repo fix

08e9cda

untested first pass on ensureScopes

7decae7

Revert "small repo fix"

bf05975

This reverts commit 08e9cda.

undo initial thing

96ba1ea

fix tests

780875d

take files as args. handle -

466edf8

handle piped STDIN

93dc62f

improve docs

7453449

tweak test

f644c11

Merge remote-tracking branch 'origin/master' into wingkwong/master

509be34

handle json error in test

f6137fa

vilmibm mentioned this pull request May 20, 2020

feat: add command to create gist #543

Closed

just use a map + stdin test

5f28751

vilmibm requested review from ampinsk, mislav and probablycorey May 20, 2020 22:47

ampinsk approved these changes May 21, 2020

View reviewed changes

mislav mentioned this pull request May 22, 2020

Respect GITHUB_TOKEN environment variable #976

Merged

1 task

mislav approved these changes May 25, 2020

View reviewed changes

vilmibm merged commit 9ebeb5f into master May 25, 2020

mislav deleted the gist-create branch June 23, 2020 13:51

wingkwong mentioned this pull request Jun 24, 2020

Feature request: better support for gist editing #1260

Closed


		Gists can be created from one or many files. This command can also read from STDIN. By default, gists are private; use --public to change this.

		Examples

		// In the future we'd rather have the ability to detect a "reauth needed" scenario and replay
		// failed requests but some short spikes indicated that that would be a fair bit of work.

Conversation

vilmibm commented May 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Considerations

Uh oh!

tierninho commented May 21, 2020

Uh oh!

vilmibm commented May 21, 2020

Uh oh!

ampinsk left a comment

Choose a reason for hiding this comment

Uh oh!

mislav left a comment

Choose a reason for hiding this comment

Uh oh!

mislav May 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mislav May 22, 2020

Choose a reason for hiding this comment

Uh oh!

mislav May 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mislav May 22, 2020

Choose a reason for hiding this comment

Uh oh!

mislav May 22, 2020

Choose a reason for hiding this comment

Uh oh!

mislav May 22, 2020

Choose a reason for hiding this comment

Uh oh!

mislav May 22, 2020

Choose a reason for hiding this comment

Uh oh!

mislav May 22, 2020

Choose a reason for hiding this comment

Uh oh!

wingkwong commented May 28, 2020

Uh oh!

dardo82 commented Jun 23, 2020

Uh oh!

wingwong commented Jun 23, 2020 via email

Uh oh!

wingkwong commented Jun 23, 2020

Uh oh!

dardo82 commented Jun 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mislav commented Jun 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

vilmibm commented May 20, 2020 •

edited

Loading

mislav May 22, 2020 •

edited

Loading

mislav May 22, 2020 •

edited

Loading

dardo82 commented Jun 23, 2020 •

edited

Loading