Add interactive select in gist view#3008
Conversation
8e90495 to
68d7277
Compare
|
Hi @vilmibm, this PR is ready to review now. Thanks! The format would be like: ? Select a gist [Use arrows to move, type to filter]
> foo.txt (about 13 minutes ago)
gistfile.txt (about 13 minutes ago) |
| } | ||
|
|
||
| if gistID == "" { | ||
| return nil |
There was a problem hiding this comment.
Since gist list simply return when there are no any gists, so I keep the behavior consistent in gist command.
Or message like "There are no gists from [username]" should be returned?
There was a problem hiding this comment.
I think a message saying that the user has no gists would be a nice improvement, here.
There was a problem hiding this comment.
Would it be better to display current username? Or just "There are no gists from current user".
If showing username is preferred, is there any way to access current username other than passing in parameter to viewRun?
vilmibm
left a comment
There was a problem hiding this comment.
just noted potential for some improvements, nice work
pkg/cmd/gist/list/http.go
Outdated
| ) | ||
|
|
||
| func listGists(client *http.Client, hostname string, limit int, visibility string) ([]shared.Gist, error) { | ||
| func ListGists(client *http.Client, hostname string, limit int, visibility string) ([]shared.Gist, error) { |
There was a problem hiding this comment.
If this is going to be shared it should be moved to pkg/cmd/gist/shared/shared.go
There was a problem hiding this comment.
Already moved to correct location.
| } | ||
|
|
||
| if gistID == "" { | ||
| return nil |
There was a problem hiding this comment.
I think a message saying that the user has no gists would be a nice improvement, here.
pkg/cmd/gist/view/view.go
Outdated
| } | ||
| } | ||
| gistTime := utils.FuzzyAgo(time.Since(gist.UpdatedAt)) | ||
| opt := fmt.Sprintf("%s (%s)", text.ReplaceExcessiveWhitespace(description), gistTime) |
There was a problem hiding this comment.
It's still possible for description to end up blank which looks weird in the prompt; how about we try setting description to something like "<no description>" if it still ends up empty at this point?
There was a problem hiding this comment.
Updated gist view
Question:
I use gistfile1.txt as filename since no filename was not provided by user when creating gist. Which prevents from showing blank at the beginning of the row. Or should I use something like "<no filename>" instead of meaningless gistfile1.txt?
Current gist list
Question:
Also, I found current gist list won't show anything if both filename and description are empty. Is it desired result?
GitHub UI gists
Reference:
GitHub UI gists listing page uses gist:[node_name] when there's no filename:
There was a problem hiding this comment.
My concern was the blank at the start of the line; just using gistfile is fine for now.
| } | ||
| } | ||
|
|
||
| func Test_promptGists(t *testing.T) { |
There was a problem hiding this comment.
these are good tests 👍 please also add one test to the gist view tests just to ensure that we go into interactive mode when no ID is supplied (it doesn't have to test anything beyond that since you're doing that here).
There was a problem hiding this comment.
Tests added. Could you please have a look? Thanks.
68d7277 to
2956840
Compare
2956840 to
5403a37
Compare
vilmibm
left a comment
There was a problem hiding this comment.
I went ahead and resolved the merge conflict and resolved the final small tweaks myself. Thank you!
Fix #2999