Added support for integrating the package_version#926
Added support for integrating the package_version#926hitbear wants to merge 6 commits intomozilla:masterfrom
Conversation
…mment of the header file. The new command line argument for this is --package-version. The TOML config file flag is package_version=true||false.
emilio
left a comment
emilio
left a comment
There was a problem hiding this comment.
Looks fine, but needs a test, and probably undoing the rust-toolchain change. Let me know if you need any help with that.
| @@ -1,2 +0,0 @@ | |||
| [toolchain] | |||
| channel = "nightly" | |||
There was a problem hiding this comment.
Thank you for your response. Yes I fixed both requests but I think that I need some help to write an appropriate test.
There was a problem hiding this comment.
You can add files to tests/rust where your toml file has the new option.
There was a problem hiding this comment.
Ok, in my latest commit I added a test crate package_version that checks whether the file is generated with the correct package version.
Since some other tests failed with cython, I modified cbindgen. Now comments such as header, trailer, etc. are written to the file after a check if the comment should be in C/C++ syntax (/*) or Python syntax (''').
cargo test now tells ok. 145 passed;
And I also run
cargo fmt;)
emilio
left a comment
emilio
left a comment
There was a problem hiding this comment.
Thanks! I think we should keep user inputs as-is and not mangle them, but the rest looks fine, thank you!
src/bindgen/bindings.rs
Outdated
| out.new_line_if_not_start(); | ||
| match self.config.language { | ||
| Language::C | Language::Cxx => { | ||
| write!(out, "/* Package version: {} */", self.package_version,); |
There was a problem hiding this comment.
I'd remove the trailing comma here and below.
src/bindgen/bindings.rs
Outdated
| if let Some(ref f) = self.config.autogen_warning { | ||
| out.new_line_if_not_start(); | ||
| write!(out, "{}", f); | ||
| match self.config.language { |
There was a problem hiding this comment.
I think we should keep these as it was, this contents are controllable by the user so we shouldn't try to mangle them.
| out.new_line(); | ||
| } | ||
|
|
||
| if let Some(ref f) = self.config.header { |
There was a problem hiding this comment.
Same here, this should probably remain as it was.
There was a problem hiding this comment.
So with these two commits, I removed the user input modification - and adjusted also the package_version test. Yes I see, it could be dangerous to change a users input.
| @@ -0,0 +1,7 @@ | |||
| package_version = true | |||
|
|
|||
| [parse.expand] | |||
There was a problem hiding this comment.
Do you need to use expand for this to work?
| [parse.expand] | ||
| crates = ["package_version"] | ||
| features = ["cbindgen"] | ||
|
|
There was a problem hiding this comment.
remove redundant newlines?
| @@ -0,0 +1,8 @@ | |||
| #[repr(C)] | |||
| pub struct Foo { | |||
| #[cfg(not(feature = "cbindgen"))] | |||
There was a problem hiding this comment.
This seems orthogonal to the feature you're testing?
There was a problem hiding this comment.
Yes, actually I could remove the whole stub code but in my latest commit, i removed just this line.
…mment of the header file. The new command line argument for this is --package-version. The TOML config file flag is package_version=true||false. Closes #926 Co-Authored-By: Emilio Cobos Álvarez <[email protected]>
…mment of the header file. The new command line argument for this is --package-version. The TOML config file flag is package_version=true||false. Closes #926 Co-Authored-By: Emilio Cobos Álvarez <[email protected]>
…mment of the header file. The new command line argument for this is --package-version. The TOML config file flag is package_version=true||false. Closes #926 Co-Authored-By: Emilio Cobos Álvarez <[email protected]>
2 participants
The package version information appears in a comment of the header file. The new command line argument for this is --package-version. The TOML config file flag is package_version=true||false.
This new feature is helpful when you are working on a growing Rust library with new functions and structs. The package version entry in the header file appears as
"/Package version: {}/",
and gives a better overview which header file fits to which version of the library.