Improve error messages about conflicted parameters

[yutannihilation]

This is suggested in #2761 (comment).

The current error messages are like:

Specify either position or nudge_x/nudge_y

@clauswilke's suggestion is:

position and width/height cannot both be specified. Please specify only one

I feel the latter follows the tidyverse style of error messages; it consists of "problem statement" + "hint".

[hadley]

I think you can use "must" here: "You must specify one of position and width/height".

Also for nudging error, I think it would be worthwhile to extract the repeated code into a function like check_nudge_specification()

[yutannihilation]

Thanks, I agree with you on both.

[yutannihilation]

For check_nudge_specification(), I feel a bit afraid that I have to scratch my head to figure out how to pass missingness. Let me think a bit more...

[hadley]

Oh yeah, I always forget about that. In that case, don't worry about a function.

f <- function(x, y = NULL) {
  g(x, y)
}

g <- function(x, y) {
  c(x = missing(x), y = missing(y))
}

f()
#>     x     y 
#>  TRUE FALSE
f(1, 2)
#>     x     y 
#> FALSE FALSE

Created on 2018-08-24 by the reprex
package (v0.2.0).

[yutannihilation]

Sorry, I can't come up with any better way; in this case, we cannot use waiver() for position, since default of position must be a valid name of an existing Position class.

I hope duplication of the error message will be resolved by the new error system which will be introduced i rlang 0.3.0.

So, I want to give up a function for now.

[clauswilke]

This looks fine to me. @hadley, do you have any more concerns? Otherwise we should include it in the 3.0.1 release also.

[hadley]

@batpigandme can you suggest any improvements to the phrasing here?

[clauswilke]

My preference would be to be a bit more specific about the exclusive nature of the constraint. E.g., instead of writing "you must specify one", I would write "you must specify at most one" or "you must specify exactly one" or "you must specify only one".

Or, something along the lines of "You cannot specify both ... at the same time. You must specify only one."

[clauswilke]

@batpigandme Just a friendly reminder: Could you look over the proposed error messages and either approve or suggest alternative wording? Thanks!

[batpigandme]

Thanks @clauswilke, I'd totally missed this. How about:

#> You must specify either `var 1` or `var 2`

[batpigandme]

Proposed mashup of old/new:
"You must specify either position or width/height"?

[clauswilke]

Would it be terrible to add "but not both"? The reason why I started this conversation in the first place was that I felt the error message didn't sufficiently clearly express that the error is that both options are specified at once.

[batpigandme]

I think the either...or construct provides that.

[clauswilke]

Ok.

@yutannihilation Would you mind updating the error text to what @batpigandme proposed, and then I'll merge?

[clauswilke]

I think this one is still missing a period.

[yutannihilation]

Aw, sorry...

[yutannihilation]

Thanks for suggestion, @batpigandme. I updated the text.

[clauswilke]

Thanks!

[lock]

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/