Skip to content

Commit

Permalink
simply let the handler() function use location info (loc) directly …
Browse files Browse the repository at this point in the history 
…without adding `at lines`
  • Loading branch information
@yihui
yihui committed Apr 9, 2024
1 parent ec1823a commit 1182e51
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 2 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Package: xfun
Type: Package
Title: Supporting Functions for Packages Maintained by 'Yihui Xie'
Version: 0.43.1
Version: 0.43.2
Authors@R: c(
person("Yihui", "Xie", role = c("aut", "cre", "cph"), email = "xie@yihui.name", comment = c(ORCID = "0000-0003-0645-5666")),
person("Wush", "Wu", role = "ctb"),
Expand Down
1 change: 0 additions & 1 deletion R/utils.R
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,6 @@ handle_error = function(
) {
withCallingHandlers(expr, error = function(e) {
loc = if (is.function(fun)) trimws(fun(label)) else ''
if (loc != '') loc = sprintf(' at lines %s', loc)
message(one_string(handler(e, loc)))
})
}

2 comments on commit 1182e51

@yihui
Copy link
Owner Author

@yihui yihui commented on 1182e51 Apr 10, 2024

Choose a reason for hiding this comment

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

@cderv Just FYI since this is relevant to quarto: in the last release of xfun, I accidentally added the phrase "at lines" to the error message for knitr. The consequence is that quarto will be unable to parse or adjust the error line numbers: https://github.com/quarto-dev/quarto-cli/blob/6f233b161d770a76db9b5b3a7755fd8f88641ff3/src/execute/rmd.ts#L148 Therefore the line numbers will be incorrect with xfun 0.43 and knitr 1.46. This commit of xfun has fixed the issue.

It took me quite a few hours to figure out why the line numbers were off. During debugging, I have realized for the first time that quarto would first write .qmd to .rmarkdown and then knit the .rmarkdown file: https://github.com/quarto-dev/quarto-cli/blob/6f233b161d770a76db9b5b3a7755fd8f88641ff3/src/resources/rmd/execute.R#L20-L29 This will append some extra \n around code chunks, so the line numbers of .rmarkdown are not the same as .qmd. I didn't try to understand why extra empty lines are needed, but it means users can't easily get the line numbers of .qmd via knitr:::current_lines(), since knitr only sees .rmarkdown but not .qmd. I discovered this problem when I was trying to record the execution time of code chunks and their line numbers in .qmd. Then I found the line numbers were off by some numbers.

Anyway, this is not a big problem. I can probably figure out the line number mapping. I just want you to be aware of the xfun issue, in case someone else report this problem. I think I'll make a new release of xfun in a couple of weeks.

@cderv
Copy link
Collaborator

@cderv cderv commented on 1182e51 Apr 11, 2024

Choose a reason for hiding this comment

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

Thanks for the update on this !

I don't think the empty lines are needed, so I can try fix this. Otherwise, I know we have some mapped string logic in Quarto already so that the correct lines number for source is ok when using tools like OJS cells.

So we may need to do the same logic for knitr.

I'll record this as an improvement to be made in quarto. Thanks !

Please sign in to comment.