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 handling of line break characters in a string in pretty printing #1636

Merged
merged 3 commits into from
Mar 14, 2024

Conversation

lgoettgens
Copy link
Collaborator

Resolves oscar-system/Oscar.jl#3515.

The field io.printed should keep track of the width already printed in the current line (indentation is not tracked here).
In two places, a newline char was printed, but the field was not reset to 0.

There is unfortunately no way to test this in a doctest, as all of this linebreaking business is skipped for doctests.

In local testing, this both resolves the MWE in oscar-system/Oscar.jl#3515 (comment), and the printing issues with MonomialBasis (at least those that I could observe locally).

cc @benlorenz @lkastner

@lgoettgens lgoettgens added the bugfix This change fixes an existing bug label Mar 14, 2024
@lgoettgens lgoettgens requested review from fingolfin and thofma March 14, 2024 13:54
Copy link

codecov bot commented Mar 14, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 87.00%. Comparing base (7e756b8) to head (74fca7e).

Files Patch % Lines
src/PrettyPrinting.jl 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1636      +/-   ##
==========================================
+ Coverage   86.94%   87.00%   +0.05%     
==========================================
  Files         114      114              
  Lines       29700    29707       +7     
==========================================
+ Hits        25823    25846      +23     
+ Misses       3877     3861      -16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lgoettgens lgoettgens changed the title Fix literal line breaks in pretty printing Fix line break characters in a string in pretty printing Mar 14, 2024
@lgoettgens lgoettgens changed the title Fix line break characters in a string in pretty printing Fix handling of line break characters in a string in pretty printing Mar 14, 2024
@benlorenz
Copy link
Collaborator

Thanks! Looks correct now (also when passing through our fake repl pipes).

@lgoettgens lgoettgens merged commit 70133e0 into Nemocas:master Mar 14, 2024
29 of 30 checks passed
@lgoettgens lgoettgens deleted the lg/fix-print-linebreaks branch March 14, 2024 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Printing of MonomialBasis is broken
2 participants