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

Extend to multiple searches? #7

Closed
robitalec opened this issue Feb 20, 2020 · 4 comments · Fixed by #9
Closed

Extend to multiple searches? #7

robitalec opened this issue Feb 20, 2020 · 4 comments · Fixed by #9

Comments

@robitalec
Copy link

robitalec commented Feb 20, 2020

Right now, it seems errorist just searches for the first warning or message.
This came up for me in trying out the package, I was actually more interested in the search results for the second warning..


Reproducible example:

library(errorist)

warn2 <- function() {
  warning('warning one')
  warning('warning two')
}

warn2()
@coatless
Copy link
Collaborator

@robitalec yes, only searching the first error and warning for the latest function call is accurate.

Having said this, there are two parts here that are drastically different: error and warning.

Error-wise, it isn't possible to have two errors back-to-back within code execution since the code is exited once a single error is detected. We're able to then use the error handler to trigger an appropriate dispatch.

Warning dispatch is another story. errorist was setup to search the first warning returned as multiple identical warnings may occur in a single function call. Though, this might have been slightly too conservative. Potentially, we could trigger a search on distinct warnings by parsing names(list.warning).

Self-note:
https://github.com/r-assist/errorist/blob/8a41cb8be894470cc03aa6b0b1a953018ab3be1b/R/handlers.R#L88-L96

@robitalec
Copy link
Author

Thanks @coatless!
Silly mistake - you never get two errors. Wrote this up a bit quickly clearly, edited above..

Yes - I think we definitely wouldn't want repeated searches from the same warning repeated, but might be useful to search for unique ones, like you say. Thanks!

coatless added a commit that referenced this issue Feb 22, 2020
* Enable searches for multiple warnings. (Close #7)

* Roll new version
@coatless
Copy link
Collaborator

@robitalec searching multiple warnings should now work. Please try it out in the development version:

remotes::install_github('r-assist/errorist')

@robitalec
Copy link
Author

Perfect! That worked @coatless.
Thank you.

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 a pull request may close this issue.

2 participants