From 92900a3466792b2d65a25ec1adec055f6b5a1780 Mon Sep 17 00:00:00 2001 From: Lech Lorens Date: Thu, 10 Jan 2019 22:33:51 +0100 Subject: [PATCH 1/3] Add coloring for review state in "git appraise list" This commit adds different color highlighting to different review states in the output of "git-appraise list". The default colors are: - abandon: magenta, - accepted: green, - danger: yellow red bold blink, - pending: cyan, - rejected: yellow red bold strike, - submitted: yellow, - tbr: red white bold blink. They can be modified by the user by setting the corresponding configuration options in the color.appraise section. E.g. to use magenta for the "pending" state, set color.appraise.pending to "magenta": [color "appraise"] pending = magenta --- commands/list.go | 2 +- commands/output/output.go | 35 ++++++++++++++++++++++++++++++++--- repository/git.go | 34 ++++++++++++++++++++++++++++++++++ repository/mock_repo.go | 8 ++++++++ repository/repo.go | 3 +++ 5 files changed, 78 insertions(+), 4 deletions(-) diff --git a/commands/list.go b/commands/list.go index cc9338dd..9936052c 100644 --- a/commands/list.go +++ b/commands/list.go @@ -57,7 +57,7 @@ func listReviews(repo repository.Repo, args []string) error { return nil } for _, r := range reviews { - output.PrintSummary(&r) + output.PrintSummary(&r, &repo) } return nil } diff --git a/commands/output/output.go b/commands/output/output.go index 4613cd38..0cc4da8a 100644 --- a/commands/output/output.go +++ b/commands/output/output.go @@ -23,6 +23,7 @@ import ( "strings" "time" + "github.com/google/git-appraise/repository" "github.com/google/git-appraise/review" ) @@ -52,6 +53,15 @@ status: %s // Number of lines of context to print for inline comments contextLineCount = 5 ) +var default_color = map[string]string{ + "tbr": "red white bold blink", + "pending": "cyan", + "submitted": "yellow", + "accepted": "green", + "danger": "yellow red bold blink", + "abandon": "magenta", + "rejected": "yellow red bold strike", +} // getStatusString returns a human friendly string encapsulating both the review's // resolved status, and its submitted status. @@ -78,10 +88,29 @@ func getStatusString(r *review.Summary) string { } // PrintSummary prints a single-line summary of a review. -func PrintSummary(r *review.Summary) { +func PrintSummary(r *review.Summary, repo *repository.Repo) { + var use_color bool = false + + if repo != nil { + use_color = (*repo).GetColorBool("color.appraise") + } + statusString := getStatusString(r) indentedDescription := strings.Replace(r.Request.Description, "\n", "\n ", -1) - fmt.Printf(reviewSummaryTemplate, statusString, r.Revision, indentedDescription) + + var colorOn string = "" + var colorOff string = "" + if use_color { + defaultColor, _ := default_color[statusString] + colorOn = (*repo).GetColor( + fmt.Sprintf("color.appraise.%s", statusString), + defaultColor, + ) + colorOff = "\033[00m" + } + coloredStatusString := fmt.Sprintf("%s%s%s", colorOn, statusString, colorOff) + + fmt.Printf(reviewSummaryTemplate, coloredStatusString, r.Revision, indentedDescription) } // reformatTimestamp takes a timestamp string of the form "0123456789" and changes it @@ -184,7 +213,7 @@ func printComments(r *review.Review) error { // PrintDetails prints a multi-line overview of a review, including all comments. func PrintDetails(r *review.Review) error { - PrintSummary(r.Summary) + PrintSummary(r.Summary, nil) fmt.Printf(reviewDetailsTemplate, r.Request.ReviewRef, r.Request.TargetRef, strings.Join(r.Request.Reviewers, ", "), r.Request.Requester, r.GetBuildStatusMessage()) diff --git a/repository/git.go b/repository/git.go index 31d27ea6..9e6cefb4 100644 --- a/repository/git.go +++ b/repository/git.go @@ -985,3 +985,37 @@ func (repo *GitRepo) FetchAndReturnNewReviewHashes(remote, notesRefPattern, } return updatedReviews, nil } + +func (repo *GitRepo) GetColorBool(name string) bool { + ok := repo.runGitCommandInline("config", "--get-colorbool", name) + return (ok == nil) +} + +func (repo *GitRepo) GetColor(name, default_value string) string { + var res string + var ok error + if default_value == "" { + res, ok = repo.runGitCommand( + "config", + "--type=color", + "-z", + "--get", + name, + ) + } else { + res, ok = repo.runGitCommand( + "config", + "--type=color", + "-z", + "--default", + default_value, + "--get", + name, + ) + } + + if ok != nil { + return "" + } + return res +} diff --git a/repository/mock_repo.go b/repository/mock_repo.go index 2d8debe4..67da06d4 100644 --- a/repository/mock_repo.go +++ b/repository/mock_repo.go @@ -611,3 +611,11 @@ func (repo *mockRepoForTest) FetchAndReturnNewReviewHashes(remote, notesRefPatte archiveRefPattern string) ([]string, error) { return nil, nil } + +func (repo *mockRepoForTest) GetColorBool(name string) bool { + return false +} + +func (repo *mockRepoForTest) GetColor(name, default_value string) string { + return "" +} diff --git a/repository/repo.go b/repository/repo.go index 91acd177..ebe466a5 100644 --- a/repository/repo.go +++ b/repository/repo.go @@ -218,4 +218,7 @@ type Repo interface { // changed because the _names_ of these files correspond to the revisions // they point to. FetchAndReturnNewReviewHashes(remote, notesRefPattern, archiveRefPattern string) ([]string, error) + + GetColorBool(name string) bool + GetColor(name, default_value string) string } From da84096691df278ef70de92403300c88a08e48ee Mon Sep 17 00:00:00 2001 From: Lech Lorens Date: Wed, 16 Jan 2019 23:14:38 +0100 Subject: [PATCH 2/3] Follow code review comments. --- commands/list.go | 2 +- commands/output/output.go | 52 +++++++++++++++++++++++++-------------- repository/git.go | 37 +++++----------------------- repository/mock_repo.go | 8 +++--- repository/repo.go | 9 ++++--- 5 files changed, 50 insertions(+), 58 deletions(-) diff --git a/commands/list.go b/commands/list.go index 9936052c..cc9338dd 100644 --- a/commands/list.go +++ b/commands/list.go @@ -57,7 +57,7 @@ func listReviews(repo repository.Repo, args []string) error { return nil } for _, r := range reviews { - output.PrintSummary(&r, &repo) + output.PrintSummary(&r) } return nil } diff --git a/commands/output/output.go b/commands/output/output.go index 0cc4da8a..c09bd036 100644 --- a/commands/output/output.go +++ b/commands/output/output.go @@ -87,30 +87,44 @@ func getStatusString(r *review.Summary) string { return "rejected" } -// PrintSummary prints a single-line summary of a review. -func PrintSummary(r *review.Summary, repo *repository.Repo) { - var use_color bool = false +func getColoredStatusString(repo repository.Repo, statusString string) string { + useColor, err := repo.GetColorBool("color.appraise") + if err != nil || !useColor { + return statusString + } - if repo != nil { - use_color = (*repo).GetColorBool("color.appraise") + var colorOn string + var colorOff string + + defaultColor, _ := default_color[statusString] + colorOn, err = repo.GetColor( + fmt.Sprintf("color.appraise.%s", statusString), + defaultColor, + ) + if err != nil { + return statusString } - statusString := getStatusString(r) - indentedDescription := strings.Replace(r.Request.Description, "\n", "\n ", -1) + colorOff, err = repo.GetColor("", "reset") - var colorOn string = "" - var colorOff string = "" - if use_color { - defaultColor, _ := default_color[statusString] - colorOn = (*repo).GetColor( - fmt.Sprintf("color.appraise.%s", statusString), - defaultColor, - ) - colorOff = "\033[00m" + if err != nil { + return statusString } - coloredStatusString := fmt.Sprintf("%s%s%s", colorOn, statusString, colorOff) - fmt.Printf(reviewSummaryTemplate, coloredStatusString, r.Revision, indentedDescription) + return fmt.Sprintf("%s%s%s", colorOn, statusString, colorOff) +} + +// PrintSummary prints a single-line summary of a review. +func PrintSummary(r *review.Summary) { + statusString := getStatusString(r) + indentedDescription := strings.Replace(r.Request.Description, "\n", "\n ", -1) + + fmt.Printf( + reviewSummaryTemplate, + getColoredStatusString(r.Repo, statusString), + r.Revision, + indentedDescription, + ) } // reformatTimestamp takes a timestamp string of the form "0123456789" and changes it @@ -213,7 +227,7 @@ func printComments(r *review.Review) error { // PrintDetails prints a multi-line overview of a review, including all comments. func PrintDetails(r *review.Review) error { - PrintSummary(r.Summary, nil) + PrintSummary(r.Summary) fmt.Printf(reviewDetailsTemplate, r.Request.ReviewRef, r.Request.TargetRef, strings.Join(r.Request.Reviewers, ", "), r.Request.Requester, r.GetBuildStatusMessage()) diff --git a/repository/git.go b/repository/git.go index 9e6cefb4..ba990763 100644 --- a/repository/git.go +++ b/repository/git.go @@ -986,36 +986,11 @@ func (repo *GitRepo) FetchAndReturnNewReviewHashes(remote, notesRefPattern, return updatedReviews, nil } -func (repo *GitRepo) GetColorBool(name string) bool { - ok := repo.runGitCommandInline("config", "--get-colorbool", name) - return (ok == nil) -} - -func (repo *GitRepo) GetColor(name, default_value string) string { - var res string - var ok error - if default_value == "" { - res, ok = repo.runGitCommand( - "config", - "--type=color", - "-z", - "--get", - name, - ) - } else { - res, ok = repo.runGitCommand( - "config", - "--type=color", - "-z", - "--default", - default_value, - "--get", - name, - ) - } +func (repo *GitRepo) GetColorBool(name string) (bool, error) { + err := repo.runGitCommandInline("config", "--get-colorbool", name) + return (err == nil), nil +} - if ok != nil { - return "" - } - return res +func (repo *GitRepo) GetColor(name, defaultValue string) (string, error) { + return repo.runGitCommand("config", "--get-color", name, defaultValue) } diff --git a/repository/mock_repo.go b/repository/mock_repo.go index 67da06d4..c959526c 100644 --- a/repository/mock_repo.go +++ b/repository/mock_repo.go @@ -612,10 +612,10 @@ func (repo *mockRepoForTest) FetchAndReturnNewReviewHashes(remote, notesRefPatte return nil, nil } -func (repo *mockRepoForTest) GetColorBool(name string) bool { - return false +func (repo *mockRepoForTest) GetColorBool(name string) (bool, error) { + return false, nil } -func (repo *mockRepoForTest) GetColor(name, default_value string) string { - return "" +func (repo *mockRepoForTest) GetColor(name, default_value string) (string, error) { + return "", nil } diff --git a/repository/repo.go b/repository/repo.go index ebe466a5..5c4e2db3 100644 --- a/repository/repo.go +++ b/repository/repo.go @@ -51,6 +51,12 @@ type Repo interface { // GetSubmitStrategy returns the way in which a review is submitted GetSubmitStrategy() (string, error) + // GetColorBool returns color setting for "name" (e.g. color.diff). + GetColorBool(name string) (bool, error) + + // GetColor returns color configured for "name" (e.g. color.diff.new). + GetColor(name, default_value string) (string, error) + // HasUncommittedChanges returns true if there are local, uncommitted changes. HasUncommittedChanges() (bool, error) @@ -218,7 +224,4 @@ type Repo interface { // changed because the _names_ of these files correspond to the revisions // they point to. FetchAndReturnNewReviewHashes(remote, notesRefPattern, archiveRefPattern string) ([]string, error) - - GetColorBool(name string) bool - GetColor(name, default_value string) string } From fa20e5e1e09588e1927cd728803f5c598df8d02c Mon Sep 17 00:00:00 2001 From: Lech Lorens Date: Thu, 17 Jan 2019 21:16:47 +0100 Subject: [PATCH 3/3] More naming problems. --- commands/output/output.go | 8 ++++---- repository/mock_repo.go | 2 +- repository/repo.go | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/commands/output/output.go b/commands/output/output.go index c09bd036..6e52e0ae 100644 --- a/commands/output/output.go +++ b/commands/output/output.go @@ -53,7 +53,7 @@ status: %s // Number of lines of context to print for inline comments contextLineCount = 5 ) -var default_color = map[string]string{ +var defaultColors = map[string]string{ "tbr": "red white bold blink", "pending": "cyan", "submitted": "yellow", @@ -87,7 +87,7 @@ func getStatusString(r *review.Summary) string { return "rejected" } -func getColoredStatusString(repo repository.Repo, statusString string) string { +func getColorStatusString(repo repository.Repo, statusString string) string { useColor, err := repo.GetColorBool("color.appraise") if err != nil || !useColor { return statusString @@ -96,7 +96,7 @@ func getColoredStatusString(repo repository.Repo, statusString string) string { var colorOn string var colorOff string - defaultColor, _ := default_color[statusString] + defaultColor, _ := defaultColors[statusString] colorOn, err = repo.GetColor( fmt.Sprintf("color.appraise.%s", statusString), defaultColor, @@ -121,7 +121,7 @@ func PrintSummary(r *review.Summary) { fmt.Printf( reviewSummaryTemplate, - getColoredStatusString(r.Repo, statusString), + getColorStatusString(r.Repo, statusString), r.Revision, indentedDescription, ) diff --git a/repository/mock_repo.go b/repository/mock_repo.go index c959526c..18b5f637 100644 --- a/repository/mock_repo.go +++ b/repository/mock_repo.go @@ -616,6 +616,6 @@ func (repo *mockRepoForTest) GetColorBool(name string) (bool, error) { return false, nil } -func (repo *mockRepoForTest) GetColor(name, default_value string) (string, error) { +func (repo *mockRepoForTest) GetColor(name, defaultValue string) (string, error) { return "", nil } diff --git a/repository/repo.go b/repository/repo.go index 5c4e2db3..7836aba9 100644 --- a/repository/repo.go +++ b/repository/repo.go @@ -55,7 +55,7 @@ type Repo interface { GetColorBool(name string) (bool, error) // GetColor returns color configured for "name" (e.g. color.diff.new). - GetColor(name, default_value string) (string, error) + GetColor(name, defaultValue string) (string, error) // HasUncommittedChanges returns true if there are local, uncommitted changes. HasUncommittedChanges() (bool, error)