Allow the binding generator to not depend on builtin bindings.

[mhammond]

Previously there were internal Config details which only worked with
builtin bindings types. This leans in to the BindingGenerator trait
to make things less coupled with the builtin bindings and better
for external bindings.

[SalvatoreT]

If I could wave a magic wand, I'd also have the API take in the actual Config instead of a path to the file that needs to be parsed. I think think the Config loading + parsing should happen in the CLI tool (as soon as possible).

[mhammond]

Can you please elaborate here? I've some vague ideas for going further in a later patch and I'd like to understand your thoughts. Here in particular I don't think we have the path to the TOML yet - we use the cargo metadata to find potentially one TOML file per crate.

What I think I'd like to do is work out how to separate the Config from the Generator - this would mean everything could pass around a dyn BindingGenerator and a dyn BindingConfig which would simplify a number of things. However, the only way I can see to make that work is to lean in to std::any::Any and have the binding downcast from the dyn BindingConfig back into its concrete Config. Any thoughts on that?

[SalvatoreT]

I like it!

[SalvatoreT]

Oops, I meant to comment on generate_external_bindings instead of find_source! Instead of loading a config and an override file and then parsing them to Config, I think that should happen outside the library and then pass in the Config. Right now, to use generate_external_bindings, you have to have a file-based configuration.

uniffi-rs/uniffi_bindgen/src/lib.rs

Lines 223 to 253 in 6b09f11

	pub fn generate_external_bindings<T: BindingGenerator>(
	binding_generator: &T,
	udl_file: impl AsRef<Utf8Path>,
	config_file_override: Option<impl AsRef<Utf8Path>>,
	out_dir_override: Option<impl AsRef<Utf8Path>>,
	library_file: Option<impl AsRef<Utf8Path>>,
	crate_name: Option<&str>,
	try_format_code: bool,
	) -> Result<()> {
	let crate_name = crate_name
	.map(|c| Ok(c.to_string()))
	.unwrap_or_else(|| crate_name_from_cargo_toml(udl_file.as_ref()))?;
	let mut component = parse_udl(udl_file.as_ref(), &crate_name)?;
	if let Some(ref library_file) = library_file {
	macro_metadata::add_to_ci_from_library(&mut component, library_file.as_ref())?;
	}
	let crate_root = &guess_crate_root(udl_file.as_ref()).context("Failed to guess crate root")?;
	let config_file_override = config_file_override.as_ref().map(|p| p.as_ref());
	let config = {
	let mut config = load_initial_config::<T::Config>(crate_root, config_file_override)?;
	config.update_from_ci(&component);
	if let Some(ref library_file) = library_file {
	if let Some(cdylib_name) = crate::library_mode::calc_cdylib_name(library_file.as_ref())
	{
	config.update_from_cdylib_name(cdylib_name)
	}
	};
	config
	};

[mhammond]

Right now, to use generate_external_bindings, you have to have a file-based configuration.

This seems tricky to avoid in "library mode" - we need to possibly load one file per crate, but don't know the list of crates before hand. What would a non-file-based config setup look like in this scenario?

[bendk]

The code looks fine to me and I have a vague sense of how this will simplify the bindings generation. But could you update the commit/PR message to discuss exactly how this will help?

[bendk]

I didn't get this docstring at first, I think it refers to the match languages statement which does seem tedious. How about moving it to the match to make it more obvious.