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 wfits multi-line HISTORY field #489

Merged
merged 2 commits into from
Aug 22, 2024
Merged

Fix wfits multi-line HISTORY field #489

merged 2 commits into from
Aug 22, 2024

Conversation

d-lamb
Copy link
Member

@d-lamb d-lamb commented Aug 7, 2024

WIP! This just adds one passing and one failing test illustrating the problem in #488.

@d-lamb d-lamb added the pr:wip label Aug 7, 2024
@d-lamb d-lamb linked an issue Aug 7, 2024 that may be closed by this pull request
@coveralls
Copy link

coveralls commented Aug 7, 2024

Coverage Status

coverage: 32.616%. remained the same
when pulling 74e0566 on wfits-multiline-history
into 1426340 on master.

@mohawk2 mohawk2 marked this pull request as draft August 7, 2024 18:58
@mohawk2
Copy link
Member

mohawk2 commented Aug 7, 2024

Hi, thank you for this! I've taken the liberty of "convert[ing it] to draft" (under the Reviewers section, top right - I had to look up how). When it's not a draft anymore, the "Ready for review" button is in the "Checks" box at the bottom.

For the test, could you please make files in a temporary directory, that gets auto-deleted? Then you don't need to manually unlink. See Libtmp/t/diskcache.t for prior art.

@mohawk2
Copy link
Member

mohawk2 commented Aug 7, 2024

Also, it may be helpful for you if you rebase this to current master - this branch is currently 5 behind that.

This adds one passing and one (currently) failing test with
multi-line HISTORY strings in the header.
@mohawk2 mohawk2 force-pushed the wfits-multiline-history branch 2 times, most recently from 4c525ed to 1d983cf Compare August 22, 2024 18:04
@mohawk2 mohawk2 marked this pull request as ready for review August 22, 2024 18:05
@mohawk2 mohawk2 merged commit 5ab5a9f into master Aug 22, 2024
137 checks passed
@mohawk2 mohawk2 deleted the wfits-multiline-history branch August 22, 2024 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wfits doesn't preserve line order for multi-line HISTORY strings
3 participants