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

Show error and warning tracebacks #2219

Open
3 tasks done
hadley opened this issue Feb 6, 2023 · 4 comments
Open
3 tasks done

Show error and warning tracebacks #2219

hadley opened this issue Feb 6, 2023 · 4 comments
Labels
feature Feature requests

Comments

@hadley
Copy link
Contributor

hadley commented Feb 6, 2023

Currently, rlang includes some custom hooks to ensure that we give high-quality error tracebacks inside Rmds (implemented in r-lib/rlang#1215). We implemented this in rlang for expediency, but I wonder if it's time to move this to knitr?

Similarly, we have been considering ways to make it easier to find where warnings occurred by supplying full backtraces. We have all the machinery to make this work, we just need to add a similar sew() method for warnings. Is this something that might also live in knitr?


By filing an issue to this repo, I promise that

  • I have fully read the issue guide at https://yihui.org/issue/.
  • I have provided the necessary information about my issue.
    • If I'm asking a question, I have already asked it on Stack Overflow or RStudio Community, waited for at least 24 hours, and included a link to my question there.
    • If I'm filing a bug report, I have included a minimal, self-contained, and reproducible example, and have also included xfun::session_info('knitr'). I have upgraded all my packages to their latest versions (e.g., R, RStudio, and R packages), and also tried the development version: remotes::install_github('yihui/knitr').
    • If I have posted the same issue elsewhere, I have also mentioned it in this issue.
  • I have learned the Github Markdown syntax, and formatted my issue correctly.

I understand that my issue may be closed if I don't fulfill my promises.

@cderv cderv added the feature Feature requests label Feb 6, 2023
@cderv
Copy link
Collaborator

cderv commented Feb 6, 2023

I think that fits well into knitr, especially if we can make the addition conditional. It seems to me that when rlang_error and other class are used, it means rlang is available, and it does not require to have a new dependency for now.

IMO that definitely worth it to make all this available in our tool without the workaround in other package. Better traceback from knitting step will benefit users.

@lionel-
Copy link
Contributor

lionel- commented Feb 9, 2023

@hadley IIUC you are planning to entrace all errors and all warnings?

If it's only the rlang conditions (thrown via cli::cli_abort() and cli::cli_warn()) then I'd prefer to keep the methods in rlang to make updates easier and reduce the interdependency. If this includes base conditions, then knitr needs to own the methods and call into rlang. But we probably still want to abstract that behind an API so that the bulk of the implementation lives in rlang.

@hadley
Copy link
Contributor Author

hadley commented Feb 9, 2023

@lionel- all errors and warnings

@lionel-
Copy link
Contributor

lionel- commented Feb 10, 2023

I wrote some more thoughts in r-lib/rlang#1551 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature requests
Projects
None yet
Development

No branches or pull requests

3 participants