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.

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

Optionally suppress pretty-printing ? #90

Closed
eddelbuettel opened this issue Aug 23, 2021 · 17 comments
Closed

Optionally suppress pretty-printing ? #90

eddelbuettel opened this issue Aug 23, 2021 · 17 comments

Comments

@eddelbuettel
Copy link
Contributor

As you know, I luuv tinytest and its pretty output. However, the pretty-on-stdout output can be a pain in logs. I am currently running under valgrind, pipeing out to stdout tee somelogfile.txt and it just does not look great in the logfile.

Would it make sense to allow suppression of the output?

Feel free to shoot this down 'go away, this is stoopid' as has happend to a few other suggestions I have made (mostly in private over Slack DMs) (though maybe one or two made to features). Thanks again for all things tinytest.

@markvanderloo
Copy link
Owner

There is an option to set the number of outputs that is are printed 'completely', while the rest is printed as single-lune output.

Try tt.pr.nlong=0 and tt.pr.limit=10. By default only failing tests and sidefx are printed but you can print all output with tt.pr.passes=TRUE. Color can be toggled with tt.pr.color

@eddelbuettel
Copy link
Contributor Author

Yes, sorry, I somehow (doh!!) managed to not think of looking at the docs. I also suspected verbosity may help. Will play.

@markvanderloo
Copy link
Owner

No problem. Let me know if it works. The verbose argument is for controlling output while tests are running.

@MichaelChirico
Copy link

Color can be toggled with tt.pr.color

Any consideration for auto-detecting whether color output is supported? {cli} (as well as {crayon}) does so, see cli::num_ansi_colors(). The option is nice for contexts we can control, but still leaves ugly-looking logs in more general contexts.

@MichaelChirico
Copy link

See tdhock/data.table-revdeps#33 -- is it possible to set this option for R CMD check?

@markvanderloo
Copy link
Owner

There is some auto-detection for 'dumb' terminals through an environment variable (which apparently is not set at CRAN). Perhaps we should look more explicitly for support of ANSI color support. We'll not depend on cli for this, because it is tinytest after all.

@eddelbuettel
Copy link
Contributor Author

That should actually make it operational for data.table too: just expose the TERM variable. Using my preferred wrapper from littler shows colours used by default, and suppressed if TERM=dumb:

image

Apologies for a screenshot like a rube and noob but we are talking colours here....

@markvanderloo
Copy link
Owner

I looked it up, it's in the .onLoad() function:

.onLoad <- function(libname, pkgname){
  # turn off color printing for dumb terminals
  term <- tolower(trimws(Sys.getenv("TERM")))
  if ( identical( term, "dumb" ) ){
    options(tt.pr.color=FALSE)
  }
}

We could improve this by looking at a more reliable way of detecting whether colors are supported.

@markvanderloo
Copy link
Owner

markvanderloo commented Jan 7, 2025

Looked into this a little bit, and it ain't pretty. See e.g. this SO answer and the code used in CLI.

Basically, you need to explicitly have data on all possible environments and perform specific checks on all of them. I do think we could look for a NO_COLOR environment variable, since that seems to be a kind of standard.

@MichaelChirico
Copy link

I wonder if you'd be open to depending on a tiny package which extracts the {cli} logic for use elsewhere (something like {termcolors} which can then be imported by {cli} and {tinytest} both). It does seem like a pretty useful bit of logic on its own without needing the rest of {cli} to come with.

If that sounds amenable I can follow up with Gabor and see what happens.

For now, a NO_COLOR envvar sounds appropriate too.

@markvanderloo
Copy link
Owner

the CLI code depends on rlang which would introduce unnecessary extra (and heavy!) dependencies. I will add the NO_COLOR envvar to the checks.

In general, it is difficult to detect on what system you are because you have no control over how the owners configure it. However you can very easily make it detectable whether you are on your own machine.

@MichaelChirico
Copy link

the CLI code depends on {rlang}

Hmm, AFAICT (the code really gets quite dizzying at some point with rstudio_detect()), the dependency on {rlang} is really lightweight. It amounts to this: get_real_output() > cli_output_connection() > rlang::is_interactive()

https://github.com/r-lib/cli/blob/8320914f110ddaa5f3004f2a4421535661b18ea1/R/tty.R#L32-L34

Anyway, NO_COLOR is a good option.

@MichaelChirico
Copy link

Oh, even that's not right -- is_interactive() is not {rlang}-based:

https://github.com/r-lib/cli/blob/8320914f110ddaa5f3004f2a4421535661b18ea1/R/defer.R#L213-L225

Actually, {rlang} is just Suggests for {cli}, which in terms of strong dependencies is pretty lightweight:

Depends: 
    R (>= 3.4)
Imports:
    utils

@MichaelChirico
Copy link

Went ahead and posed r-lib/cli#747, regardless of whether it might be be adopted here, I think it would be a useful package to exist.

@eddelbuettel
Copy link
Contributor Author

What is wrong with (base R's) interactive() ?

@MichaelChirico
Copy link

MichaelChirico commented Jan 17, 2025

rlang::interactive() accounts for {knitr} or {testthat} being in progress. It also allows tweaking interactivity with an option (facilitates testing, I assume).

https://rlang.r-lib.org/reference/is_interactive.html

The implementation is only a few lines so it's easy to read:

https://github.com/r-lib/rlang/blob/fa4fcbd582373c9b6389426160beb789e5e236c5/R/state.R#L111-L129

cli:::is_interactive() basically copies that logic.

@MichaelChirico
Copy link

MichaelChirico commented Jan 21, 2025

In response to the request for a standalone color detection package, Gabor pointed to {crayon}, which is R-only and pretty lightweight (about 40K). It's superseded but will receive continued updates for keeping the color detection in sync with {cli}.

If you're supportive I can try to incorporate that to {tinytest} under a Suggests dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants