fkautz commented 3 years ago

In naml, we need to know what packages are included in the AST so that we can handle imports properly.

This patch adds Result.Packages which informs the caller of AST about which packages the returned AST uses.

Signed-off-by: Frederick F. Kautz IV fkautz@alumni.cmu.edu

codecov-commenter commented 3 years ago

Codecov Report

Merging #14 (4babe51) into main (c24911a) will increase coverage by 0.57%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #14      +/-   ##
==========================================
+ Coverage   73.32%   73.89%   +0.57%     
==========================================
  Files           6        6              
  Lines         716      724       +8     
==========================================
+ Hits          525      535      +10     
+ Misses        144      143       -1     
+ Partials       47       46       -1

Impacted Files	Coverage Δ
valast.go	`80.25% <100.00%> (+0.92%)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c24911a...4babe51. Read the comment docs.

slimsag commented 3 years ago

Thank you for the PR @fkautz !

Instead of adding a new top-level method ASTWithPackages, how about just adding this as a new field on the existing Result type? I think that'd be super reasonable and unlikely to break any existing usages.

Aside from that, this looks good (though I'd love a test case if you're up for it!)

fkautz commented 3 years ago

I'll add a test case tomorrow.

fkautz commented 3 years ago

@slimsag finally got around to a test case, let me know what you think or if you want anything else added.

slimsag commented 2 years ago

I will fix https://github.com/hexops/valast/issues/17 and release a new tagged version soon

hexops / valast

add Result.Packages for insight into the packages used by the returned AST #14

Codecov Report