Fix Bug, Put Page into its own Variable category (#14)

src/lib.rs

@@ -48,7 +48,8 @@ use std::num::{NonZeroI16, NonZeroUsize};
 use quick_xml::de::{Deserializer, SliceReader};
 use serde::{Deserialize, Serialize};
 use taxonomy::{
-    DateVariable, Kind, Locator, NameVariable, NumberVariable, OtherTerm, Term, Variable,
+    DateVariable, Kind, Locator, NameVariable, NumberOrPageVariable, NumberVariable,
+    OtherTerm, Term, Variable,
 };
 
 use self::util::*;
@@ -742,6 +743,11 @@ fn minimal(
     x: &str,
     y: &str,
 ) -> Result<(), fmt::Error> {
+    if y.len() > x.len() {
+        // y is no abbrev. write it
+        return write!(buf, "{y}");
+    }
+
     let mut xs = String::new();
     let mut ys = String::new();
     for (c, d) in x.chars().zip(y.chars()).skip_while(|(c, d)| c == d) {
@@ -2635,7 +2641,7 @@ pub struct Substitute {
 pub struct Label {
     /// The variable for which to print the label.
     #[serde(rename = "@variable")]
-    pub variable: NumberVariable,
+    pub variable: NumberOrPageVariable,
     /// The form of the label.
     #[serde(flatten)]
     pub label: VariablelessLabel,
@@ -3729,6 +3735,7 @@ mod test {
         assert_eq!("13792–803", run(c15, "13792", "13803"));
         assert_eq!("1496–1504", run(c15, "1496", "1504"));
         assert_eq!("2787–2816", run(c15, "2787", "2816"));
+        assert_eq!("101–8", run(c15, "101", "108"));
 
         assert_eq!("3–10", run(c16, "3", "10"));
         assert_eq!("71–72", run(c16, "71", "72"));
@@ -3746,6 +3753,7 @@ mod test {
         assert_eq!("11564–68", run(c16, "11564", "11568"));
         assert_eq!("13792–803", run(c16, "13792", "13803"));
         assert_eq!("12991–3001", run(c16, "12991", "13001"));
+        assert_eq!("12991–123001", run(c16, "12991", "123001"));
 
         assert_eq!("42–45", run(exp, "42", "45"));
         assert_eq!("321–328", run(exp, "321", "328"));
@@ -3761,6 +3769,19 @@ mod test {
         assert_eq!("2787–816", run(mi2, "2787", "2816"));
     }
 
+    /// Tests the bug from PR typst/hayagriva#155
+    #[test]
+    fn test_bug_hayagriva_115() {
+        fn run(format: PageRangeFormat, start: &str, end: &str) -> String {
+            let mut buf = String::new();
+            format.format(&mut buf, start, end, None).unwrap();
+            buf
+        }
+        let c16 = PageRangeFormat::Chicago16;
+
+        assert_eq!("12991–123001", run(c16, "12991", "123001"));
+    }
+
     #[test]
     fn page_range_prefix() {
         fn run(format: PageRangeFormat, start: &str, end: &str) -> String {

src/taxonomy.rs

@@ -12,7 +12,13 @@ use serde::{de, Deserialize, Deserializer, Serialize};
 pub enum Variable {
     /// The set of variables with no other attributes.
     Standard(StandardVariable),
-    /// Variables that can be formatted as numbers.
+    /// The page variable. Can be formatted as a page range.
+    ///
+    /// CSL does not distinguish between page ranges and other number variables.
+    /// See the [`NumberOrPageVariable`] enum for a CSL-like view on the
+    /// variants.
+    Page(PageVariable),
+    /// Variables that can be formatted as numbers and are not page ranges.
     Number(NumberVariable),
     /// Variables that can be formatted as dates.
     Date(DateVariable),
@@ -39,6 +45,7 @@ impl fmt::Display for Variable {
             Self::Number(v) => v.fmt(f),
             Self::Date(v) => v.fmt(f),
             Self::Name(v) => v.fmt(f),
+            Self::Page(v) => v.fmt(f),
         }
     }
 }
@@ -250,6 +257,53 @@ impl fmt::Display for StandardVariable {
     }
 }
 
+#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash, Serialize, Deserialize)]
+/// Number variables as the CSL spec knows them, though we separate between
+/// number variables and page ranges.
+#[serde(untagged)]
+pub enum NumberOrPageVariable {
+    /// The page variable.
+    Page(PageVariable),
+    /// Variables formattable as numbers.
+    Number(NumberVariable),
+}
+
+impl From<NumberOrPageVariable> for Term {
+    fn from(value: NumberOrPageVariable) -> Self {
+        match value {
+            NumberOrPageVariable::Page(p) => p.into(),
+            NumberOrPageVariable::Number(n) => n.into(),
+        }
+    }
+}
+
+/// Variable that can be formatted as page numbers
+#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash, Serialize, Deserialize)]
+pub enum PageVariable {
+    #[serde(rename = "page")]
+    /// Range of pages the item (e.g. a journal article) covers in a container
+    /// (e.g. a journal issue).
+    Page,
+}
+
+impl From<PageVariable> for Term {
+    fn from(_: PageVariable) -> Self {
+        Self::PageVariable
+    }
+}
+
+impl From<PageVariable> for Variable {
+    fn from(value: PageVariable) -> Self {
+        Self::Page(value)
+    }
+}
+
+impl fmt::Display for PageVariable {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "page")
+    }
+}
+
 /// Variables that can be formatted as numbers.
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Deserialize, Serialize)]
 #[serde(rename_all = "kebab-case")]
@@ -286,9 +340,6 @@ pub enum NumberVariable {
     NumberOfPages,
     /// Total number of volumes, used when citing multi-volume books and such.
     NumberOfVolumes,
-    /// Range of pages the item (e.g. a journal article) covers in a container
-    /// (e.g. a journal issue).
-    Page,
     /// First page of the range of pages the item (e.g. a journal article)
     /// covers in a container (e.g. a journal issue).
     PageFirst,
@@ -326,7 +377,6 @@ impl fmt::Display for NumberVariable {
             Self::Number => write!(f, "number"),
             Self::NumberOfPages => write!(f, "number-of-pages"),
             Self::NumberOfVolumes => write!(f, "number-of-volumes"),
-            Self::Page => write!(f, "page"),
             Self::PageFirst => write!(f, "page-first"),
             Self::PartNumber => write!(f, "part-number"),
             Self::PrintingNumber => write!(f, "printing-number"),
@@ -526,6 +576,8 @@ pub enum Term {
     NameVariable(NameVariable),
     /// Variables that can be formatted as numbers.
     NumberVariable(NumberVariable),
+    /// The Page variable.
+    PageVariable,
     /// A locator.
     Locator(Locator),
     /// Terms that are not defined via types, name or number variables.
@@ -595,16 +647,16 @@ impl Term {
                 (
                     Self::Locator(Locator::Issue),
                     Self::NumberVariable(NumberVariable::Issue),
-                ) | (
-                    Self::Locator(Locator::Page),
-                    Self::NumberVariable(NumberVariable::Page),
-                ) | (
-                    Self::Locator(Locator::Section),
-                    Self::NumberVariable(NumberVariable::Section),
-                ) | (
-                    Self::Locator(Locator::Volume),
-                    Self::NumberVariable(NumberVariable::Volume),
-                ) | (Self::Locator(Locator::Book), Self::Kind(Kind::Book))
+                ) | (Self::Locator(Locator::Page), Self::PageVariable,)
+                    | (
+                        Self::Locator(Locator::Section),
+                        Self::NumberVariable(NumberVariable::Section),
+                    )
+                    | (
+                        Self::Locator(Locator::Volume),
+                        Self::NumberVariable(NumberVariable::Volume),
+                    )
+                    | (Self::Locator(Locator::Book), Self::Kind(Kind::Book))
                     | (Self::Locator(Locator::Chapter), Self::Kind(Kind::Chapter))
                     | (Self::Locator(Locator::Figure), Self::Kind(Kind::Figure))
             )