Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle the --color flag via proper global state #6743

Merged
merged 1 commit into from
Jul 13, 2021
Merged

Handle the --color flag via proper global state #6743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 38 additions & 43 deletions cli/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ import (
"github.com/filecoin-project/lotus/chain/actors/builtin"
"github.com/filecoin-project/lotus/chain/actors/builtin/market"
"github.com/filecoin-project/lotus/chain/types"
cliutil "github.com/filecoin-project/lotus/cli/util"
"github.com/filecoin-project/lotus/lib/tablewriter"
)

Expand Down Expand Up @@ -1234,7 +1233,6 @@ var clientListRetrievalsCmd = &cli.Command{
&cli.BoolFlag{
Name: "color",
Usage: "use color in display output",
Value: cliutil.DefaultColorUse,
DefaultText: "depends on output being a TTY",
},
&cli.BoolFlag{
Expand All @@ -1252,6 +1250,10 @@ var clientListRetrievalsCmd = &cli.Command{
},
},
Action: func(cctx *cli.Context) error {
if cctx.IsSet("color") {
color.NoColor = !cctx.Bool("color")
}

api, closer, err := GetFullNodeAPI(cctx)
if err != nil {
return err
Expand All @@ -1260,7 +1262,6 @@ var clientListRetrievalsCmd = &cli.Command{
ctx := ReqContext(cctx)

verbose := cctx.Bool("verbose")
color := cctx.Bool("color")
watch := cctx.Bool("watch")
showFailed := cctx.Bool("show-failed")
completed := cctx.Bool("completed")
Expand All @@ -1280,7 +1281,7 @@ var clientListRetrievalsCmd = &cli.Command{
tm.Clear()
tm.MoveCursor(1, 1)

err = outputRetrievalDeals(ctx, tm.Screen, localDeals, verbose, color, showFailed, completed)
err = outputRetrievalDeals(ctx, tm.Screen, localDeals, verbose, showFailed, completed)
if err != nil {
return err
}
Expand All @@ -1306,15 +1307,15 @@ var clientListRetrievalsCmd = &cli.Command{
}
}

return outputRetrievalDeals(ctx, cctx.App.Writer, localDeals, verbose, color, showFailed, completed)
return outputRetrievalDeals(ctx, cctx.App.Writer, localDeals, verbose, showFailed, completed)
},
}

func isTerminalError(status retrievalmarket.DealStatus) bool {
// should patch this in go-fil-markets but to solve the problem immediate and not have buggy output
return retrievalmarket.IsTerminalError(status) || status == retrievalmarket.DealStatusErrored || status == retrievalmarket.DealStatusCancelled
}
func outputRetrievalDeals(ctx context.Context, out io.Writer, localDeals []lapi.RetrievalInfo, verbose bool, color bool, showFailed bool, completed bool) error {
func outputRetrievalDeals(ctx context.Context, out io.Writer, localDeals []lapi.RetrievalInfo, verbose bool, showFailed bool, completed bool) error {
var deals []api.RetrievalInfo
for _, deal := range localDeals {
if !showFailed && isTerminalError(deal.Status) {
Expand Down Expand Up @@ -1350,13 +1351,13 @@ func outputRetrievalDeals(ctx context.Context, out io.Writer, localDeals []lapi.
w := tablewriter.New(tableColumns...)

for _, d := range deals {
w.Write(toRetrievalOutput(d, color, verbose))
w.Write(toRetrievalOutput(d, verbose))
}

return w.Flush(out)
}

func toRetrievalOutput(d api.RetrievalInfo, color bool, verbose bool) map[string]interface{} {
func toRetrievalOutput(d api.RetrievalInfo, verbose bool) map[string]interface{} {

payloadCID := d.PayloadCID.String()
provider := d.Provider.String()
Expand All @@ -1369,7 +1370,7 @@ func toRetrievalOutput(d api.RetrievalInfo, color bool, verbose bool) map[string
"PayloadCID": payloadCID,
"DealId": d.ID,
"Provider": provider,
"Status": retrievalStatusString(color, d.Status),
"Status": retrievalStatusString(d.Status),
"PricePerByte": types.FIL(d.PricePerByte),
"Received": units.BytesSize(float64(d.BytesReceived)),
"TotalPaid": types.FIL(d.TotalPaid),
Expand Down Expand Up @@ -1399,19 +1400,17 @@ func toRetrievalOutput(d api.RetrievalInfo, color bool, verbose bool) map[string
return retrievalOutput
}

func retrievalStatusString(c bool, status retrievalmarket.DealStatus) string {
func retrievalStatusString(status retrievalmarket.DealStatus) string {
s := retrievalmarket.DealStatuses[status]
if !c {
return s
}

if isTerminalError(status) {
switch {
case isTerminalError(status):
return color.RedString(s)
}
if retrievalmarket.IsTerminalSuccess(status) {
case retrievalmarket.IsTerminalSuccess(status):
return color.GreenString(s)
default:
return s
}
return s
}

var clientInspectDealCmd = &cli.Command{
Expand Down Expand Up @@ -1808,7 +1807,6 @@ var clientListDeals = &cli.Command{
&cli.BoolFlag{
Name: "color",
Usage: "use color in display output",
Value: cliutil.DefaultColorUse,
DefaultText: "depends on output being a TTY",
},
&cli.BoolFlag{
Expand All @@ -1821,6 +1819,10 @@ var clientListDeals = &cli.Command{
},
},
Action: func(cctx *cli.Context) error {
if cctx.IsSet("color") {
color.NoColor = !cctx.Bool("color")
}

api, closer, err := GetFullNodeAPI(cctx)
if err != nil {
return err
Expand All @@ -1829,7 +1831,6 @@ var clientListDeals = &cli.Command{
ctx := ReqContext(cctx)

verbose := cctx.Bool("verbose")
color := cctx.Bool("color")
watch := cctx.Bool("watch")
showFailed := cctx.Bool("show-failed")

Expand All @@ -1848,7 +1849,7 @@ var clientListDeals = &cli.Command{
tm.Clear()
tm.MoveCursor(1, 1)

err = outputStorageDeals(ctx, tm.Screen, api, localDeals, verbose, color, showFailed)
err = outputStorageDeals(ctx, tm.Screen, api, localDeals, verbose, showFailed)
if err != nil {
return err
}
Expand All @@ -1874,7 +1875,7 @@ var clientListDeals = &cli.Command{
}
}

return outputStorageDeals(ctx, cctx.App.Writer, api, localDeals, verbose, color, showFailed)
return outputStorageDeals(ctx, cctx.App.Writer, api, localDeals, verbose, showFailed)
},
}

Expand All @@ -1897,7 +1898,7 @@ func dealFromDealInfo(ctx context.Context, full v0api.FullNode, head *types.TipS
}
}

func outputStorageDeals(ctx context.Context, out io.Writer, full v0api.FullNode, localDeals []lapi.DealInfo, verbose bool, color bool, showFailed bool) error {
func outputStorageDeals(ctx context.Context, out io.Writer, full v0api.FullNode, localDeals []lapi.DealInfo, verbose bool, showFailed bool) error {
sort.Slice(localDeals, func(i, j int) bool {
return localDeals[i].CreationTime.Before(localDeals[j].CreationTime)
})
Expand Down Expand Up @@ -1949,7 +1950,7 @@ func outputStorageDeals(ctx context.Context, out io.Writer, full v0api.FullNode,
d.LocalDeal.ProposalCid,
d.LocalDeal.DealID,
d.LocalDeal.Provider,
dealStateString(color, d.LocalDeal.State),
dealStateString(d.LocalDeal.State),
onChain,
slashed,
d.LocalDeal.PieceCID,
Expand Down Expand Up @@ -1998,7 +1999,7 @@ func outputStorageDeals(ctx context.Context, out io.Writer, full v0api.FullNode,
"DealCid": propcid,
"DealId": d.LocalDeal.DealID,
"Provider": d.LocalDeal.Provider,
"State": dealStateString(color, d.LocalDeal.State),
"State": dealStateString(d.LocalDeal.State),
"On Chain?": onChain,
"Slashed?": slashed,
"PieceCID": piece,
Expand All @@ -2013,12 +2014,8 @@ func outputStorageDeals(ctx context.Context, out io.Writer, full v0api.FullNode,
return w.Flush(out)
}

func dealStateString(c bool, state storagemarket.StorageDealStatus) string {
func dealStateString(state storagemarket.StorageDealStatus) string {
s := storagemarket.DealStates[state]
if !c {
return s
}

switch state {
case storagemarket.StorageDealError, storagemarket.StorageDealExpired:
return color.RedString(s)
Expand Down Expand Up @@ -2339,7 +2336,6 @@ var clientListTransfers = &cli.Command{
&cli.BoolFlag{
Name: "color",
Usage: "use color in display output",
Value: cliutil.DefaultColorUse,
DefaultText: "depends on output being a TTY",
},
&cli.BoolFlag{
Expand All @@ -2356,6 +2352,10 @@ var clientListTransfers = &cli.Command{
},
},
Action: func(cctx *cli.Context) error {
if cctx.IsSet("color") {
color.NoColor = !cctx.Bool("color")
}

api, closer, err := GetFullNodeAPI(cctx)
if err != nil {
return err
Expand All @@ -2370,7 +2370,6 @@ var clientListTransfers = &cli.Command{

verbose := cctx.Bool("verbose")
completed := cctx.Bool("completed")
color := cctx.Bool("color")
watch := cctx.Bool("watch")
showFailed := cctx.Bool("show-failed")
if watch {
Expand All @@ -2384,7 +2383,7 @@ var clientListTransfers = &cli.Command{

tm.MoveCursor(1, 1)

OutputDataTransferChannels(tm.Screen, channels, verbose, completed, color, showFailed)
OutputDataTransferChannels(tm.Screen, channels, verbose, completed, showFailed)

tm.Flush()

Expand All @@ -2409,13 +2408,13 @@ var clientListTransfers = &cli.Command{
}
}
}
OutputDataTransferChannels(os.Stdout, channels, verbose, completed, color, showFailed)
OutputDataTransferChannels(os.Stdout, channels, verbose, completed, showFailed)
return nil
},
}

// OutputDataTransferChannels generates table output for a list of channels
func OutputDataTransferChannels(out io.Writer, channels []lapi.DataTransferChannel, verbose, completed, color, showFailed bool) {
func OutputDataTransferChannels(out io.Writer, channels []lapi.DataTransferChannel, verbose, completed, showFailed bool) {
sort.Slice(channels, func(i, j int) bool {
return channels[i].TransferID < channels[j].TransferID
})
Expand Down Expand Up @@ -2445,7 +2444,7 @@ func OutputDataTransferChannels(out io.Writer, channels []lapi.DataTransferChann
tablewriter.Col("Voucher"),
tablewriter.NewLineCol("Message"))
for _, channel := range sendingChannels {
w.Write(toChannelOutput(color, "Sending To", channel, verbose))
w.Write(toChannelOutput("Sending To", channel, verbose))
}
w.Flush(out) //nolint:errcheck

Expand All @@ -2459,17 +2458,13 @@ func OutputDataTransferChannels(out io.Writer, channels []lapi.DataTransferChann
tablewriter.Col("Voucher"),
tablewriter.NewLineCol("Message"))
for _, channel := range receivingChannels {
w.Write(toChannelOutput(color, "Receiving From", channel, verbose))
w.Write(toChannelOutput("Receiving From", channel, verbose))
}
w.Flush(out) //nolint:errcheck
}

func channelStatusString(useColor bool, status datatransfer.Status) string {
func channelStatusString(status datatransfer.Status) string {
s := datatransfer.Statuses[status]
if !useColor {
return s
}

switch status {
case datatransfer.Failed, datatransfer.Cancelled:
return color.RedString(s)
Expand All @@ -2480,7 +2475,7 @@ func channelStatusString(useColor bool, status datatransfer.Status) string {
}
}

func toChannelOutput(useColor bool, otherPartyColumn string, channel lapi.DataTransferChannel, verbose bool) map[string]interface{} {
func toChannelOutput(otherPartyColumn string, channel lapi.DataTransferChannel, verbose bool) map[string]interface{} {
rootCid := channel.BaseCID.String()
otherParty := channel.OtherPeer.String()
if !verbose {
Expand All @@ -2500,7 +2495,7 @@ func toChannelOutput(useColor bool, otherPartyColumn string, channel lapi.DataTr

return map[string]interface{}{
"ID": channel.TransferID,
"Status": channelStatusString(useColor, channel.Status),
"Status": channelStatusString(channel.Status),
otherPartyColumn: otherParty,
"Root Cid": rootCid,
"Initiated?": initiated,
Expand Down
10 changes: 10 additions & 0 deletions cli/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ package cli
import (
"context"
"fmt"
"os"
"time"

"github.com/fatih/color"
"github.com/hako/durafmt"
"github.com/ipfs/go-cid"
"github.com/mattn/go-isatty"

"github.com/filecoin-project/go-state-types/abi"

Expand All @@ -15,6 +18,13 @@ import (
"github.com/filecoin-project/lotus/chain/types"
)

// Set the global default, to be overridden by individual cli flags in order
func init() {
color.NoColor = os.Getenv("GOLOG_LOG_FMT") != "color" &&
!isatty.IsTerminal(os.Stdout.Fd()) &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strictly I think you should check GOLOG_OUTPUT to see if logging is being redirected to stderr here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Urgh... that's a bit of a can of worms...
Perhaps we should just make this a public method and inspect the result?
https://github.com/ipfs/go-log/blob/5b4daa4491c2cc9e0ba85ce4b5fcc913f6fcdf6b/setup.go#L274-L377

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sensible suggestion to avoid duplicating that logic

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to think about this more, that'll have to come via another PR

!isatty.IsCygwinTerminal(os.Stdout.Fd())
}

func parseTipSet(ctx context.Context, api v0api.FullNode, vals []string) (*types.TipSet, error) {
var headers []*types.BlockHeader
for _, c := range vals {
Expand Down
14 changes: 0 additions & 14 deletions cli/util/color.go
View file
Open in desktop

This file was deleted.

7 changes: 4 additions & 3 deletions cmd/lotus-shed/actor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"github.com/filecoin-project/lotus/chain/actors/builtin/miner"
"github.com/filecoin-project/lotus/chain/types"
lcli "github.com/filecoin-project/lotus/cli"
cliutil "github.com/filecoin-project/lotus/cli/util"
"github.com/filecoin-project/lotus/lib/tablewriter"
)

Expand Down Expand Up @@ -267,12 +266,14 @@ var actorControlList = &cli.Command{
},
&cli.BoolFlag{
Name: "color",
Value: cliutil.DefaultColorUse,
Usage: "use color in display output",
DefaultText: "depends on output being a TTY",
},
},
Action: func(cctx *cli.Context) error {
color.NoColor = !cctx.Bool("color")
if cctx.IsSet("color") {
color.NoColor = !cctx.Bool("color")
}

var maddr address.Address
if act := cctx.String("actor"); act != "" {
Expand Down
7 changes: 4 additions & 3 deletions cmd/lotus-storage-miner/actor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/filecoin-project/lotus/chain/actors/builtin/miner"
"github.com/filecoin-project/lotus/chain/types"
lcli "github.com/filecoin-project/lotus/cli"
cliutil "github.com/filecoin-project/lotus/cli/util"
"github.com/filecoin-project/lotus/lib/tablewriter"
)

Expand Down Expand Up @@ -390,12 +389,14 @@ var actorControlList = &cli.Command{
},
&cli.BoolFlag{
Name: "color",
Value: cliutil.DefaultColorUse,
Usage: "use color in display output",
DefaultText: "depends on output being a TTY",
},
},
Action: func(cctx *cli.Context) error {
color.NoColor = !cctx.Bool("color")
if cctx.IsSet("color") {
color.NoColor = !cctx.Bool("color")
}

nodeApi, closer, err := lcli.GetStorageMinerAPI(cctx)
if err != nil {
Expand Down
2 changes: 0 additions & 2 deletions cmd/lotus-storage-miner/info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ var infoCmd = &cli.Command{
}

func infoCmdAct(cctx *cli.Context) error {
color.NoColor = !cctx.Bool("color")

nodeApi, closer, err := lcli.GetStorageMinerAPI(cctx)
if err != nil {
return err
Expand Down
Loading