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

Handle \r without a \n in cygwin #3320

Merged
merged 1 commit into from
Mar 19, 2019

Conversation

ChrisJefferson
Copy link
Contributor

This fixes the problem whereby if GAP on windows sees a \r, it will remove the next \n, no matter how far away it is.

This isn't tested, because I found it very hard to come up with a sensible test, as git (or other tools) can try to "clean up" line endings. Hopefully the PR is clean enough.

In practice, there turns out to be a whole bunch of things that could be "improved" around here, but I don't really want to rewrite lots of the file handling. This fixes a single, immediate, problem.

@coveralls
Copy link

coveralls commented Mar 1, 2019

Coverage Status

Coverage increased (+0.0003%) to 85.076% when pulling 7e1e177 on ChrisJefferson:newlines into 1213dab on gap-system:master.

@codecov
Copy link

codecov bot commented Mar 2, 2019

Codecov Report

Merging #3320 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3320      +/-   ##
==========================================
- Coverage   85.26%   85.25%   -0.01%     
==========================================
  Files         697      698       +1     
  Lines      344153   344158       +5     
==========================================
- Hits       293426   293416      -10     
- Misses      50727    50742      +15
Impacted Files Coverage Δ
lib/process.gi 41.5% <0%> (-2.5%) ⬇️
src/iostream.c 65.39% <0%> (-0.77%) ⬇️
src/gasman.c 88.75% <0%> (-0.44%) ⬇️
src/saveload.c 67.93% <0%> (-0.28%) ⬇️
src/gvars.c 86.42% <0%> (-0.14%) ⬇️
src/vecgf2.c 75.71% <0%> (-0.12%) ⬇️
src/blister.c 98.55% <0%> (-0.04%) ⬇️
lib/morpheus.gi 84% <0%> (-0.03%) ⬇️
src/stats.c 95.13% <0%> (-0.01%) ⬇️
src/permutat.h 83.87% <0%> (ø) ⬆️
... and 13 more

@olexandr-konovalov
Copy link
Member

I triggered a build of this PR on Jenkins here (St Andrews access only)

@fingolfin fingolfin added os: windows Issues and PRs that are (at least partially) specific to Windows topic: kernel release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Mar 8, 2019
@wilfwilson
Copy link
Member

@alex-konovalov What was the result of your Jenkins build? (I don't have access).

@olexandr-konovalov
Copy link
Member

@ChrisJefferson this has to be rebased to include #3326, otherwise Windows test terminates early.

@fingolfin fingolfin merged commit d91d7c7 into gap-system:master Mar 19, 2019
@fingolfin fingolfin added the kind: bug Issues describing general bugs, and PRs fixing them label Mar 19, 2019
@ChrisJefferson ChrisJefferson deleted the newlines branch May 8, 2019 16:53
@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 os: windows Issues and PRs that are (at least partially) specific to Windows release notes: added PRs introducing changes that have since been mentioned in the release notes topic: kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants