refactor: just disable color output rather than tracking terminal wid…

…th (#1087) This lets us drop some conditionals - weirdly it doesn't seem to work if we move the conditional `text.DisableColors` call into `reporter.New` even though that does work for the vertical reporter...
google · Jul 2, 2024 · 8f2c059 · 8f2c059
1 parent 430321b
commit 8f2c059
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 17 deletions.
diff --git a/internal/output/markdowntable.go b/internal/output/markdowntable.go
@@ -4,15 +4,17 @@ import (
 	"io"
 
 	"github.com/google/osv-scanner/pkg/models"
-
 	"github.com/jedib0t/go-pretty/v6/table"
+	"github.com/jedib0t/go-pretty/v6/text"
 )
 
 // PrintTableResults prints the osv scan results into a human friendly table.
 func PrintMarkdownTableResults(vulnResult *models.VulnerabilityResults, outputWriter io.Writer) {
+	text.DisableColors()
+
 	outputTable := table.NewWriter()
 	outputTable.SetOutputMirror(outputWriter)
-	outputTable = tableBuilder(outputTable, vulnResult, false)
+	outputTable = tableBuilder(outputTable, vulnResult)
 
 	if outputTable.Length() != 0 {
 		outputTable.RenderMarkdown()

diff --git a/internal/output/table.go b/internal/output/table.go
@@ -24,9 +24,13 @@ const OSVBaseVulnerabilityURL = "https://osv.dev/"
 
 // PrintTableResults prints the osv scan results into a human friendly table.
 func PrintTableResults(vulnResult *models.VulnerabilityResults, outputWriter io.Writer, terminalWidth int) {
+	if terminalWidth <= 0 {
+		text.DisableColors()
+	}
+
 	// Render the vulnerabilities.
 	outputTable := newTable(outputWriter, terminalWidth)
-	outputTable = tableBuilder(outputTable, vulnResult, terminalWidth > 0)
+	outputTable = tableBuilder(outputTable, vulnResult)
 	if outputTable.Length() != 0 {
 		outputTable.Render()
 	}
@@ -44,25 +48,27 @@ func newTable(outputWriter io.Writer, terminalWidth int) table.Writer {
 	outputTable := table.NewWriter()
 	outputTable.SetOutputMirror(outputWriter)
 
-	if terminalWidth > 0 { // If output is a terminal, set max length to width and add styling
+	outputTable.Style().Options.DoNotColorBordersAndSeparators = true
+	outputTable.Style().Color.Row = text.Colors{text.Reset, text.BgHiBlack}
+	outputTable.Style().Color.RowAlternate = text.Colors{text.Reset, text.BgBlack}
+
+	// use fancy characters if we're outputting to a terminal
+	if terminalWidth > 0 {
 		outputTable.SetStyle(table.StyleRounded)
-		outputTable.Style().Color.Row = text.Colors{text.Reset, text.BgHiBlack}
-		outputTable.Style().Color.RowAlternate = text.Colors{text.Reset, text.BgBlack}
-		outputTable.Style().Options.DoNotColorBordersAndSeparators = true
 		outputTable.SetAllowedRowLength(terminalWidth)
-	} // Otherwise use default ascii (e.g. getting piped to a file)
+	}
 
 	return outputTable
 }
 
-func tableBuilder(outputTable table.Writer, vulnResult *models.VulnerabilityResults, addStyling bool) table.Writer {
+func tableBuilder(outputTable table.Writer, vulnResult *models.VulnerabilityResults) table.Writer {
 	outputTable.AppendHeader(table.Row{"OSV URL", "CVSS", "Ecosystem", "Package", "Version", "Source"})
-	rows := tableBuilderInner(vulnResult, addStyling, true)
+	rows := tableBuilderInner(vulnResult, true)
 	for _, elem := range rows {
 		outputTable.AppendRow(elem.row, table.RowConfig{AutoMerge: elem.shouldMerge})
 	}
 
-	uncalledRows := tableBuilderInner(vulnResult, addStyling, false)
+	uncalledRows := tableBuilderInner(vulnResult, false)
 	if len(uncalledRows) == 0 {
 		return outputTable
 	}
@@ -83,7 +89,7 @@ type tbInnerResponse struct {
 	shouldMerge bool
 }
 
-func tableBuilderInner(vulnResult *models.VulnerabilityResults, addStyling bool, calledVulns bool) []tbInnerResponse {
+func tableBuilderInner(vulnResult *models.VulnerabilityResults, calledVulns bool) []tbInnerResponse {
 	allOutputRows := []tbInnerResponse{}
 	workingDir := mustGetWorkingDirectory()
 
@@ -107,11 +113,7 @@ func tableBuilderInner(vulnResult *models.VulnerabilityResults, addStyling bool,
 				var links []string
 
 				for _, vuln := range group.IDs {
-					if addStyling {
-						links = append(links, OSVBaseVulnerabilityURL+text.Bold.EscapeSeq()+vuln+text.Reset.EscapeSeq())
-					} else {
-						links = append(links, OSVBaseVulnerabilityURL+vuln)
-					}
+					links = append(links, OSVBaseVulnerabilityURL+text.Bold.Sprintf("%s", vuln))
 				}
 
 				outputRow = append(outputRow, strings.Join(links, "\n"))