Misc improvements

[Imberflur]

These are various changes I made while reading the code (or in response to notes I made while reading the code).

For various bits of behavior I wanted to understand and bugs I wanted confirm before fixing I added some new tests.

Summary of changes

Bug fixes

• Fixed bug in reload_paths in certain cases where new files prefixed by existing paths would overwrite the existing file.
• Fixed bug in reload_files where some files failing to load would lead to a mismatch in iteration over file IDs and file values.

Method reworks

• Rename CtxNode::filter to CtxNode::iter_all_children and have the caller use Iterator::filter instead of taking a filter closure. This separates iterating all children and the filtering logic, which should make the logic more clear to readers.
• Rename CtxNode::walk/walk_mut to CtxNode::node_at/node_at_mut and remove closure parameter. Instead return node reference that caller can use. Again this separate the concern of getting the node and doing things with it. In some cases this takes more lines, but it uses the Option methods familiar to many. Additionally, in one case this avoids needing to re-get the actual node.
• Rename ref_kinds to refs_of_kind since it returns references of a specific kind rather than a list of kinds.

Misc

• Documentation added to various methods and types.
• Removed a few type aliases that were only used in a few places. IMO avoiding layers of indirection outweighs the benefits of aliases in these cases.
• Minor improvements to bauble_test macro.
• Remove unused default_uses parameter in value::register_assets.
• Add TODOs to a couple places that may need further investigation.

[MavethGH]

This needs to be updated now that this returns a Result

[Imberflur]

Only the inner walk_find_inner function was changed to return a result

[MavethGH]

No substantial issues that I can find. Just a few typos and documentation improvements.

[hazelwiss]

Yeah these changes look good to me too!

[hazelwiss]

Should it ever be possible that a redirect leads to a node which is None? Seems like a case we might want to catch otherwise?

[Imberflur]

I will need to look into this. For reference, redirects are excusively used to implement default uses.

[Imberflur]

It looks like it would be possible to add default uses that point to non-existent paths. I don't know how we want to handle that from a user standpoint though? Produce an error when building the Context? Perhaps this can be deferred to an issue, especially since the changes here don't modify the behavior for this case.

Also note that default uses apply to all namespaces, not just types . I'm not sure if that is intended.

[Imberflur]

#56

[hazelwiss]

I do not fully understand the doc comment Note, if a file `a` exists and a file `a::b::c` exists, `a::b` will get the ID of the file at `a`. here

[Imberflur]

If a file a::b::c exists, then a node will be created for b but this node will not be associated to a file necause there is no a::b file. walk_find will check the each parent node if the closure returns None for the node at the end of the provided path (in this example b). So it will return the file ID of a (if there is a file a).

If a::b::c didnt exist then b would not exist (as there is no file for it in this example).

When I discussed with Isse this behavior was at the time of writing to keep the option open for defining assets like foo::bar in-file without creating a foo submodule (or having some form of inline submodules).