Replace `ExpanderFunctions` with `Expander` trait
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 withinVariables
orDirectoryStack
- Had weird artifacts like having a direct reference to a
Variables
but also a separatevariable(&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
implementExpander
and useShell
as theExpander
for most non-testing instances - Refactored
let_assignment
andexport_assigment
intoVariableStore
trait, which represents storing aBinding
as either a local variable or a global (exported) variable- This was done as
let_assignment
andexport_assignment
took in a mutableVariables
, but also required anExpanderFunctions
to expand the right hand side. Instead,Shell
now implementsVariableStore
- This was done as
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!