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

Prefer multiple functions with convenient syntax #82

Merged
merged 6 commits into from
Jan 17, 2023
Merged

Conversation

hadley
Copy link
Member

@hadley hadley commented Jan 10, 2023

Fixes #77

@ijlyttle what do you think of this interface? I'm slightly concerned that conflicts_prefer() might be confusingly close to conflict_prefer().

@hadley
Copy link
Member Author

hadley commented Jan 10, 2023

Still needs more documentation work to steer people towards conflicts_prefer().

@ijlyttle
Copy link

ijlyttle commented Jan 10, 2023

I had seen your initial message; I had got so far as to think it could be called conflict_favor() ("f" for fully-qualified function name), but I see you've got it all worked out!

The interface is exactly what I had in mind, with the bonus of specifying either dplyr::filter or dplyr::filter().

@hadley hadley merged commit c35d8ee into main Jan 17, 2023
@hadley hadley deleted the multi-conflict branch January 17, 2023 14:47
R/favor.R Show resolved Hide resolved
Comment on lines +18 to +21
#' conflicts_prefer(
#' dplyr::filter(),
#' dplyr::lag(),
#' )
Copy link
Member

Choose a reason for hiding this comment

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

It might be nice to show the other form in the examples, because i think that is what I'd personally prefer, but it is likely a little confusing to newish R users

Copy link
Member Author

Choose a reason for hiding this comment

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

It's also harder to type the form without parens because of autocomplete. But I'll add an example.

#' dplyr::lag(),
#' )
conflicts_prefer <- function(..., .quiet = FALSE) {
calls <- exprs(...)
Copy link
Member

Choose a reason for hiding this comment

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

exprs() vs enexprs()? not sure if it matters, we don't capture pure expressions often

is_ok <- vapply(calls, is_ns_call, logical(1))
if (any(!is_ok)) {
cli::cli_abort(
"All arguments must be in form {.code pkg::fun} or {.fn pkg::fun}"
Copy link
Member

Choose a reason for hiding this comment

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

Full stop

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops and I need to test it.

@@ -4,16 +4,13 @@
#' `conflict_prefer()` allows you to declare "winners" of conflicts.
#' You can either declare a specific pairing (i.e. `dplyr::filter()` beats
#' `base::filter()`), or an overall winner (i.e. `dplyr::filter()` beats
#' all comers).
#' all comers). As conflicted 1.2.0, in most case you should use
Copy link
Member

Choose a reason for hiding this comment

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

As _of_ conflicted 1.2.0, in most case_s_ you

#> Either pick the one you want with `::`:
#> • dplyr::filter
#> • stats::filter
#> Or declare a preference with `conflict_prefer()`:
Copy link
Member

Choose a reason for hiding this comment

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

I think you want to mention conflicts_prefer() here

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.

consider accepting input like "dplyr::filter"?
3 participants