Merge pull request #141 from zotero/previewCluster

cormacrelf · web-flow · commit f181977d3dab · 2021-12-05T17:35:13.000+11:00
previewCitationCluster =&gt; previewCluster
diff --git a/crates/wasm/README.md b/crates/wasm/README.md
@@ -471,14 +471,15 @@ Sometimes, a user wants to see how a cluster will look while they are editing
 it, before confirming the change.
 
 ```javascript
-let cites = [ { id: "citekey", locator: "45" }, { ... } ];
+let cluster = { cites: [ { id: "citekey", locator: "45" }, { ... } ] };
 let positions = [ ... before, { note: 34 }, ... after ];
-let preview = driver.previewCitationCluster(cites, positions, "html").unwrap();
+let preview = driver.previewCluster(cluster, positions).unwrap();
+let plainPreview = driver.previewCluster(cluster, positions, "plain").unwrap();
 ```
 
-The format argument is like the format passed to `Driver.new`: one of `"html"`,
-`"rtf"` or `"plain"`. The driver will use that instead of its normal output
-format.
+The cluster argument is just a cluster, without an `id` field, since it's
+ephemeral. The lack of `id` field is reflected in the `positions` argument as
+well.
 
 The positions array is exactly like a call to `setClusterOrder`, except exactly 
 one of the positions omits the id field. This could either:
@@ -492,6 +493,10 @@ mean you would never see "ibid" in a preview.** So for maximum utility,
 assemble the positions array as you would a call to `setClusterOrder` with 
 exactly the operation you're previewing applied.
 
+The format argument is optional, and works like the format passed to
+`Driver.new`: one of `"html"`, `"rtf"` or `"plain"`. The driver will use that
+instead of its normal output format.
+
 
 ### `AuthorOnly`, `SuppressAuthor` & `Composite`
 
@@ -652,9 +657,9 @@ Both of these methods will require throwing out almost all cached computation,
 so use sparingly.
 
 If you need to render a preview in a different format, there is an argument on
-`previewCitationCluster` for doing just that. It does not throw out all the
+`previewCluster` for doing just that. It does not throw out all the
 computation. `citeproc-rs`' disambiguation procedures do take formatting into
 account, so `<i>Title</i>` can be distinct from `<b>Title</b>` in HTML and RTF,
 but not if the whole driver's output format is `"plain"`, since they both look
-identical in plain text. `previewCitationCluster` will simply translate the
-formatting into another format, without re-computing all the disambiguation.
+identical in plain text. `previewCluster` will simply translate the formatting
+into another format, without re-computing all the disambiguation.
diff --git a/crates/wasm/js-tests/node/index.test.ts b/crates/wasm/js-tests/node/index.test.ts
@@ -13,6 +13,16 @@ let boldStyle = mkNoteStyle(
         <text variable="URL" prefix=" " />
     `,
 );
+let authorTitleStyle = mkInTextStyle(
+    `
+    <group delimiter=", ">
+    <names variable="author" />
+    <text variable="title" />
+    </group>
+    `,
+    ``
+);
+
 
 describe("Driver", () => {
 
@@ -182,7 +192,7 @@ describe("batchedUpdates", () => {
 
 });
 
-describe("previewCitationCluster", () => {
+describe("previewCluster", () => {
 
     let ibidStyle = mkNoteStyle(
         `
@@ -198,7 +208,7 @@ describe("previewCitationCluster", () => {
         ``,
     );
 
-    function pccSetup(callback) {
+    function pccSetup(callback: (driver: Driver, ids: [string, string]) => void) {
         withDriver({ style: ibidStyle }, driver => {
             let one = "cluster-one";
             let two = "cluster-two";
@@ -213,8 +223,8 @@ describe("previewCitationCluster", () => {
     test("between two other clusters", () => {
         pccSetup((driver, [one, two]) => {
             // between the other two
-            let pcc = driver.previewCitationCluster(
-                [{ id: "r1" }],
+            let pcc = driver.previewCluster(
+                { cites: [{ id: "r1" }] },
                 [{ id: one }, {}, { id: two }],
                 "plain"
             ).unwrap();
@@ -225,15 +235,15 @@ describe("previewCitationCluster", () => {
     test("replacing a cluster", () => {
         pccSetup((driver, [one, two]) => {
             // replacing #1
-            var pcc = driver.previewCitationCluster(
-                [{ id: "r1" }],
+            var pcc = driver.previewCluster(
+                { cites: [{ id: "r1" }] },
                 [{}, { id: two }],
                 "plain"
             ).unwrap();
             expect(pcc).toEqual("ONE");
             // replacing #1, with note numbers isntead
-            pcc = driver.previewCitationCluster(
-                [{ id: "r1" }],
+            pcc = driver.previewCluster(
+                { cites: [{ id: "r1" }] },
                 [{ note: 1, }, { id: two, note: 5 }],
                 "plain"
             ).unwrap();
@@ -242,27 +252,51 @@ describe("previewCitationCluster", () => {
     })
 
     test("should error when supplying unsupported output format", () => {
-        pccSetup((driver, [one, two]) => {
-            let res = driver.previewCitationCluster([{ id: "r1" }], [{}], "plaintext");
+        pccSetup((driver) => {
+            let res = driver.previewCluster({ cites: [{ id: "r1" }] }, [{}], "plaintext");
             expect(() => res.unwrap()).toThrow("Unknown output format \"plaintext\"");
         })
-    })
+    });
+
+    test("should allow omitting the format argument", () => {
+        pccSetup((driver, [_, two]) => {
+            let res = driver.previewCluster(
+                { cites: [{ id: "r1" }] },
+                [{ note: 1 }, { id: two, note: 5 }]
+            ).unwrap();
+            expect(res).toEqual("ONE");
+        })
+    });
+
+    test("should handle cluster modes", () => {
+        pccSetup((driver, [_, two]) => {
+            driver.setStyle(authorTitleStyle).unwrap();
+            driver.insertReference(
+                { title: "ONE", id: "r1", type: "book", author: [{ family: "Smith" }] }
+            ).unwrap();
+            let res = driver.previewCluster(
+                { cites: [{ id: "r1" }], mode: "Composite", infix: ", whose book" },
+                [{ note: 1 }, { id: two, note: 5 }]
+            ).unwrap();
+            expect(res).toEqual("Smith, whose book ONE");
+        })
+    });
+
+    test("should also work via deprecated previewCitationCluster(cites: Cite[], ...)", () => {
+        pccSetup((driver, [_, two]) => {
+            let res = driver.previewCitationCluster(
+                [{ id: "r1" }],
+                [{ note: 1 }, { id: two, note: 5 }]
+            ).unwrap();
+            expect(res).toEqual("ONE");
+        })
+    });
 
 });
 
 describe("AuthorOnly and friends", () => {
-    let style = mkInTextStyle(
-        `
-        <group delimiter=", ">
-            <names variable="author" />
-            <text variable="title" />
-        </group>
-    `,
-        ``
-    );
-
-    function withSupp(callback) {
-        withDriver({ style }, driver => {
+    function withSupp(callback: (driver: Driver, ids: [string, string]) => void) {
+        withDriver({ style: authorTitleStyle }, driver => {
             let one = "cluster-one";
             let two = "cluster-two";
             oneOneOne(driver, { title: "ONE", id: "r1", author: [{ family: "Smith" }] }, "cluster-one");
diff --git a/crates/wasm/src/lib.rs b/crates/wasm/src/lib.rs
@@ -20,7 +20,6 @@ extern crate log;
 use js_sys::Promise;
 use std::cell::RefCell;
 use std::rc::Rc;
-use std::str::FromStr;
 use std::sync::Arc;
 use wasm_bindgen::prelude::*;
 use wasm_bindgen_futures::{future_to_promise, JsFuture};
@@ -296,37 +295,64 @@ impl Driver {
         })
     }
 
+    /// @deprecated Use `previewCluster` instead
+    #[wasm_bindgen(js_name = "previewCitationCluster")]
+    pub fn preview_citation_cluster(
+        &self,
+        cites: Box<[JsValue]>,
+        positions: Box<[JsValue]>,
+        format: Option<String>,
+    ) -> StringResult {
+        typescript_serde_result(|| {
+            let cites = utils::read_js_array_2(cites)?;
+            self.preview_cluster_inner(PreviewCluster::new(cites, None), positions, format)
+        })
+    }
+
     /// Previews a formatted citation cluster, in a particular position.
     ///
-    /// - `cites`: The cites to go in the cluster
+    /// - `cluster`: A cluster, without an `id` field. You'll want this to contain some cites.
     /// - `positions`: An array of `ClusterPosition`s as in set_cluster_order, but with a single
     ///   cluster's id set to zero. The cluster with id=0 is the position to preview the cite. It
     ///   can replace another cluster, or be inserted before/after/between existing clusters, in
     ///   any location you can think of.
+    /// - `format`: an optional argument, an output format as a string, that is used only for this
+    ///   preview.
     ///
-    #[wasm_bindgen(js_name = "previewCitationCluster")]
-    pub fn preview_citation_cluster(
+    #[wasm_bindgen(js_name = "previewCluster")]
+    pub fn preview_cluster(
         &self,
-        cites: Box<[JsValue]>,
+        preview_cluster: TPreviewCluster,
         positions: Box<[JsValue]>,
-        format: &str,
+        format: Option<String>,
     ) -> StringResult {
         typescript_serde_result(|| {
-            let cites: Vec<Cite<Markup>> = utils::read_js_array_2(cites)?;
-            let positions: Vec<string_id::ClusterPosition> = utils::read_js_array_2(positions)?;
-            let mut eng = self.engine.borrow_mut();
-            let preview = eng.preview_citation_cluster(
-                PreviewCluster::new(cites, None),
-                PreviewPosition::MarkWithZeroStr(&positions),
-                Some(
-                    SupportedFormat::from_str(format)
-                        .map_err(|()| DriverError::UnknownOutputFormat(format.to_owned()))?,
-                ),
-            );
-            Ok(preview?)
+            let preview_cluster: PreviewCluster = preview_cluster.into_serde()?;
+            self.preview_cluster_inner(preview_cluster, positions, format)
         })
     }
 
+    fn preview_cluster_inner(
+        &self,
+        preview_cluster: PreviewCluster,
+        positions: Box<[JsValue]>,
+        format: Option<String>,
+    ) -> Result<Arc<SmartString>, DriverError> {
+        let positions: Vec<string_id::ClusterPosition> = utils::read_js_array_2(positions)?;
+        let mut eng = self.engine.borrow_mut();
+        let preview = eng.preview_citation_cluster(
+            preview_cluster,
+            PreviewPosition::MarkWithZeroStr(&positions),
+            format
+                .map(|frmt| {
+                    frmt.parse::<SupportedFormat>()
+                        .map_err(|()| DriverError::UnknownOutputFormat(frmt))
+                })
+                .transpose()?,
+        );
+        Ok(preview?)
+    }
+
     #[wasm_bindgen(js_name = "makeBibliography")]
     pub fn make_bibliography(&self) -> BibEntriesResult {
         typescript_serde_result(|| {
@@ -508,7 +534,7 @@ interface InitOptions {
     localeOverride?: string,
 
     /** Disables sorting in the bibliography; items appear in cited order. */
-    bibliographyNoSort?: bool,
+    bibliographyNoSort?: boolean,
 }
 
 /** This interface lets citeproc retrieve locales or modules asynchronously,
@@ -559,7 +585,7 @@ export type Cluster = {
     cites: Cite[];
 } & ClusterMode;
 
-export type PreviewCluster {
+export type PreviewCluster = {
     cites: Cite[];
 } & ClusterMode;
 
@@ -706,11 +732,11 @@ interface WasmResult<T> {
     is_ok(): boolean;
     is_err(): boolean;
     /** If this is an error, returns the default value. */
-    unwrap_or(default: T): T;
+    unwrap_or(defaultValue: T): T;
     /** If this is Ok, returns f(ok_val), else returns Err unmodified. */
     map<R>(f: (t: T) => R): WasmResult<T>;
     /** If this is Ok, returns f(ok_val), else returns the default value. */
-    map_or<R>(default: R, f: (t: T) => R): R;
+    map_or<R>(defaultValue: R, f: (t: T) => R): R;
 }
 "#;
 
@@ -753,11 +779,11 @@ interface IndependentMeta {
     /** A list of languages for which a locale override was specified.
       * Does not include the language-less final override. */
     localeOverrides: string[],
-    hasBibliography: bool,
+    hasBibliography: boolean,
 }
 interface StyleMeta {
     info: StyleInfo,
-    features: { [feature: string]: bool },
+    features: { [feature: string]: boolean },
     defaultLocale: string,
     /** May be absent on a dependent style */
     class?: "in-text" | "note",
