Use cli #1956

jennybc · 2024-02-29T23:46:42Z

This is a huge PR, most of which is mind-numbingly repetitive. Recommended way to review:

Read the new developer-facing article vignettes/articles/ui-cli-conversion.Rmd, which is glorified notes-to-self. I may create a blog post about this and these notes would be helpful. This will also be helpful to link to in future PRs that introduce unconventional UI. It is probably worth doing pr_fetch(1956) and rendering this because then you can enjoy the asciicast chunks that really show the before and after.
Look for the comments I sprinkled around, where I'll draw attention to a few interesting things.
utils-ui.R is the most important file, because that's where the new UI functions live.

I plan to merge this soon, but am happy to receive feedback. But I really want to get friends and family using this soon so we can put a few miles on it be fore I submit to CRAN.

Closes #956, closes #1809

jennybc · 2024-02-29T23:57:00Z

R/addin.R

@@ -15,13 +15,15 @@ use_addin <- function(addin = "new_addin", open = rlang::is_interactive()) {
  if (!file_exists(addin_dcf_path)) {
    create_directory(proj_path("inst", "rstudio"))
    file_create(addin_dcf_path)
-    ui_done("Creating {ui_path(addin_dcf_path)}")
+    ui_bullets(c("v" = "Creating {.path {pth(addin_dcf_path)}}"))


ui_bullets() is a new unexported wrapper around cli::cli_bullets() that applies a custom usethis style and honors the usethis.quiet option. All of the old "block styles" get replaced by ui_bullets().

{.path {pth(addin_dcf_path)}} is a clunky but effective pattern you will see a lot of. This is explained more fully in the article.

pth() is an internal helper that forms paths relative to a base and it's super important in usethis. But I also really, really want cli's file hyperlinks. I don't see a way to create a custom inline span that allows me to combine my pre-processsing with cli's file hyperlink support. But I'd love to learn that I'm wrong.

jennybc · 2024-03-01T00:04:39Z

R/git-default-branch.R

+  # TODO: the ui_paths used to be a nested bullet list
+  # if that ever becomes possible/easier with cli, go back to that


It would be nice to have nested bullets in cli::cli_bullets(). But AFAICT that's only possible if you're making a bullet list more "by hand" with cli_ol(), cli_ul(), cli_li(), and the like.

jennybc · 2024-03-01T00:06:17Z

R/git.R

+  ui_paths <- map_chr(paths, ui_path_impl)
+  ui_bullets(c(
+    "i" = "{cli::qty(n)}There {?is/are} {n} untracked file{?s}:",
+    bulletize(usethis_map_cli(ui_paths, template = "{.file <<x>>}"))


In several places I need to programmatically make bullet lists and, at the same time, apply inline styling. usethis_map_cli() and bulletize() are versions of similar functions I've written in, e.g., googledrive.

jennybc · 2024-03-01T00:08:42Z

R/git.R

-    kv_line("Global (user-level) gitignore file", git_ignore_path("user"))
+    kv_line(
+      "Global (user-level) gitignore file",
+      I("{.path {git_ignore_path('user')}}")


Occasionally the value of a kev-value line (which is what kv_line() handles) needs explicit inline styling and interpolation. Applying AsIs allows to me detect that. The default behaviour is to style the value as {.val {value}}.

jennybc · 2024-03-01T00:11:08Z

R/author.R

+           DESCRIPTION.",
+    " " = "usethis only supports modification of the {.field Authors@R} field.",
+    "i" = "We recommend one of these paths forward:",
+    "_" = "Delete the legacy fields and rebuild with {.fun use_author}; or",


I introduce a new bullet type with shortcode "_". This is for TODOs and is meant to evoke a place where you could check something off.

jennybc · 2024-03-01T00:38:21Z

R/utils-ui.R

  cli::cli_abort(
    message,
    class = c(class, "usethis_error"),
    .envir = .envir,
    ...
  )
 }
+
+# questions --------------------------------------------------------------------
+ui_yep <- function(x,


Very similar to ui_yeah() but cli is used for preparing utils:menu(title =, choices =).

jennybc · 2024-03-01T00:39:20Z

tests/testthat/_snaps/author.md

-      Error:
-      ! User input required, but session is not interactive.
-      Query: Do you want to cancel this operation and sort that out first?
+      Error in `ui_yep()`:


Now that usethis uses rlang::abort() indirectly, it becomes obvious that I also haven't yet threaded the caller through everywhere. That is a separate job.

jennybc · 2024-03-01T00:41:35Z

tests/testthat/_snaps/author.md

-        * Delete these fields and rebuild with `use_author()`.
-        * Convert to Authors@R with `desc::desc_coerce_authors_at_r()`,
-          then delete the legacy fields.
+      [ ] Delete the legacy fields and rebuild with `use_author()`; or


It's a little bit of a bummer that the TODO checkbox becomes [ ] when Unicode is not available, so it takes up more characters than the other bullets. But I really think we care more about making the output pretty in a Unicode setting.

jennybc · 2024-03-01T00:42:38Z

tests/testthat/_snaps/utils-github.md

@@ -9,18 +9,188 @@
      [7] "fork_cannot_push_origin"            "fork_upstream_is_not_origin_parent"
      [9] "upstream_but_origin_is_not_fork"   

-# fork_upstream_is_not_origin_parent is detected
+# 'no_github' is reported correctly


Snapshot tests that are newly possible.

jennybc · 2024-03-01T00:44:10Z

tests/testthat/test-utils-ui.R

@@ -0,0 +1,230 @@
+cli::test_that_cli("ui_bullets() look as expected", {


cli::test_that_cli() is a supercool function! 🤓

I guess temp paths vary in length, even across separate runs on the same platform?

olivroy

Looks very good! Huge amount of work.

Just need to make sure .run, .fun .code, .help are used consistently.

.run should be used for things that you almost always want to run immediately after seeing it in the current session. (no base functions ideally to avoid bad UX in RStduio) Edit: IDE won't let you run anything with extra parenthesis inside
code for base fns cli::cli_text("{.run cli::cli_bullets(c('a', ''a))}") will show a forbidden tooltip.
help when wanting to direct to help file
fun elsewhere?

R/badge.R

R/git-default-branch.R

R/git.R

R/sitrep.R

R/spelling.R

R/test.R

R/tidyverse.R

Co-authored-by: olivroy <52606734+olivroy@users.noreply.github.com>

jennybc · 2024-03-01T17:36:01Z

Thanks @olivroy! I know that my mojo and style around .run, .fun, .code, .help evolved over the course of this task and I didn't have the heart to go back and rationalize it.

I've accepted all the suggestions that I can. If you want to convert some of the remaining comments to suggestions, I would accept those too.

olivroy · 2024-03-01T17:52:51Z

Thanks @olivroy! I know that my mojo and style around .run, .fun, .code, .help evolved over the course of this task and I didn't have the heart to go back and rationalize it.

I will send a follow-up post-merge instead (if necessary). My browser has difficulty handling this PR :)

I've accepted all the suggestions that I can. If you want to convert some of the remaining comments to suggestions, I would accept those too.

Great!

hadley

I took a quick look and it looks good to me! Feel free to ignore any comments that you don't are worth the effort; I mostly wanted to draw your attention to things that I spotted not specifically request changes.

hadley · 2024-03-04T14:08:38Z

vignettes/articles/ui-cli-conversion.Rmd

+
+Summary of what I've done for todo's:
+
+-   Introduce a new bullet shortcode for a todo. Proposal: `_` (instead of `*`), which seems to be the best single ascii character that evokes a place to check something off.


[] would another option. It's not a single character but it does better evoke the checkbox.

I think I care more about the shortcode being single character and, therefore, lining up nicely with its friends in code, than about evoking the checkbox.

hadley · 2024-03-04T14:10:03Z

vignettes/articles/ui-cli-conversion.Rmd

+
+Here's the general conversion plan:
+
+-   `ui_field()` becomes `{.field blah}`. In `usethis_theme()`, I tweak the `.field` style to apply single quotes if color is not available, which is what `ui_field()` has always done.


Would be good to do this in cli, IMO.

r-lib/cli#674

hadley · 2024-03-04T14:12:27Z

vignettes/articles/ui-cli-conversion.Rmd

+    ))
+    ```
+    It would be nice to create a custom inline class, e.g. `{.ui_path {some_path}}`, which I have done in, e.g., googledrive.
+    But it's not easy to do this *while still inheriting cli's `file:` hyperlink behaviour*, which is very desirable.


What if you used format_inline() + .file in your custom inline helper? Or do you think the usethis path logic is sufficiently useful that it should move to cli?

I guess I'd rather not "explode" (as in pre-format) filepaths like this. It introduces a special behaviour that I suspect would come back to haunt me. In general, I try to keep the actual formatting as late as possible to reduce the chance of calling format_inline() under one set of conditions, but then having another set hold when the user actually sees the message.

In terms of having something similar in cli, I'm not sure how you would pass the base information to an inline style? I don't see how to pass 2 pieces of info to an inline style (display path + optional base to make it relative to).

hadley · 2024-03-04T14:19:50Z

vignettes/articles/ui-cli-conversion.Rmd

+}
+```
+
+The main point of `ui_abort()` is to use to `cli_abort()` (and to continue applying the `"usethis_error"` class).


I assume we mostly use that error class for tests?

hadley · 2024-03-04T14:23:16Z

tests/testthat/_snaps/author.md

-      Query: Do you want to cancel this operation and sort that out first?
+      Error in `ui_yep()`:
+      x User input required, but session is not interactive.
+      i Query: "Do you want to cancel this operation and sort that out first?"


Drop "query:"? I think the i bullet is now sufficient.

hadley · 2024-03-04T14:24:48Z

tests/testthat/_snaps/lifecycle.md

-      * Add badges in documentation topics by inserting one of:
+      v Adding lifecycle to 'Imports' field in DESCRIPTION.
+      [ ] Refer to functions with `lifecycle::fun()`.
+      v Adding "@importFrom lifecycle deprecated" to 'R/{TESTPKG}-package.R'.


Why the mix of double and single quotes?

This comes from cli and it's a quoting difference between {.val VALUE} and {.file some/path/to/some/thing}.

span.val -> cli_format.character -> default is "

https://github.com/r-lib/cli/blob/1b5e691a44497c8a3708ed33ab2026c640a2c66c/R/themes.R#L238-L241

... span.val = list( transform = function(x, ...) cli_format(x, ...), color = "blue" ), ...

https://github.com/r-lib/cli/blob/1b5e691a44497c8a3708ed33ab2026c640a2c66c/R/format.R#L73

cli_format.character <- function(x, style = NULL, ...) { quote <- style$`string-quote` %||% style$string_quote %||% "\"" encodeString(x, quote = quote) }

span.file -> theme_file() -> quote_weird_name -> default is '

https://github.com/r-lib/cli/blob/1b5e691a44497c8a3708ed33ab2026c640a2c66c/R/themes.R#L207

... span.file = theme_file(), ...

https://github.com/r-lib/cli/blob/1b5e691a44497c8a3708ed33ab2026c640a2c66c/R/themes.R#L365

theme_file <- function() { list( color = "blue", transform = function(x) make_link(x, type = "file"), fmt = quote_weird_name ) }

https://github.com/r-lib/cli/blob/1b5e691a44497c8a3708ed33ab2026c640a2c66c/R/themes.R#L294-L300

quote_weird_name <- function(x) { x2 <- quote_weird_name0(x) if (x2[[2]] || num_ansi_colors() == 1) { x2[[1]] <- paste0("'", x2[[1]], "'") } x2[[1]] }

jennybc added 30 commits February 19, 2024 08:46

Init article on the conversion task

1dc845c

Build up the cli-based UI in one place

f7b906a

Introduce ui_cli_bullets()

2194623

author.R

26f1d31

Factor out the path-processing logic into ui_path_impl()

7c96412

addin.R

1a63223

ui_bullets() is better because shorter

de288ca

badge.R

1db32b4

ui_abort() feels more consistent with everything else

4ab8df7

Introduce ui_code_snippet()

5827019

block.R

fa943c2

Some testing improvements

d4b4009

Test ui_code_snippet()

c8b7c62

Replace all calls to ui_code_block()

2fb1a4c

Typo

5377788

Update conversion article

4251e5c

browse.R

8a7a250

Eliminate usage of ui_warn()

46a9f22

ci.R

87783e8

code_of_conduct.R

d500a8f

course.R

da160a9

coverage.R

cd7e298

cpp11.R

fb2e179

Single quote inline .field if no color

c9ef28c

create.R

681ad46

data.R

582d271

description.R

8186323

directory.R

bf88fde

edit.R

a468a4a

git-default-branch.R

2530f03

jennybc added 2 commits February 29, 2024 15:44

Yet another straggler

a3ab22b

Deal with utils::menu() prep and other unusual stuff

b2805d2

jennybc commented Mar 1, 2024

View reviewed changes

jennybc marked this pull request as ready for review March 1, 2024 00:48

Make test less sensitive to local conditions

c462abf

jennybc requested review from hadley and gaborcsardi March 1, 2024 00:56

jennybc added 2 commits February 29, 2024 17:08

Article needs asciicast

12ee388

Increase width to forcibly prevent snapshot diff due to linebreaks

c1c9297

I guess temp paths vary in length, even across separate runs on the same platform?

jennybc mentioned this pull request Mar 1, 2024

Use cli in messages #1928

Closed

olivroy reviewed Mar 1, 2024

View reviewed changes

jennybc and others added 4 commits March 1, 2024 08:27

Organize the articles

db3c536

Update README

9143717

Make the width even larger???

d7b30f7

Apply suggestions from code review

226c4a0

Co-authored-by: olivroy <52606734+olivroy@users.noreply.github.com>

Update snapshot

4cea077

jennybc added 5 commits March 1, 2024 11:38

How about cli.width?????

0baf2b5

YOLO

3e343a3

.run fixups

210905f

These run afoul of the .run rules, so why bother

56c130c

Better captures the intent

d6a3552

hadley approved these changes Mar 4, 2024

View reviewed changes

jennybc mentioned this pull request Mar 4, 2024

Quote {.field FIELD} when there's no color r-lib/cli#674

Open

jennybc merged commit fc8472c into main Mar 4, 2024
13 checks passed

jennybc deleted the use-cli branch March 5, 2024 17:17

This was referenced Mar 6, 2024

Cli tweaks #1959

Merged

Error chaining tweaks #1963

Draft

olivroy mentioned this pull request Mar 14, 2024

usethis theme i bullet #1965

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use cli #1956

Use cli #1956

jennybc commented Feb 29, 2024 •

edited

jennybc Feb 29, 2024 •

edited

jennybc Mar 1, 2024

jennybc Mar 1, 2024

jennybc Mar 1, 2024

jennybc Mar 1, 2024

jennybc Mar 1, 2024

jennybc Mar 1, 2024

jennybc Mar 1, 2024

jennybc Mar 1, 2024

jennybc Mar 1, 2024

olivroy left a comment •

edited

jennybc commented Mar 1, 2024

olivroy commented Mar 1, 2024

hadley left a comment

hadley Mar 4, 2024

jennybc Mar 4, 2024

hadley Mar 4, 2024

jennybc Mar 4, 2024

hadley Mar 4, 2024

jennybc Mar 4, 2024

jennybc Mar 4, 2024

hadley Mar 4, 2024

jennybc Mar 4, 2024

hadley Mar 4, 2024

hadley Mar 4, 2024

jennybc Mar 4, 2024 •

edited

		# TODO: the ui_paths used to be a nested bullet list
		# if that ever becomes possible/easier with cli, go back to that

		@@ -0,0 +1,230 @@
		cli::test_that_cli("ui_bullets() look as expected", {


		Summary of what I've done for todo's:

		- Introduce a new bullet shortcode for a todo. Proposal: `_` (instead of `*`), which seems to be the best single ascii character that evokes a place to check something off.


		Here's the general conversion plan:

		- `ui_field()` becomes `{.field blah}`. In `usethis_theme()`, I tweak the `.field` style to apply single quotes if color is not available, which is what `ui_field()` has always done.

Use cli #1956

Use cli #1956

Conversation

jennybc commented Feb 29, 2024 • edited

jennybc Feb 29, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olivroy left a comment • edited

Choose a reason for hiding this comment

jennybc commented Mar 1, 2024

olivroy commented Mar 1, 2024

hadley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jennybc Mar 4, 2024 • edited

Choose a reason for hiding this comment

jennybc commented Feb 29, 2024 •

edited

jennybc Feb 29, 2024 •

edited

olivroy left a comment •

edited

jennybc Mar 4, 2024 •

edited