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 CRLF/LF problems for Windows dev #270

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

Conversation

YDX-2147483647
Copy link

@YDX-2147483647 YDX-2147483647 commented Jan 23, 2025

This PR fixes the following CRLF/LF problems for Windows dev. (only affects test documents)

Test suites

On Windows, git clone may automatically convert LF to CRLF. (See git config core.autocrlf.)
For tests expecting multiple lines, the RESULT section will get corrupted, leading to tons of failures.

For example,
https://github.com/citation-style-language/test-suite/blob/d2bd47ff09db6e2995aeed2be0b096b12c0f8f70/processor-tests/humans/variables_TitleShortOnShortTitleNoTitleGroup.txt#L5-L9

>>==== RESULT ====>>
Check this out: My Short Title is from title-short
Check this out:
Check this out:
<<==== RESULT ====<<
truncated output of git clone … && cargo test --features csl-json on Windows
running 8 tests
…
test test_local_files ... FAILED
…
test test_parse_tests ... FAILED

…

Test target/haya-cache\test-suite\processor-tests/humans/variables_TitleShortOnShortTitleNoTitleGroup.txt failed
Expected:
Check this out: My Short Title is from title-short
Check this out:
Check this out:

Got:
Check this out: My Short Title is from title-short
Check this out:
Check this out:

Total: 793, skipped: 317, passed: 259
54.41% passed
❌ Test bugreports_BadCitationUpdate should pass but failed
❌ Test bugreports_SectionAndLocator should pass but failed
❌ Test bugreports_YearSuffixLingers should pass but failed
❌ Test collapse_CitationNumberRangesMixed should pass but failed
❌ Test collapse_CitationNumberRangesMixed2 should pass but failed
❌ Test collapse_CitationNumberRangesMixed3 should pass but failed
❌ Test condition_EmptyDate should pass but failed
❌ Test condition_VariableAll should pass but failed
❌ Test condition_VariableAny should pass but failed
❌ Test condition_VariableNone should pass but failed
❌ Test date_LocalizedDateFormats-af-ZA should pass but failed
❌ Test date_LocalizedDateFormats-ar-AR should pass but failed
❌ Test date_LocalizedDateFormats-bg-BG should pass but failed
❌ Test date_LocalizedDateFormats-ca-AD should pass but failed
❌ Test date_LocalizedDateFormats-cs-CZ should pass but failed
❌ Test date_LocalizedDateFormats-da-DK should pass but failed
❌ Test date_LocalizedDateFormats-de-AT should pass but failed
❌ Test date_LocalizedDateFormats-de-CH should pass but failed
❌ Test date_LocalizedDateFormats-de-DE should pass but failed
❌ Test date_LocalizedDateFormats-el-GR should pass but failed
❌ Test date_LocalizedDateFormats-en-US should pass but failed
❌ Test date_LocalizedDateFormats-es-ES should pass but failed
❌ Test date_LocalizedDateFormats-et-EE should pass but failed
❌ Test date_LocalizedDateFormats-fr-FR should pass but failed
❌ Test date_LocalizedDateFormats-he-IL should pass but failed
❌ Test date_LocalizedDateFormats-hu-HU should pass but failed
…
❌ Test variables_TitleShortOnShortTitleNoTitleGroup should pass but failed
thread 'test_parse_tests' panicked at tests\citeproc.rs:341:9:
Some tests that should pass failed or were not found


failures:
    test_local_files
    test_parse_tests

test result: FAILED. 4 passed; 2 failed; 2 ignored; 0 measured; 0 filtered out; finished in 12.52s

error: test failed, to rerun pass `--test citeproc`

Reason: test_file uses LF for output, but the RESULT section uses CRLF for expected:

output.push('\n');

To fix it, I replace all CRLF in the RESULT section with LF.

CBOR and CSL/XML

Git does not natively recognize *.cbor, and guesses their types (text/binary) based on their content.
If git misclassifies a CBOR as binary, it will corrupt it by altering end-of-line.

For example, the following <summary> containing newlines will be literally embedded in CBOR.
https://github.com/citation-style-language/styles/blob/2e90b3afc1aa6fc90c46d674798936cd438af1fa/spie-journals.csl#L16-L19

    <summary>Designed for SPIE journals, based on information at http://spie.org/x85036.xml and sample .docx manuscript given there (http://spie.org/Documents/Publications/Journals/journal%20Word%20template.doc). The latter probably originated from the style for e-journals (which was previously available at http://spie.org/x3658.xml), and thus slightly contradicts former requirements. The differences are:
    1) style on the web page requires using et al., when there are more than three authors - we follow this rule.
    2) style in the sample manuscript asks to add DOI if available, while the style on the web page says nothing about it - we do include DOI (this should not harm in all cases).
    Also previous versions of this style were based on more detailed instructions for SPIE e-journals (including larger number of document types). Those details are present in the current version as well, but no more supported by a documented style.</summary>

To fix it, I explicitly mark *.cbor as binary in .gitattributes, and freeze end-of-line of CSL/XML.

Relates-to: tafia/quick-xml#454

@YDX-2147483647
Copy link
Author

CI failed because of outdated archives. I just bumped. I can reset it if not desired.
#202

@YDX-2147483647 YDX-2147483647 marked this pull request as draft January 23, 2025 05:28
@YDX-2147483647

This comment was marked as outdated.

@YDX-2147483647 YDX-2147483647 force-pushed the crlf branch 2 times, most recently from bd60346 to 4e39184 Compare January 23, 2025 06:10
@YDX-2147483647 YDX-2147483647 changed the title Fix CRLF/LF of expected outputs in citeproc tests Fix CRLF/LF problems for Windows dev Jan 23, 2025
@YDX-2147483647 YDX-2147483647 marked this pull request as ready for review January 24, 2025 05:16
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.

1 participant