Skip to content

Replace `ExpanderFunctions` with `Expander` trait

Michael Aaron Murphy requested to merge huntergoldstein:expand_trait into master

Created by: huntergoldstein

Problem: ExpanderFunctions was somewhat unruly as:

  • It was mostly used through get_expanders! which made tracking down errors more cumbersome as it was generated through a macro
  • Hard to extend sensibly as the main instance, get_expanders!, would lose access to any information that was not contained within Variables or DirectoryStack
  • Had weird artifacts like having a direct reference to a Variables but also a separate variable(&str, bool) -> Option<Value> function?

Solution: Replace ExpanderFunctions with an Expander trait which provides roughly the same interface as ExpanderFunctions but allows the backing implementation to be more complex

Changes introduced by this pull request:

  • Removed ExpanderFunctions, get_expanders! etc.
  • Add the aforementioned Expander trait
  • Have Shell implement Expander and use Shell as the Expander for most non-testing instances
  • Refactored let_assignment and export_assigment into VariableStore trait, which represents storing a Binding as either a local variable or a global (exported) variable
    • This was done as let_assignment and export_assignment took in a mutable Variables, but also required an ExpanderFunctions to expand the right hand side. Instead, Shell now implements VariableStore

Drawbacks: I am not sure this is as performant as ExpanderFunctions but its more idiomatic IMO.

Fixes: This will make #424 (closed) much much easier.

State: Good to go pending review!

Merge request reports