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

Solve year-suffix / no-date interaction #137

Merged
merged 19 commits into from
Dec 1, 2021
Merged

Solve year-suffix / no-date interaction #137

merged 19 commits into from
Dec 1, 2021

Conversation

cormacrelf
Copy link
Collaborator

@cormacrelf cormacrelf commented Dec 1, 2021

As discussed in andras-simonyi/citeproc-el#70 (comment)

This makes the "no date" term behave like a variable for group suppression. It also makes the GroupVars code work less by accident and more on purpose.

@github-actions
Copy link

github-actions bot commented Dec 1, 2021

Test results (failure; full results): 4 regressions, 2 new passing tests

regression: csl_test_suite::variables_TitleShortOnShortTitleNoTitleCondition.txt

current (now failing):
 My Long Title 1 has title-short
 My Long Title 2 does not have title-short
-[CSL STYLE ERROR: reference with no printed form.]
+does not have title-short
        
    stack backtrace:
       0: rust_begin_unwind
                 at /rustc/2f662b14033c4369b0a7b5c2656592ef08edf434/library/std/src/panicking.rs:517:5
       1: std::panicking::begin_panic_fmt
                 at /rustc/2f662b14033c4369b0a7b5c2656592ef08edf434/library/std/src/panicking.rs:460:5
       2: suite::csl_test_suite
                 at ./tests/suite.rs:112:13
       3: suite::__TEST_TRAMPOLINE_csl_test_suite
                 at ./tests/suite.rs:99:1
       4: datatest::runner::render_files_test::{{closure}}
                 at /home/runner/.cargo/git/checkouts/datatest-8f1d8ca908a37906/96d0154/src/runner.rs:166:83
       5: core::ops::function::FnOnce::call_once{{vtable.shim}}
                 at /rustc/2f662b14033c4369b0a7b5c2656592ef08edf434/library/core/src/ops/function.rs:227:5
       6: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
                 at /rustc/2f662b14033c4369b0a7b5c2656592ef08edf434/library/alloc/src/boxed.rs:1636:9

regression: csl_test_suite::bugreports_ChicagoAuthorDateLooping.txt

current (now failing):
-(Manstein 1982; [CSL STYLE ERROR: reference with no printed form.]; [CSL STYLE ERROR: reference with no printed form.])
+(Manstein 1982; Anon.; Anon.)
        
    stack backtrace:
       0: rust_begin_unwind
                 at /rustc/2f662b14033c4369b0a7b5c2656592ef08edf434/library/std/src/panicking.rs:517:5
       1: std::panicking::begin_panic_fmt
                 at /rustc/2f662b14033c4369b0a7b5c2656592ef08edf434/library/std/src/panicking.rs:460:5
       2: suite::csl_test_suite
                 at ./tests/suite.rs:112:13
       3: suite::__TEST_TRAMPOLINE_csl_test_suite
                 at ./tests/suite.rs:99:1
       4: datatest::runner::render_files_test::{{closure}}
                 at /home/runner/.cargo/git/checkouts/datatest-8f1d8ca908a37906/96d0154/src/runner.rs:166:83
       5: core::ops::function::FnOnce::call_once{{vtable.shim}}
                 at /rustc/2f662b14033c4369b0a7b5c2656592ef08edf434/library/core/src/ops/function.rs:227:5
       6: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
                 at /rustc/2f662b14033c4369b0a7b5c2656592ef08edf434/library/alloc/src/boxed.rs:1636:9

regression: csl_test_suite::bugreports_DoubleEncodedAngleBraces.txt

current (now failing):
    ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Snapshot Differences ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
    Snapshot file: crates/citeproc/tests/snapshots/suite__bugreports_DoubleEncodedAngleBraces.txt.snap
    Snapshot: bugreports_DoubleEncodedAngleBraces.txt
    Source: crates/citeproc/tests/suite.rs:110
    -old snapshot
    +new results
    ────────────────────────────────────────────────────────────────────────────────
    res
    ────────────┬───────────────────────────────────────────────────────────────────
        1       │-<span style="font-variant:small-caps;">Center for History and New Media</span>, « Zotero Quick Start Guide », s.d., [En ligne]. &lt;http://zotero.org/support/quick_start_guide&gt;.
              1 │+<span style="font-variant:small-caps;">Center for History and New Media</span>, « Zotero Quick Start Guide », [En ligne]. &lt;http://zotero.org/support/quick_start_guide&gt;.
    ────────────┴───────────────────────────────────────────────────────────────────
    To update snapshots run `cargo insta review`
    thread 'csl_test_suite::bugreports_DoubleEncodedAngleBraces.txt' panicked at 'snapshot assertion for 'bugreports_DoubleEncodedAngleBraces.txt' failed in line 110', /home/runner/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/insta-1.1.0/src/runtime.rs:1059:9
    stack backtrace:
       0: rust_begin_unwind
                 at /rustc/2f662b14033c4369b0a7b5c2656592ef08edf434/library/std/src/panicking.rs:517:5
       1: std::panicking::begin_panic_fmt
                 at /rustc/2f662b14033c4369b0a7b5c2656592ef08edf434/library/std/src/panicking.rs:460:5
       2: insta::runtime::assert_snapshot
                 at /home/runner/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/insta-1.1.0/src/runtime.rs:1059:9
       3: suite::csl_test_suite
                 at ./tests/suite.rs:110:13
       4: suite::__TEST_TRAMPOLINE_csl_test_suite
                 at ./tests/suite.rs:99:1
       5: datatest::runner::render_files_test::{{closure}}
                 at /home/runner/.cargo/git/checkouts/datatest-8f1d8ca908a37906/96d0154/src/runner.rs:166:83
       6: core::ops::function::FnOnce::call_once{{vtable.shim}}
                 at /rustc/2f662b14033c4369b0a7b5c2656592ef08edf434/library/core/src/ops/function.rs:227:5
       7: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
                 at /rustc/2f662b14033c4369b0a7b5c2656592ef08edf434/library/alloc/src/boxed.rs:1636:9

regression: csl_test_suite::bugreports_SingleQuoteXml.txt

current (now failing):
-[[CSL STYLE ERROR: reference with no printed form.] ; O A. "hello" <i>Book Title</i>]
+[Cite with a composer ; O A. "hello" <i>Book Title</i>]
        
    stack backtrace:
       0: rust_begin_unwind
                 at /rustc/2f662b14033c4369b0a7b5c2656592ef08edf434/library/std/src/panicking.rs:517:5
       1: std::panicking::begin_panic_fmt
                 at /rustc/2f662b14033c4369b0a7b5c2656592ef08edf434/library/std/src/panicking.rs:460:5
       2: suite::csl_test_suite
                 at ./tests/suite.rs:112:13
       3: suite::__TEST_TRAMPOLINE_csl_test_suite
                 at ./tests/suite.rs:99:1
       4: datatest::runner::render_files_test::{{closure}}
                 at /home/runner/.cargo/git/checkouts/datatest-8f1d8ca908a37906/96d0154/src/runner.rs:166:83
       5: core::ops::function::FnOnce::call_once{{vtable.shim}}
                 at /rustc/2f662b14033c4369b0a7b5c2656592ef08edf434/library/core/src/ops/function.rs:227:5
       6: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
                 at /rustc/2f662b14033c4369b0a7b5c2656592ef08edf434/library/alloc/src/boxed.rs:1636:9

added passing tests:

  • humans::group_YearSuffixExplicit.yml
  • humans::group_TextDateYearSuffix.yml
    test result: 4 regressions, 2 new passing tests, 0 new ignores, 0 outputs changed, out of 938 intersecting tests
    master: 914 passed; 0 failed; 24 ignored
    no-date: 912 passed; 4 failed; 24 ignored

You can see how the issue might apply to the GroupVars
promote_plain()'s

    match self { Plain | Important => Important, _ => self }
@github-actions
Copy link

github-actions bot commented Dec 1, 2021

Test results (success; full results): 2 new passing tests

added passing tests:

  • humans::group_YearSuffixExplicit.yml
  • humans::group_TextDateYearSuffix.yml
    test result: 0 regressions, 2 new passing tests, 0 new ignores, 0 outputs changed, out of 938 intersecting tests
    master: 914 passed; 0 failed; 24 ignored
    no-date: 916 passed; 0 failed; 24 ignored

@github-actions
Copy link

github-actions bot commented Dec 1, 2021

Test results (success; full results): 2 new passing tests

added passing tests:

  • humans::group_TextDateYearSuffix.yml
  • humans::group_YearSuffixExplicit.yml
    test result: 0 regressions, 2 new passing tests, 0 new ignores, 0 outputs changed, out of 938 intersecting tests
    master: 914 passed; 0 failed; 24 ignored
    no-date: 916 passed; 0 failed; 24 ignored

Comment on lines 914 to 919
if added_suffix {
tree.recompute_group_vars();
return;
}

// Then attempt to do it for the ones that are embedded in date output
Copy link
Collaborator Author

@cormacrelf cormacrelf Dec 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicit year suffixes weren't recomputing the tree's groupvars, only implicit ones were.

Comment on lines +190 to +199
let gv = if let csl::TextTermSelector::Simple(csl::SimpleTermSelector::Misc(
csl::MiscTerm::NoDate,
_,
)) = term_selector
{
GroupVars::Important // Make this Important (same for element.rs) to have no-date act as a variable
} else {
GroupVars::Plain
};
(RefIR::Edge(content), gv)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where we make the no date term behave like a variable

@@ -120,7 +120,7 @@ impl Disambiguation<Markup> for Element {
let gv = GroupVars::rendered_if(edge.is_some());
return (RefIR::Edge(edge), gv);
} else {
return (RefIR::Edge(None), GroupVars::Plain);
return (RefIR::Edge(None), GroupVars::Missing);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where we remove the special case for YearSuffix

@cormacrelf cormacrelf merged commit c280dea into master Dec 1, 2021
@cormacrelf cormacrelf deleted the no-date branch December 1, 2021 07:30
@cormacrelf cormacrelf added the A-core Area: affects all builds of citeproc-rs label Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: affects all builds of citeproc-rs
Development

Successfully merging this pull request may close these issues.

1 participant