arcnmx / wireplumber.rs

wireplumber rust bindings
https://arcnmx.github.io/wireplumber.rs/main/wireplumber/
MIT License
21 stars 3 forks source link

Rename non-varargs functions to their varargs equivalent #28

Closed tom-a-wagner closed 1 year ago

tom-a-wagner commented 1 year ago

Since varargs function are ignores because they cannot be safely called from rust, but in Wireplumber usually have a less verbose name than their non-varargs equivalent (e.g. do_stuff vs do_stuff_full or do_stuff_array), this commit renames the non-varargs functions to the less verbose name.

Some of these are pretty clear IMO, e.g. theres no reason to type add_keys_array when no non-array variant exist.
A few others might be more debatable, e.g. new vs new_empty for properties, and port vs lookup_port on node.

Also marks pattern = "(new|matches)" on ObjectInterest as ignored as they aren't actually manually implemented.

tom-a-wagner commented 1 year ago

Or perhaps Properties::new_empty() could be renamed to Properties::empty() instead of Properties::new().

The former stays close to the C naming, just with the rust convention of no having a new prefix.

On the other hand, the latter follows the convention of rust std collections where new creates an empty container.

Thoughts?

arcnmx commented 1 year ago

Sorry, could you rebase onto 4522b9b548457de049b62afa07430888f5d0e038 and rerun gir to clean up the diff? I thought I had filtered out the // from gir-files noise but nope.

this commit renames the non-varargs functions to the less verbose name.

Sounds good to me!

Or perhaps Properties::new_empty() could be renamed to Properties::empty() instead of Properties::new().

I don't have a strong preference personally, new() works fine and fits Rust conventions as you've said, and "empty" is implied anyway.

Some of these are pretty clear IMO, e.g. theres no reason to type add_keys_array when no non-array variant exist. A few others might be more debatable, e.g. new vs new_empty for properties, and port vs lookup_port on node.

Yeah, there's a difference between those two cases for sure. I think it does make sense to keep closer to the upstream wording like lookup_port when it's meaningful to what the function does and not just used to describe the argument types or disambiguate an overload. (while I do have a tendency to abbreviate when there is no other operation it could be confused with, I don't necessarily believe that being overly succinct is ideal)

tom-a-wagner commented 1 year ago

Sorry, could you rebase onto 4522b9b and rerun gir to clean up the diff? I thought I had filtered out the // from gir-files noise but nope.

@arcnmx Done.