jsdelivr / globalping-cli

A simple CLI tool to run networking commands remotely from hundreds of globally distributed servers
Mozilla Public License 2.0
117 stars 14 forks source link

Restore stdout/stderr handling #117

Closed MartinKolarik closed 2 weeks ago

MartinKolarik commented 1 month ago

It seems that through some refactoring, we lost changes from #55 and #56. Let's restore them. Things that should go to stderr:

MartinKolarik commented 1 month ago

It also seems like a good idea to refactor the handling of colored output. Right now, we have conditions like this all over the place:

if r.ctx.CIMode {
    r.printer.Printf("> %s\n", e.Message)
} else {
    r.printer.Printf(r.printer.Color("> "+e.Message, view.ColorLightYellow) + "\n")
}

It seems easy to forget. Would be better if the Color function had a required bool param like useColor and would simply return the original string when called with false.

radulucut commented 1 month ago

@MartinKolarik, outputting to stderr will cause this behaviour: #94, are we ok with that? Wouldn't make more sense to have a separate flag for this, i.e. --noInfo?

MartinKolarik commented 1 month ago

Do you mean it'll cause issues specifically with less or with the CI mode or piping in general?

radulucut commented 1 month ago

with less (and piping in general, it happens with more, head ...), it's unrelated to CI mode

MartinKolarik commented 1 month ago

Ok, I see. I most likely misunderstood that issue back then since the description said CI, and you didn't really describe the changes in the fix... I'll check this a little later.

MartinKolarik commented 1 month ago

@radulucut let's do this. It seems the previous behavior was in line with the idea from https://github.com/jsdelivr/globalping-cli/issues/56#issue-1685200265, and the other issue was just a misunderstanding.