Skip to content

Conversation

@hadley
Copy link
Member

@hadley hadley commented Jul 30, 2025

Use expect_snapshot_failure() to look at more of the messaging, and then use that to make a bunch of minor improvements.

Fixes #2135

hadley added 3 commits July 30, 2025 09:55
Use `expect_snapshot_failure()` to look at more of the messaging, and then use that to make a bunch of minor improvements.
@hadley hadley requested a review from krlmlr July 30, 2025 15:16
Copy link
Member

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

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

Nice!

Comment on lines +42 to +47
check_character(object)
check_string(regexp)
check_bool(perl)
check_bool(fixed)
check_bool(all)

Copy link
Member

Choose a reason for hiding this comment

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

  • label is likely checked above by quasi_label(), do we want to check info ? It's still used in the empty case below.
  • Do you want to take this as an opportunity to add a grepl_args = list() (or = NULL ) argument and soft-deprecate ... (with a lifecycle warning)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't care that much about info since we're phasing it out in new expectations.

I don't want to do any bigger refactorings because if I did I'd really want to stop it from being vectorising and instead make it work with a single string; but that's going to break a bunch of tests.

Copy link
Member

Choose a reason for hiding this comment

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

Could we use the all argument for enforcing a scalar? all = NA or all = "scalar" would mean to expect a scalar, with a lifecycle warning to add all = TRUE if a non-scalar is provided without setting all ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it's important enough to do all that work.

hadley and others added 2 commits July 30, 2025 17:44
Co-authored-by: Kirill Müller <krlmlr@users.noreply.github.com>
@hadley hadley merged commit 2896db2 into main Jul 31, 2025
11 checks passed
@hadley hadley deleted the expect-match-escape branch July 31, 2025 12:37
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

Successfully merging this pull request may close these issues.

expect_match_ shouldn't escape label

3 participants