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

Inconsistent behaviour in cli::cli_inform() #682

Closed
serkor1 opened this issue Apr 6, 2024 · 8 comments
Closed

Inconsistent behaviour in cli::cli_inform() #682

serkor1 opened this issue Apr 6, 2024 · 8 comments

Comments

@serkor1
Copy link

serkor1 commented Apr 6, 2024

The rlang::inform()-wrapper in the cli-package have a different behavior than using it directly, see the two examples below,

Using rlang::inform()

rlang::inform(
  message = cli::rule(
    left  = "left",
    right = "right"
  )
)
── left ─── right ──

Using cli::cli_inform()

cli::cli_inform(
  message = cli::rule(
    left  = "left",
    right = "right"
  )
)
── left ───── right
──

If I only specify left, the following happens,

cli::cli_inform(
  message = cli::rule(
    left  = "left"
  )
)
── left 
─────

This behavior is, as far as my tests go, independent of console_width(). Am I using cli::cli_inform incorrectly, or is this a bug?

@gaborcsardi
Copy link
Member

gaborcsardi commented Apr 6, 2024

I don't know if this is a bug, cli::cli_inform() wraps the next, rlang::inform() does not. Your rule is exactly as wide as the terminal width, so it will be wrapped, because of wrapper is conservative and does not create lines that are as wide as the console width.

The root of the issue is that rule() does not play very well with cli_inform() and similar functions, because rule() assumes no wrapping. You could create a shorter rule, but that might fail (= get wrapped) as well, e.g. if there is indentation or a label in cli_inform():

cli::cli_bullets(c("!" = cli::rule(left = "left")))
! ── left
  ───────────────────────────
cli::cli_bullets(c("!" = cli::rule(left = "left", width = cli::console_width() - 2)))
! ── left
  ─────────────────────────

@serkor1
Copy link
Author

serkor1 commented Apr 6, 2024

I see your point - however, a fair expectation is that the wrapper function behaves as the wrapped function, with less arguments. The rlang::inform()-function has no issues dealing with cli::rule(), at all.

And considering that there is an interest in using cli for packageStartupMessage instead of using rlang directly, I think it would be a great feature that we can get consistent behavior between these two functions!

  • See for example here!

Also, I love this library - and its just an awesome feature that I think should be implemented, honestly! 💯

@gaborcsardi
Copy link
Member

Neither functions are wrappers of the other, they are actually quite different. E.g.

options(cli.width = 30, width = 30)
txt <- cli:::lorem_ipsum(1, 4)
cli::cli_inform(txt)
Commodo et deserunt et dolor
enim aliqua tempor pariatur.
Tempor anim nisi commodo
labore nostrud do dolore non
enim pariatur ad incididunt.
Enim cupidatat nisi esse sit
fugiat enim voluptate sit
consectetur incididunt ex.
rlang::inform(txt)
Commodo et deserunt et dolor enim aliqua tempor pariatur. Tempor anim nisi commodo labore nostrud do dolore non enim pariatur ad incididunt. Enim cupidatat nisi esse sit fugiat enim voluptate sit consectetur incididunt ex.

Also perhaps most importantly:

cli::cli_inform("letters: {letters[1:5]}")
letters: a, b, c, d, and e
rlang::inform("letter{?s}: {letters[1:5]}")
letter{?s}: {letters[1:5]}

@serkor1
Copy link
Author

serkor1 commented Apr 6, 2024

This is an interesting comparison, thank you for clarifying the different behaviour. I was referring to your code below,

cli/R/rlang.R

Lines 67 to 72 in bcb5c78

cli_inform <- function(message, ..., .envir = parent.frame()) {
rlang::inform(
format_message(message, .envir = .envir),
...
)
}

and labelled it as a wrapper-function. When reading the documentation I get the idea that its essentially rlang::inform() with all the cool features of cli.

But I take from your examples and comments that I am wrong about this, yes?

@gaborcsardi
Copy link
Member

with all the cool features of cli.

format_message() does a lot of things there, though. E.g. it wraps the lines, which means that it will not play well with cli::rule().

@serkor1
Copy link
Author

serkor1 commented Apr 6, 2024

Yeah, I made a fork to check it out - way above my skills! 🤣 Ok, Ill leave this issue then. If you ever decide to add something like the above, without the wraps then know that I, for one, would be over-joyous!

Amazing work, honestly!

@gaborcsardi
Copy link
Member

In the next major version of cli you'll be able to embed cli_rule() into cli_bullets(), etc.

@serkor1
Copy link
Author

serkor1 commented Apr 6, 2024

So I can essentially use cli for packageStartupMessages with cli::rule()?

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

2 participants