cli / go-gh

A Go module for interacting with gh and the GitHub API from the command line.
https://pkg.go.dev/github.com/cli/go-gh/v2
MIT License
331 stars 48 forks source link

[Discussion] Desired features #5

Closed heaths closed 1 year ago

heaths commented 2 years ago

This looks like a great start and has good features for basic binary extensions, but when I opened cli/cli#4264 I was hoping to see more low-level implementations either ported or refactored out into a shared library both the CLI and binary extension developers could use. For example,

heaths commented 2 years ago

As a specific example, it seems there's no way to specify the repo org and name to commands, nor get these from environment variables like $GH_REPO. Ideally, extensions can be used in the same contexts as CLI commands by either supplying their own -R values (parsing could be an extension problem, but being able to pass these values should be supported here) or getting it from $GH_REPO or any other related environment variables you support.

samcoe commented 2 years ago

@heaths Thanks for writing in, looks like I prematurely closed out cli/cli#4264 and I will reopen it. I think both issues are relevant to keep open. This one can be targeted at what features/code we want to extract from the cli code base and move to go-gh and cli/cli#4264 can be targeted at general extracting of modules from the cli code base into their own repositories. For example I think that output formatting code you referenced could easily be generalized and extracted into its own go module but it doesn't need to be tied to go-gh and could just be used in tandem with it.

Regarding the -R command line switch, my position on it is that go-gh should not bundle in large dependencies like Cobra and that parsing of command line arguments and flags should be left up to the extension author. There are many options it to do that and I think go-gh should not prescribe or require a specific approach. How I envision this working is that the extension will parse the command line args and if -R is not specified then the gh.CurrentRepository() function can be used to determine the repository context for the command. It does look like we are missing support for the GH_REPO environment variable in gh.CurrentRepository() though so that is something we should probably add. Does that address your concerns?

heaths commented 2 years ago

I agree go-gh shouldn't depend on Cobra, but what about providing a function to parse [domain/]org/repo into a gh/Repository so there's at least consistent parsing across binary extensions? However extensions want to support and parse -R (or whatever) would be completely up to them, but at least end users can get a consistent experience. This would also apply for parsing env vars like $GH_REPO which can use the same format.

samcoe commented 2 years ago

Great idea, I added the task to https://github.com/cli/go-gh/issues/8 since it has a lot of overlap.

heaths commented 2 years ago

@samcoe thanks for adding repository support - both getting the current or parsing a [host/]owner/repo string. As for the other ask, any problem with a PR (after gh label clone) exposing a few things (perhaps with some abstractions) like the table printer and related functions for truncating, coloring, etc?

samcoe commented 2 years ago

@heaths Rather than adding that functionality to go-gh what are your thoughts on extracting the table printer and related functions into their own repo, and go package, that can be imported by gh, go gh extensions, along with other go projects? They seem are already fairly self contained and could provide more value to the larger go community that way. Something like go-term-table-printer?

heaths commented 2 years ago

Seems reasonable, though would it make more sense just to have a terminal package that contains what gh and extensions could use beyond TablePrinter? Maybe just cli/go-terminal or something that contains the TablePrinter and related truncation, color, and more functions?

samcoe commented 2 years ago

I like that idea, a package that is a collection of terminal utilities that are useful and can be used together or stand alone. I would like to see a proposal of exactly what packages we would want to extract from gh and what it would look like. Right now, in gh many of the packages are not designed to be used outside of gh. Many have complex dependency trees and others have interfaces that were not designed to be exposed as public packages.

heaths commented 2 years ago

Similarly, what about extracting your very nice HTTP mocking functionality? Though, perhaps that makes more sense in this module because that does seem inextricably tied to your HTTP abstractions. After using it for several PRs to the CLI now, I love it!

EDIT: Opened #28 to track.

heaths commented 2 years ago

As for a cli/go-terminal (or whatever), here's a few things that would be nice:

  1. TablePrinter
  2. Pluralize and FuzzyAgo (and friends) from utils
  3. Terminal functions from utils
  4. ColorScheme, which has functions useful for TablePrinter
  5. iostreams, which also facilitates a lot of functionality and testing. Though, quite a bit is specific to cli/cli so I wonder if it could be simplified without a ton of work in cli/cli. Is the goal for cli/cli to also use this hypothetical cli/go-terminal? Seems less maintenance long-term.
heaths commented 2 years ago

Maybe it's outside the realm of something terminal-focused - perhaps not - but you also have a lot of great Cobra Command extensions like the one I ended up copying here: https://github.com/heaths/gh-projects/blob/fad0aedddce5a5f1a49616fb694fc8c230cdd9d2/internal/cmd/flags.go

samcoe commented 2 years ago

@heaths Appreciate the proposal. Just wanted to circle back and let you know that this work made it into our roadmap https://github.com/orgs/cli/projects/3/views/1 and it is being tracked in https://github.com/cli/cli/issues/5544. Also, that @mislav will be the one picking up this work.

heaths commented 2 years ago

FWIW, I've been cribbing / rewriting some of the table printing functionality in heaths/gh-projects (Projects beta V2 support) and would love to help out with this work (and then use it) if possible. /cc @mislav

samcoe commented 1 year ago

Kind of hard to parse through this thread but I believe we have added or decided not to add everything that was mentioned here. Going to close this out for now. I think anything that is left over can get its own dedicated issue for easier tracking.