From 0ec0a0c7acd85b97baffb8bccc049f7604d4e069 Mon Sep 17 00:00:00 2001 From: Greg Hellings Date: Thu, 26 Jun 2025 02:19:26 -0500 Subject: [PATCH] Builds, runs, and tests succeed* *Tests only succeed if you run them in a single-threaded instance. With cargo this means invoking them as `cargo run -- --test-threads=1`. There is an instance of global state in the Sword engine with the VersificationMgr class that can easily cause a race condition. Since VersificationMgr is intended to be a global singleton, when it is first being instantiated the pointer is null. Sometimes the tests get into that method at the same time and overwrite one another with the instance of the manager, thus causing some of the VerseKeys that get instantiated to be invalid. TODO: On the Rust side of the code, create an instance of the VersificationMgr before allowing any VerseKey objects to be created. Probably also necessary to run it before allowing SWMgr to be instantiated, as well, just to be safe. --- flake.nix | 1 + src/cxx/mod.rs | 179 +++++++++++------ src/cxx/module.cc | 10 +- src/cxx/module.h | 2 +- src/cxx/versekey.cc | 84 +++++--- src/cxx/versekey.h | 39 ++-- src/module.rs | 23 ++- src/versekey.rs | 464 ++++++++++++++------------------------------ 8 files changed, 373 insertions(+), 429 deletions(-) diff --git a/flake.nix b/flake.nix index 4762097..cae5770 100644 --- a/flake.nix +++ b/flake.nix @@ -46,6 +46,7 @@ cargo clippy libgcc + lldb rustc.llvm rustc rustfmt diff --git a/src/cxx/mod.rs b/src/cxx/mod.rs index e23ca56..f4bae05 100644 --- a/src/cxx/mod.rs +++ b/src/cxx/mod.rs @@ -19,7 +19,7 @@ pub mod ffi { fn decrement(self: Pin<&mut Module>); fn get_name(self: &Module) -> String; fn get_type(self: &Module) -> String; - fn get_verse_key(self: &Module) -> *mut VerseKey; + fn get_verse_key(self: &Module) -> *mut SwordVerseKey; fn increment(self: Pin<&mut Module>); fn is_skip_consecutive_links(self: &Module) -> bool; fn render_text(self: Pin<&mut Module>) -> String; @@ -35,19 +35,28 @@ pub mod ffi { fn new_versification() -> UniquePtr; type VerseKey; - // Factory functions for VerseKey - fn new_verse_key() -> UniquePtr; - fn new_verse_key_from_text(text: &String) -> UniquePtr; - unsafe fn delete_verse_key(key: *mut VerseKey); + type SwordVerseKey; - // From Key + // Factory functions for VerseKey - return raw pointers for manual management + fn new_verse_key() -> *mut VerseKey; + fn new_verse_key_from_text(text: String) -> *mut VerseKey; + + // Wrapper function for module VerseKeys + unsafe fn get_verse_key_wrapper(sword_key: *mut SwordVerseKey) -> *mut VerseKey; + + // Clone function + fn clone_verse_key(source: &VerseKey) -> *mut VerseKey; + + // Delete function for cleanup + unsafe fn delete_verse_key(verse_key: *mut VerseKey); + + // VerseKey methods fn get_error(self: &VerseKey) -> u8; fn get_locale(self: &VerseKey) -> String; fn get_range_text(self: &VerseKey) -> String; fn get_short_text(self: &VerseKey) -> String; fn get_text(self: &VerseKey) -> String; fn normalize(self: Pin<&mut VerseKey>, autocheck: bool); - // From VerseKey fn get_book_name(self: &VerseKey) -> String; fn get_book_abbrev(self: &VerseKey) -> String; fn get_osis_book_name(self: &VerseKey) -> String; @@ -62,17 +71,12 @@ pub mod ffi { fn get_chapter_max(self: &VerseKey) -> u8; fn get_verse(self: &VerseKey) -> u8; fn get_verse_max(self: &VerseKey) -> u8; - - // From VerseKey fn is_autonormalize(self: &VerseKey) -> bool; fn is_intros(self: &VerseKey) -> bool; - // From Key + fn is_valid(self: &VerseKey) -> bool; fn pop_error(self: Pin<&mut VerseKey>) -> u8; - - // From Key fn set_locale(self: Pin<&mut VerseKey>, locale: &String); fn set_text(self: Pin<&mut VerseKey>, text: &String); - // From VerseKey fn set_autonormalize(self: Pin<&mut VerseKey>, autoNormalize: bool); fn set_book_name(self: Pin<&mut VerseKey>, bookName: &String); fn set_intros(self: Pin<&mut VerseKey>, intros: bool); @@ -80,9 +84,6 @@ pub mod ffi { fn set_book(self: Pin<&mut VerseKey>, book: u8); fn set_chapter(self: Pin<&mut VerseKey>, chapter: u8); fn set_verse(self: Pin<&mut VerseKey>, verse: u8); - - // Explicit cleanup - fn cleanup(self: Pin<&mut VerseKey>); } } @@ -128,8 +129,8 @@ mod tests { assert!(mgr.get_modules().len() == 1); assert!(mgr.get_modules().contains(&String::from("KJVdummy"))); - let mut module = mgr.pin_mut().get_module(&String::from("KJVdummy")); - assert!(module.pin_mut().get_name() == "KJVdummy"); + let module = mgr.pin_mut().get_module(&String::from("KJVdummy")); + assert!(module.get_name() == "KJVdummy"); } #[test] @@ -147,42 +148,103 @@ mod tests { #[test] fn can_create_verse_key() { - let verse_key = ffi::new_verse_key(); - assert!(!verse_key.is_null()); + let verse_key_ptr = ffi::new_verse_key(); + assert!(!verse_key_ptr.is_null()); - // Test that we can call methods without errors - let _testament = verse_key.get_testament(); - let _book = verse_key.get_book(); - let _chapter = verse_key.get_chapter(); - let _verse = verse_key.get_verse(); - let _book_name = verse_key.get_book_name(); - let _text = verse_key.get_text(); + unsafe { + let verse_key = &*verse_key_ptr; + assert!(verse_key.is_valid()); + + // Test that we can call methods without errors + let _testament = verse_key.get_testament(); + let _book = verse_key.get_book(); + let _chapter = verse_key.get_chapter(); + let _verse = verse_key.get_verse(); + let _book_name = verse_key.get_book_name(); + let _text = verse_key.get_text(); + + // Cleanup + ffi::delete_verse_key(verse_key_ptr); + } } #[test] fn can_create_verse_key_from_text() { - let verse_key = ffi::new_verse_key_from_text(&"Genesis 1:1".to_string()); - assert!(!verse_key.is_null()); + let verse_key_ptr = ffi::new_verse_key_from_text("Genesis 1:1".to_string()); + assert!(!verse_key_ptr.is_null()); - // Test that we can get values without errors - let _chapter = verse_key.get_chapter(); - let _verse = verse_key.get_verse(); - let _text = verse_key.get_text(); + unsafe { + let verse_key = &*verse_key_ptr; + assert!(verse_key.is_valid()); + + // Test that we can get values without errors + let chapter = verse_key.get_chapter(); + assert_eq!(chapter, 1); + let verse = verse_key.get_verse(); + assert_eq!(verse, 1); + let text = verse_key.get_text(); + assert_eq!(text, "Genesis 1:1"); + + // Cleanup + ffi::delete_verse_key(verse_key_ptr); + } } #[test] fn verse_key_can_be_modified() { - let mut verse_key = ffi::new_verse_key(); + let verse_key_ptr = ffi::new_verse_key(); + assert!(!verse_key_ptr.is_null()); - // Test that we can call setter methods without panicking - verse_key.pin_mut().set_chapter(5); - verse_key.pin_mut().set_verse(10); - verse_key.pin_mut().set_text(&"Genesis 1:1".to_string()); + unsafe { + let verse_key = &*verse_key_ptr; + assert!(verse_key.is_valid()); - // Test that we can call getter methods after setting - let _chapter = verse_key.get_chapter(); - let _verse = verse_key.get_verse(); - let _text = verse_key.get_text(); + // Test that we can call setter methods without panicking + std::pin::Pin::new_unchecked(&mut *(verse_key_ptr as *mut ffi::VerseKey)) + .set_chapter(5); + assert_eq!(verse_key.get_chapter(), 5); + std::pin::Pin::new_unchecked(&mut *(verse_key_ptr as *mut ffi::VerseKey)).set_verse(10); + assert_eq!(verse_key.get_verse(), 10); + std::pin::Pin::new_unchecked(&mut *(verse_key_ptr as *mut ffi::VerseKey)) + .set_text(&"Genesis 1:2".to_string()); + + // Test that we can call getter methods after setting + let chapter = verse_key.get_chapter(); + assert_eq!(chapter, 1); + let verse = verse_key.get_verse(); + assert_eq!(verse, 2); + let text = verse_key.get_text(); + assert_eq!(text, "Genesis 1:2"); + + // Cleanup + ffi::delete_verse_key(verse_key_ptr); + } + } + + #[test] + fn can_clone_verse_key() { + let verse_key_ptr = ffi::new_verse_key_from_text("John 3:16".to_string()); + assert!(!verse_key_ptr.is_null()); + + unsafe { + let verse_key = &*verse_key_ptr; + assert!(verse_key.is_valid()); + + let cloned_ptr = ffi::clone_verse_key(verse_key); + assert!(!cloned_ptr.is_null()); + + let cloned = &*cloned_ptr; + assert!(cloned.is_valid()); + + // Both should have the same text + assert_eq!(verse_key.get_text(), cloned.get_text()); + assert_eq!(verse_key.get_chapter(), cloned.get_chapter()); + assert_eq!(verse_key.get_verse(), cloned.get_verse()); + + // Cleanup both + ffi::delete_verse_key(verse_key_ptr); + ffi::delete_verse_key(cloned_ptr); + } } #[test] @@ -195,10 +257,8 @@ mod tests { // Test that we can call methods without panicking verse_key.set_chapter(3); verse_key.set_verse(16); - verse_key.set_text("John 3:16"); // Test getters - let _book_name = verse_key.get_book_name(); let _text = verse_key.get_text(); let _chapter = verse_key.get_chapter(); let _verse = verse_key.get_verse(); @@ -206,10 +266,14 @@ mod tests { // Test creation from text let verse_key2 = VerseKey::from_text("Romans 8:28"); let _text2 = verse_key2.get_text(); + + // Test cloning + let verse_key3 = verse_key2.clone(); + assert_eq!(verse_key2.get_text(), verse_key3.get_text()); } #[test] - fn borrowed_verse_key_from_module() { + fn module_verse_key_integration() { use std::fs::{create_dir, File}; use std::io::Write; use tempfile; @@ -232,24 +296,17 @@ mod tests { // Create manager and get module let mut mgr = ffi::new_mgr_with_path(&dir.path().to_str().unwrap().to_string()); - let mut module = mgr.pin_mut().get_module(&String::from("KJVdummy")); + let module = mgr.pin_mut().get_module(&String::from("KJVdummy")); - // Test that we can get a VerseKey from the module - let verse_key_ptr = module.pin_mut().get_verse_key(); - if !verse_key_ptr.is_null() { - // Test that we can create a BorrowedVerseKey and use it - let borrowed_verse_key = - unsafe { crate::versekey::BorrowedVerseKey::from_ptr(verse_key_ptr) }; + // Test that we can get a VerseKey from the module using high-level API + if let Some(verse_key) = crate::module::Module::new(module).get_verse_key() { + // Test that we can call methods without panicking + let _book_name = verse_key.get_book_name(); + let _text = verse_key.get_text(); - if let Some(mut borrowed_vk) = borrowed_verse_key { - // Test that we can call methods without panicking - let _book_name = borrowed_vk.get_book_name(); - let _text = borrowed_vk.get_text(); - borrowed_vk.set_chapter(1); - borrowed_vk.set_verse(1); - let _chapter = borrowed_vk.get_chapter(); - let _verse = borrowed_vk.get_verse(); - } + // Test that we can clone it + let cloned = verse_key.clone(); + assert_eq!(verse_key.get_text(), cloned.get_text()); } } } diff --git a/src/cxx/module.cc b/src/cxx/module.cc index 15fc4b0..1cf14e9 100644 --- a/src/cxx/module.cc +++ b/src/cxx/module.cc @@ -15,14 +15,8 @@ namespace sword_rs { return rust::String(this->module->getType()); } - VerseKey* Module::get_verse_key() const { - sword::VerseKey* key = dynamic_cast(this->module->getKey()); - if (key) { - // Create a borrowed VerseKey on the heap - caller must delete but not the inner ptr - VerseKey* result = new VerseKey{key, false}; - return result; - } - return nullptr; + SwordVerseKey* Module::get_verse_key() const { + return dynamic_cast(this->module->getKey()); } void Module::increment() { diff --git a/src/cxx/module.h b/src/cxx/module.h index a7e3100..e0eda35 100644 --- a/src/cxx/module.h +++ b/src/cxx/module.h @@ -21,7 +21,7 @@ namespace sword_rs { rust::String get_type() const; - VerseKey* get_verse_key() const; + SwordVerseKey* get_verse_key() const; void increment(); diff --git a/src/cxx/versekey.cc b/src/cxx/versekey.cc index 293e2b2..5e30982 100644 --- a/src/cxx/versekey.cc +++ b/src/cxx/versekey.cc @@ -1,9 +1,8 @@ #include "versekey.h" -#include #include namespace sword_rs { - // Key methods + // VerseKey methods - all include null pointer checks for safety uint8_t VerseKey::get_error() const { return ptr ? ptr->getError() : 0; } @@ -46,7 +45,6 @@ namespace sword_rs { } } - // VerseKey specific methods rust::String VerseKey::get_book_name() const { return ptr ? rust::String(ptr->getBookName()) : rust::String(""); } @@ -153,35 +151,75 @@ namespace sword_rs { } } - // Explicit cleanup method - void VerseKey::cleanup() { - if (owned && ptr) { - delete ptr; - ptr = nullptr; - owned = false; + bool VerseKey::is_valid() const { + return ptr != nullptr; + } + + // Factory function for self-managing VerseKey + VerseKey* new_verse_key() { + try { + auto sword_key = new sword::VerseKey(); + auto wrapper = new VerseKey(); + wrapper->ptr = sword_key; + wrapper->managed = false; // self-managing + return wrapper; + } catch (...) { + return nullptr; } } - // Factory functions - std::unique_ptr new_verse_key() { - auto verse_key = std::unique_ptr(new VerseKey{new sword::VerseKey(), true}); - return verse_key; + // Factory function for self-managing VerseKey from text + VerseKey* new_verse_key_from_text(const rust::String text) { + try { + auto sword_key = new sword::VerseKey(std::string(text).c_str()); + auto wrapper = new VerseKey(); + wrapper->ptr = sword_key; + wrapper->managed = false; // self-managing + return wrapper; + } catch (...) { + return nullptr; + } } - std::unique_ptr new_verse_key_from_text(const rust::String& text) { - auto verse_key = std::unique_ptr(new VerseKey{new sword::VerseKey(std::string(text).c_str()), true}); - return verse_key; + // Creates a managed VerseKey wrapper from module's sword::VerseKey + VerseKey* get_verse_key_wrapper(SwordVerseKey* sword_key) { + if (!sword_key) { + return nullptr; + } + + try { + auto wrapper = new VerseKey(); + wrapper->ptr = sword_key; + wrapper->managed = true; // managed by external code + return wrapper; + } catch (...) { + return nullptr; + } } - VerseKey verse_key_from_borrowed(sword::VerseKey* key) { - VerseKey verse_key{key, false}; - return verse_key; + // Clone function - always returns self-managing copy using copy constructor + VerseKey* clone_verse_key(const VerseKey& source) { + if (!source.ptr) { + return nullptr; + } + + try { + // Create a new sword::VerseKey using copy constructor + auto sword_key = new sword::VerseKey(*source.ptr); + auto wrapper = new VerseKey(); + wrapper->ptr = sword_key; + wrapper->managed = false; // clones are always self-managing + return wrapper; + } catch (...) { + return nullptr; + } } - void delete_verse_key(VerseKey* key) { - if (key) { - key->cleanup(); - delete key; + // Delete function for VerseKey wrappers + void delete_verse_key(VerseKey* verse_key) { + if (verse_key) { + verse_key->cleanup(); + delete verse_key; } } } diff --git a/src/cxx/versekey.h b/src/cxx/versekey.h index c2c4d24..9ddc868 100644 --- a/src/cxx/versekey.h +++ b/src/cxx/versekey.h @@ -1,18 +1,26 @@ #pragma once #include -#include #include #include "rust/cxx.h" namespace sword_rs { - // Trivial wrapper that just holds a pointer and ownership flag - // No destructor - cleanup is handled explicitly + // Type alias for cxx bridge compatibility + using SwordVerseKey = sword::VerseKey; + // Simple struct that holds a pointer to sword::VerseKey and tracks ownership struct VerseKey { sword::VerseKey* ptr; - bool owned; + bool managed; // true if managed by SWMgr instance, false if self-managing - // Key methods + // Manual cleanup method (no destructor to keep struct trivial) + void cleanup() { + if (!managed && ptr) { + delete ptr; + ptr = nullptr; + } + } + + // All VerseKey methods uint8_t get_error() const; rust::String get_locale() const; rust::String get_range_text() const; @@ -23,7 +31,6 @@ namespace sword_rs { void set_locale(const rust::String &locale); void set_text(const rust::String &text); - // VerseKey methods rust::String get_book_name() const; rust::String get_book_abbrev() const; rust::String get_osis_book_name() const; @@ -50,15 +57,19 @@ namespace sword_rs { void set_chapter(uint8_t chapter); void set_verse(uint8_t verse); - // Explicit cleanup method - void cleanup(); + bool is_valid() const; }; - // Factory functions for creating VerseKeys - std::unique_ptr new_verse_key(); - std::unique_ptr new_verse_key_from_text(const rust::String& text); - VerseKey verse_key_from_borrowed(sword::VerseKey* key); + // Factory functions - return raw pointers for manual management + VerseKey* new_verse_key(); + VerseKey* new_verse_key_from_text(const rust::String text); - // Memory management for raw VerseKey pointers - void delete_verse_key(VerseKey* key); + // Creates a managed VerseKey from a module's sword::VerseKey pointer + VerseKey* get_verse_key_wrapper(SwordVerseKey* sword_key); + + // Clone function - always returns self-managing copy + VerseKey* clone_verse_key(const VerseKey& source); + + // Delete function for VerseKey wrappers + void delete_verse_key(VerseKey* verse_key); } diff --git a/src/module.rs b/src/module.rs index a4aac61..3da9397 100644 --- a/src/module.rs +++ b/src/module.rs @@ -84,14 +84,23 @@ impl Module { self.module.pin_mut().strip_text() } - pub fn get_verse_key(&mut self) -> Option { + pub fn get_verse_key(&mut self) -> Option { match self.get_type() { - Type::BiblicalTexts => unsafe { - crate::versekey::BorrowedVerseKey::from_ptr(self.module.pin_mut().get_verse_key()) - }, - Type::Commentaries => unsafe { - crate::versekey::BorrowedVerseKey::from_ptr(self.module.pin_mut().get_verse_key()) - }, + Type::BiblicalTexts | Type::Commentaries => { + let sword_ptr = self.module.get_verse_key(); + if sword_ptr.is_null() { + None + } else { + unsafe { + let wrapper_ptr = ffi::get_verse_key_wrapper(sword_ptr); + if wrapper_ptr.is_null() { + None + } else { + Some(crate::versekey::VerseKey::from_raw_ptr(wrapper_ptr)) + } + } + } + } _ => None, } } diff --git a/src/versekey.rs b/src/versekey.rs index 72a3b61..3e0134f 100644 --- a/src/versekey.rs +++ b/src/versekey.rs @@ -1,183 +1,181 @@ use crate::cxx::ffi; +use std::fmt; -/// Safe wrapper for owned VerseKey instances +/// Safe Rust wrapper for VerseKey with proper memory management pub struct VerseKey { - inner: cxx::UniquePtr, + ptr: *mut ffi::VerseKey, } impl VerseKey { /// Create a new VerseKey pub fn new() -> Self { - Self { - inner: ffi::new_verse_key(), + let ptr = ffi::new_verse_key(); + if ptr.is_null() { + panic!("Failed to create VerseKey"); } + Self { ptr } } /// Create a new VerseKey from text pub fn from_text(text: &str) -> Self { - Self { - inner: ffi::new_verse_key_from_text(&text.to_string()), + let ptr = ffi::new_verse_key_from_text(text.to_string()); + if ptr.is_null() { + panic!("Failed to create VerseKey from text"); } + Self { ptr } } - /// Get the book name - pub fn get_book_name(&self) -> String { - self.inner.get_book_name() + /// Create from a raw pointer (for internal use) + pub(crate) fn from_raw_ptr(ptr: *mut ffi::VerseKey) -> Self { + if ptr.is_null() { + panic!("Cannot create VerseKey from null pointer"); + } + Self { ptr } } - /// Get the book abbreviation - pub fn get_book_abbrev(&self) -> String { - self.inner.get_book_abbrev() + /// Get a reference to the underlying VerseKey + fn as_ref(&self) -> &ffi::VerseKey { + unsafe { &*self.ptr } } - /// Get the OSIS book name - pub fn get_osis_book_name(&self) -> String { - self.inner.get_osis_book_name() + /// Get a pinned mutable reference to the underlying VerseKey + fn as_pin_mut(&mut self) -> std::pin::Pin<&mut ffi::VerseKey> { + unsafe { std::pin::Pin::new_unchecked(&mut *self.ptr) } } - /// Get the OSIS reference - pub fn get_osis_ref(&self) -> String { - self.inner.get_osis_ref() + /// Check if the VerseKey is valid + pub fn is_valid(&self) -> bool { + !self.ptr.is_null() && self.as_ref().is_valid() } - /// Get the OSIS reference range text - pub fn get_osis_ref_range_text(&self) -> String { - self.inner.get_osis_ref_range_text() - } - - /// Get the short range text - pub fn get_short_range_text(&self) -> String { - self.inner.get_short_range_text() - } - - /// Get the testament number - pub fn get_testament(&self) -> u8 { - self.inner.get_testament() - } - - /// Get the maximum testament number - pub fn get_testament_max(&self) -> u8 { - self.inner.get_testament_max() - } - - /// Get the book number - pub fn get_book(&self) -> u8 { - self.inner.get_book() - } - - /// Get the maximum book number - pub fn get_book_max(&self) -> u8 { - self.inner.get_book_max() - } - - /// Get the chapter number - pub fn get_chapter(&self) -> u8 { - self.inner.get_chapter() - } - - /// Get the maximum chapter number - pub fn get_chapter_max(&self) -> u8 { - self.inner.get_chapter_max() - } - - /// Get the verse number - pub fn get_verse(&self) -> u8 { - self.inner.get_verse() - } - - /// Get the maximum verse number - pub fn get_verse_max(&self) -> u8 { - self.inner.get_verse_max() - } - - /// Check if auto-normalize is enabled - pub fn is_autonormalize(&self) -> bool { - self.inner.is_autonormalize() - } - - /// Check if intros are enabled - pub fn is_intros(&self) -> bool { - self.inner.is_intros() - } - - /// Get the error code + // Read-only methods pub fn get_error(&self) -> u8 { - self.inner.get_error() + self.as_ref().get_error() } - /// Get the locale pub fn get_locale(&self) -> String { - self.inner.get_locale() + self.as_ref().get_locale() } - /// Get the range text pub fn get_range_text(&self) -> String { - self.inner.get_range_text() + self.as_ref().get_range_text() } - /// Get the short text pub fn get_short_text(&self) -> String { - self.inner.get_short_text() + self.as_ref().get_short_text() } - /// Get the text pub fn get_text(&self) -> String { - self.inner.get_text() + self.as_ref().get_text() } - /// Normalize the key + pub fn get_book_name(&self) -> String { + self.as_ref().get_book_name() + } + + pub fn get_book_abbrev(&self) -> String { + self.as_ref().get_book_abbrev() + } + + pub fn get_osis_book_name(&self) -> String { + self.as_ref().get_osis_book_name() + } + + pub fn get_osis_ref(&self) -> String { + self.as_ref().get_osis_ref() + } + + pub fn get_osis_ref_range_text(&self) -> String { + self.as_ref().get_osis_ref_range_text() + } + + pub fn get_short_range_text(&self) -> String { + self.as_ref().get_short_range_text() + } + + pub fn get_testament(&self) -> u8 { + self.as_ref().get_testament() + } + + pub fn get_testament_max(&self) -> u8 { + self.as_ref().get_testament_max() + } + + pub fn get_book(&self) -> u8 { + self.as_ref().get_book() + } + + pub fn get_book_max(&self) -> u8 { + self.as_ref().get_book_max() + } + + pub fn get_chapter(&self) -> u8 { + self.as_ref().get_chapter() + } + + pub fn get_chapter_max(&self) -> u8 { + self.as_ref().get_chapter_max() + } + + pub fn get_verse(&self) -> u8 { + self.as_ref().get_verse() + } + + pub fn get_verse_max(&self) -> u8 { + self.as_ref().get_verse_max() + } + + pub fn is_autonormalize(&self) -> bool { + self.as_ref().is_autonormalize() + } + + pub fn is_intros(&self) -> bool { + self.as_ref().is_intros() + } + + // Mutable methods pub fn normalize(&mut self, autocheck: bool) { - self.inner.pin_mut().normalize(autocheck) + self.as_pin_mut().normalize(autocheck) } - /// Pop an error pub fn pop_error(&mut self) -> u8 { - self.inner.pin_mut().pop_error() + self.as_pin_mut().pop_error() } - /// Set auto-normalize - pub fn set_autonormalize(&mut self, auto_normalize: bool) { - self.inner.pin_mut().set_autonormalize(auto_normalize) - } - - /// Set the book name - pub fn set_book_name(&mut self, book_name: &str) { - self.inner.pin_mut().set_book_name(&book_name.to_string()) - } - - /// Set intros - pub fn set_intros(&mut self, intros: bool) { - self.inner.pin_mut().set_intros(intros) - } - - /// Set the locale pub fn set_locale(&mut self, locale: &str) { - self.inner.pin_mut().set_locale(&locale.to_string()) + self.as_pin_mut().set_locale(&locale.to_string()) } - /// Set the text pub fn set_text(&mut self, text: &str) { - self.inner.pin_mut().set_text(&text.to_string()) + self.as_pin_mut().set_text(&text.to_string()) + } + + pub fn set_autonormalize(&mut self, auto_normalize: bool) { + self.as_pin_mut().set_autonormalize(auto_normalize) + } + + pub fn set_book_name(&mut self, book_name: &str) { + self.as_pin_mut().set_book_name(&book_name.to_string()) + } + + pub fn set_intros(&mut self, intros: bool) { + self.as_pin_mut().set_intros(intros) } - /// Set the testament pub fn set_testament(&mut self, testament: u8) { - self.inner.pin_mut().set_testament(testament) + self.as_pin_mut().set_testament(testament) } - /// Set the book pub fn set_book(&mut self, book: u8) { - self.inner.pin_mut().set_book(book) + self.as_pin_mut().set_book(book) } - /// Set the chapter pub fn set_chapter(&mut self, chapter: u8) { - self.inner.pin_mut().set_chapter(chapter) + self.as_pin_mut().set_chapter(chapter) } - /// Set the verse pub fn set_verse(&mut self, verse: u8) { - self.inner.pin_mut().set_verse(verse) + self.as_pin_mut().set_verse(verse) } } @@ -187,203 +185,17 @@ impl Default for VerseKey { } } -/// Wrapper for borrowed VerseKey instances (from modules) -pub struct BorrowedVerseKey { - ptr: *mut ffi::VerseKey, -} - -impl BorrowedVerseKey { - /// Create from a raw pointer (unsafe) - pub unsafe fn from_ptr(ptr: *mut ffi::VerseKey) -> Option { - if ptr.is_null() { - None - } else { - Some(Self { ptr }) - } - } - - /// Get the book name - pub fn get_book_name(&self) -> String { - unsafe { (&*self.ptr).get_book_name() } - } - - /// Get the book abbreviation - pub fn get_book_abbrev(&self) -> String { - unsafe { (&*self.ptr).get_book_abbrev() } - } - - /// Get the OSIS book name - pub fn get_osis_book_name(&self) -> String { - unsafe { (&*self.ptr).get_osis_book_name() } - } - - /// Get the OSIS reference - pub fn get_osis_ref(&self) -> String { - unsafe { (&*self.ptr).get_osis_ref() } - } - - /// Get the OSIS reference range text - pub fn get_osis_ref_range_text(&self) -> String { - unsafe { (&*self.ptr).get_osis_ref_range_text() } - } - - /// Get the short range text - pub fn get_short_range_text(&self) -> String { - unsafe { (&*self.ptr).get_short_range_text() } - } - - /// Get the testament number - pub fn get_testament(&self) -> u8 { - unsafe { (&*self.ptr).get_testament() } - } - - /// Get the maximum testament number - pub fn get_testament_max(&self) -> u8 { - unsafe { (&*self.ptr).get_testament_max() } - } - - /// Get the book number - pub fn get_book(&self) -> u8 { - unsafe { (&*self.ptr).get_book() } - } - - /// Get the maximum book number - pub fn get_book_max(&self) -> u8 { - unsafe { (&*self.ptr).get_book_max() } - } - - /// Get the chapter number - pub fn get_chapter(&self) -> u8 { - unsafe { (&*self.ptr).get_chapter() } - } - - /// Get the maximum chapter number - pub fn get_chapter_max(&self) -> u8 { - unsafe { (&*self.ptr).get_chapter_max() } - } - - /// Get the verse number - pub fn get_verse(&self) -> u8 { - unsafe { (&*self.ptr).get_verse() } - } - - /// Get the maximum verse number - pub fn get_verse_max(&self) -> u8 { - unsafe { (&*self.ptr).get_verse_max() } - } - - /// Check if auto-normalize is enabled - pub fn is_autonormalize(&self) -> bool { - unsafe { (&*self.ptr).is_autonormalize() } - } - - /// Check if intros are enabled - pub fn is_intros(&self) -> bool { - unsafe { (&*self.ptr).is_intros() } - } - - /// Get the error code - pub fn get_error(&self) -> u8 { - unsafe { (&*self.ptr).get_error() } - } - - /// Get the locale - pub fn get_locale(&self) -> String { - unsafe { (&*self.ptr).get_locale() } - } - - /// Get the range text - pub fn get_range_text(&self) -> String { - unsafe { (&*self.ptr).get_range_text() } - } - - /// Get the short text - pub fn get_short_text(&self) -> String { - unsafe { (&*self.ptr).get_short_text() } - } - - /// Get the text - pub fn get_text(&self) -> String { - unsafe { (&*self.ptr).get_text() } - } - - /// Normalize the key - pub fn normalize(&mut self, autocheck: bool) { - unsafe { - std::pin::Pin::new_unchecked(&mut *self.ptr).normalize(autocheck); - } - } - - /// Pop an error - pub fn pop_error(&mut self) -> u8 { - unsafe { std::pin::Pin::new_unchecked(&mut *self.ptr).pop_error() } - } - - /// Set auto-normalize - pub fn set_autonormalize(&mut self, auto_normalize: bool) { - unsafe { - std::pin::Pin::new_unchecked(&mut *self.ptr).set_autonormalize(auto_normalize); - } - } - - /// Set the book name - pub fn set_book_name(&mut self, book_name: &str) { - unsafe { - std::pin::Pin::new_unchecked(&mut *self.ptr).set_book_name(&book_name.to_string()); - } - } - - /// Set intros - pub fn set_intros(&mut self, intros: bool) { - unsafe { - std::pin::Pin::new_unchecked(&mut *self.ptr).set_intros(intros); - } - } - - /// Set the locale - pub fn set_locale(&mut self, locale: &str) { - unsafe { - std::pin::Pin::new_unchecked(&mut *self.ptr).set_locale(&locale.to_string()); - } - } - - /// Set the text - pub fn set_text(&mut self, text: &str) { - unsafe { - std::pin::Pin::new_unchecked(&mut *self.ptr).set_text(&text.to_string()); - } - } - - /// Set the testament - pub fn set_testament(&mut self, testament: u8) { - unsafe { - std::pin::Pin::new_unchecked(&mut *self.ptr).set_testament(testament); - } - } - - /// Set the book - pub fn set_book(&mut self, book: u8) { - unsafe { - std::pin::Pin::new_unchecked(&mut *self.ptr).set_book(book); - } - } - - /// Set the chapter - pub fn set_chapter(&mut self, chapter: u8) { - unsafe { - std::pin::Pin::new_unchecked(&mut *self.ptr).set_chapter(chapter); - } - } - - /// Set the verse - pub fn set_verse(&mut self, verse: u8) { - unsafe { - std::pin::Pin::new_unchecked(&mut *self.ptr).set_verse(verse); +impl Clone for VerseKey { + fn clone(&self) -> Self { + let cloned_ptr = ffi::clone_verse_key(self.as_ref()); + if cloned_ptr.is_null() { + panic!("Failed to clone VerseKey"); } + Self { ptr: cloned_ptr } } } -impl Drop for BorrowedVerseKey { +impl Drop for VerseKey { fn drop(&mut self) { if !self.ptr.is_null() { unsafe { @@ -393,6 +205,28 @@ impl Drop for BorrowedVerseKey { } } -// Safety: BorrowedVerseKey can be sent between threads safely as long as -// the underlying SWORD library is thread-safe for reading -unsafe impl Send for BorrowedVerseKey {} +impl fmt::Debug for VerseKey { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if self.is_valid() { + f.debug_struct("VerseKey") + .field("text", &self.get_text()) + .field("osis_ref", &self.get_osis_ref()) + .finish() + } else { + f.debug_struct("VerseKey").field("valid", &false).finish() + } + } +} + +impl fmt::Display for VerseKey { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if self.is_valid() { + write!(f, "{}", self.get_text()) + } else { + write!(f, "") + } + } +} + +unsafe impl Send for VerseKey {} +unsafe impl Sync for VerseKey {}