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 Display for functions with large literals, and add a bunch of tests #3207

Merged
merged 2 commits into from
Jan 22, 2019

Conversation

fingolfin
Copy link
Member

I was writing a bunch of tests when I noticed this odd printing behavior. So this PR first adds a bunch of tests; and then a commit which fixes Display for functions which large literals (and which also adjusts the tests added in the first commit accordingly).

@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them topic: tests issues or PRs related to tests topic: library release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Jan 21, 2019
@fingolfin fingolfin requested a review from stevelinton January 21, 2019 09:23
Test large float literals, ensure ReadEvalError in function calls
gets reliably triggered, and also test bounds checks for ranges
in gac compiled code.
This fixes a problem where printing functions with long literals would
wrap those literals badly. As an example, before this fix, we got:

    gap> SizeScreen([60,100]);;
    gap> Display(f->12345678901234567890123456789012345678901234567890123456789012345678901234567890);
    function ( f )
        return
         12345678901234567890123456789012345678901234567890123\
    \
    456789012345678901234567890;
    end

With this fix, we get:

    gap> SizeScreen([60,100]);;
    gap> Display(f->12345678901234567890123456789012345678901234567890123456789012345678901234567890);
    function ( f )
        return
         12345678901234567890123456789012345678901234567890123\
    456789012345678901234567890;
    end

Fixes gap-system#3205
@codecov
Copy link

codecov bot commented Jan 22, 2019

Codecov Report

Merging #3207 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3207      +/-   ##
==========================================
+ Coverage   84.76%   84.77%   +<.01%     
==========================================
  Files         687      687              
  Lines      335865   335867       +2     
==========================================
+ Hits       284696   284722      +26     
+ Misses      51169    51145      -24
Impacted Files Coverage Δ
lib/function.gi 83.54% <100%> (+0.64%) ⬆️
src/permutat.h 77.41% <0%> (-6.46%) ⬇️
src/iostream.c 62.35% <0%> (-1.15%) ⬇️
src/weakptr.c 97.8% <0%> (-0.44%) ⬇️
lib/streams.gi 73.85% <0%> (-0.22%) ⬇️
src/stats.c 95.1% <0%> (-0.2%) ⬇️
src/exprs.c 97.6% <0%> (+0.18%) ⬆️
src/intrprtr.c 99.17% <0%> (+0.3%) ⬆️
src/scanner.c 92.11% <0%> (+0.46%) ⬆️
src/range.c 98.19% <0%> (+0.51%) ⬆️
... and 2 more

@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 84.648% when pulling eb1829c on fingolfin:mh/display-funcs into f56a68c on gap-system:master.

@fingolfin fingolfin merged commit 81e2bb3 into gap-system:master Jan 22, 2019
@fingolfin fingolfin deleted the mh/display-funcs branch January 22, 2019 22:52
@DominikBernhardt DominikBernhardt added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Aug 22, 2019
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.11.0 milestone Feb 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Issues describing general bugs, and PRs fixing them release notes: added PRs introducing changes that have since been mentioned in the release notes topic: library topic: tests issues or PRs related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants