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

feat: add flag set/unset method #108

Merged
merged 4 commits into from
Jan 6, 2025
Merged

feat: add flag set/unset method #108

merged 4 commits into from
Jan 6, 2025

Conversation

dwpeng
Copy link
Contributor

@dwpeng dwpeng commented Dec 9, 2024

@jguhlin
Copy link
Owner

jguhlin commented Dec 11, 2024

Thanks! I'll give this a deeper look in a few days, but should have no problems. Next release might be awhile unless you need this sooner?

Anyway to auto generate docs for the functions from macros? No worries if not, but it'd be nice if there was a mapping or something. I'll dig around for it too

Copy link
Contributor Author

@dwpeng dwpeng left a comment

Choose a reason for hiding this comment

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

Thanks! I'll give this a deeper look in a few days, but should have no problems. Next release might be awhile unless you need this sooner?

Anyway to auto generate docs for the functions from macros? No worries if not, but it'd be nice if there was a mapping or something. I'll dig around for it too

Thanks for your code review. I found a post on stackoverflow: https://stackoverflow.com/questions/65426621/is-there-any-way-to-inline-a-const-inside-a-doc-comment-rendered-by-cargo-doc.

I tried concat! in #[doc = concat!(..)], but it didn't work. And then, I tried to use paste to implement doc, it's worked!

diff --git a/minimap2-sys/Cargo.toml b/minimap2-sys/Cargo.toml
index 82e45af..1649a83 100644
--- a/minimap2-sys/Cargo.toml
+++ b/minimap2-sys/Cargo.toml
@@ -34,6 +34,7 @@ crate-type = ["staticlib", "cdylib", "rlib"]
 
 [dependencies]
 libz-sys = { version = "1.1", default-features = false, features = ["libc"] }
+paste = "1.0.15"
 
 [build-dependencies]
 pkg-config = "0.3"
diff --git a/minimap2-sys/src/lib.rs b/minimap2-sys/src/lib.rs
index b8216f7..70ac12d 100644
--- a/minimap2-sys/src/lib.rs
+++ b/minimap2-sys/src/lib.rs
@@ -12,6 +12,8 @@ unsafe impl Send for mm_idx_t {}
 unsafe impl Send for mm_idx_reader_t {}
 unsafe impl Send for mm_mapopt_t {}
 
+use paste::paste;
+
 impl Drop for mm_idx_t {
     fn drop(&mut self) {
         unsafe { mm_idx_destroy(self) };
@@ -30,18 +32,23 @@ impl Default for mm_mapopt_t {
     }
 }
 
+
 macro_rules! add_flag_methods {
     ($ty:ty, $struct_name:ident, $(($set_name:ident, $unset_name:ident, $flag:expr)),+) => {
         impl $struct_name {
             $(
-                #[inline(always)]
-                pub fn $set_name(&mut self) {
-                    self.flag |= $flag as $ty;
-                }
-
-                #[inline(always)]
-                pub fn $unset_name(&mut self) {
-                    self.flag &= !$flag as $ty;
+                paste! {
+                    #[inline(always)]
+                    #[doc = "Set the " $flag " flag"]
+                    pub fn $set_name(&mut self) {
+                        self.flag |= $flag as $ty;
+                    }
+
+                    #[inline(always)]
+                    #[doc = "Unset the " $flag " flag"]
+                    pub fn $unset_name(&mut self) {
+                        self.flag &= !$flag as $ty;
+                    }
                 }
             )*
         }

Copy link
Contributor Author

@dwpeng dwpeng left a comment

Choose a reason for hiding this comment

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

In fact, these method that start with set/unset already have a better name, and can give user to clear information. Is it really necessary to add doc.

If necessary, how should these documents be written? Do you need to explain the behavior of each flag in minimap2? Or simply write 'set/user XXX flag'

@jguhlin
Copy link
Owner

jguhlin commented Dec 12, 2024 via email

@dwpeng
Copy link
Contributor Author

dwpeng commented Dec 13, 2024

Hi, @jguhlin, follow your advice, I have add auto doc generation support for functions from macros. Please give a code review.

image
image

By the way, if a function named is_set_FLAG is needed. This function can test if one flag is set or unset.

For example:

let aligner = ...
let is_cigar = aligner.mapopt.is_set_cigar();
assert_eq!(is_cigar, true);

@jguhlin jguhlin merged commit eb76878 into jguhlin:main Jan 6, 2025
10 checks passed
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.

2 participants