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

Improve fixupStreams output merging logic #8477

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

loneguardian
Copy link

Description

This PR is a proposed bugfix for #8473

Minimal reproducible repo is at https://github.com/loneguardian/quarto-cli-issue-8473

This PR:

  • Improve the logic of fixupStreams() so that it only groups adjacent stream outputs, instead of all stream outputs.

Referring to the original intended fix for #4968:

  • I am unsure whether this PR will cause regression
  • I am guessing it is unlikely to cause regression.

Checklist

I have (if applicable):

  • filed a contributor agreement. [In progress]
  • referenced the GitHub issue this PR closes
  • updated the appropriate changelog [not sure which file]

- Fix fixupStreams altering order of output when stream outputs are interlaced with data output
- Fix indentation
@loneguardian
Copy link
Author

I need input on the changelog part, if it is needed at all.

@cscheid
Copy link
Collaborator

cscheid commented Jan 29, 2024

We definitely would need a changelog entry on news/changelog-1.5.md.

But before you go any further, I'd like to make sure that we want this change, and that the original behavior wasn't there for a specific reason. @jjallaire, @dragonstyle, do you remember? I wasn't around when our Jupyter code was first written.

@cscheid cscheid added this to the v1.6 milestone Jun 27, 2024
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.

2 participants