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

fix: problem with multibyte characters in julia engine #9740

Merged
merged 6 commits into from
Jun 4, 2024

Conversation

MHellmund
Copy link
Contributor

@MHellmund MHellmund commented May 22, 2024

Description

This concerns the new julia engine and solves issue PumasAI/QuartoNotebookRunner.jl#130

Checklist

I have (if applicable):

  • filed a contributor agreement.
  • referenced the GitHub issue this PR closes
  • updated the appropriate changelog

@MHellmund MHellmund changed the title Juliabug2 fix: problem with multibyte characters in julia engine May 22, 2024
Copy link
Collaborator

@cscheid cscheid left a comment

Choose a reason for hiding this comment

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

I'd like to ensure consistent formatting, but the content looks good. Thank you!

src/execute/julia.ts Outdated Show resolved Hide resolved
src/execute/julia.ts Show resolved Hide resolved
@cscheid
Copy link
Collaborator

cscheid commented May 22, 2024

@MHellmund We also need a contributor agreement from you.

@cscheid
Copy link
Collaborator

cscheid commented May 22, 2024

This looks good! I'll await confirmation on the contributor agreement and we'll merge. Thank you for the good fix!

@MHellmund
Copy link
Contributor Author

I just sent the contributor agreement to jj@rstudio.com

@MHellmund
Copy link
Contributor Author

Please don't merge yet, I think I found a problem.

@MHellmund MHellmund marked this pull request as draft May 30, 2024 15:16
@MHellmund MHellmund marked this pull request as draft May 30, 2024 15:16
@cscheid
Copy link
Collaborator

cscheid commented May 30, 2024

Got it. Thanks for following up, and good thing I was out on vacation most of last week! :)

@MHellmund MHellmund marked this pull request as ready for review May 31, 2024 10:55
@MHellmund
Copy link
Contributor Author

It's amazing how many tests a wrong code can pass before you run into heisenbugs with a larger document.
I think I've got it now.

@cscheid
Copy link
Collaborator

cscheid commented Jun 4, 2024

Thanks, @MHellmund! If you have tests you can share publicly, we'd love to run them in our suite going forward.

@cscheid cscheid merged commit b237f60 into quarto-dev:main Jun 4, 2024
49 checks passed
@MHellmund
Copy link
Contributor Author

There is the test enclosed to the original bug report PumasAI/QuartoNotebookRunner.jl#130 .
This Quarto book is produced with the new engine. Source. But I don't have a more formal test.

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.

3 participants