unhandled kwargs throw errors

[ericphanson]

closes #416

My editor is configured to remove trailing whitespace, which caused some diffs here. Let me know if that's too annoying and I'll undo it.

Locally I get the test failures:

Parse core.jl: Test Failed at /Users/eph/JuliaSyntax.jl/test/test_utils.jl:237
  Expression: parsers_agree

Parse core.jl: Test Failed at /Users/eph/JuliaSyntax.jl/test/test_utils.jl:241
  Expression: reduced_failures == []
   Evaluated: AbstractString["for f() in 1:3\n        push!(a, f())\n        push!(fs, f)\n    end", " {{}}where{}"] == Any[]

same as in #420, which I think are unrelated.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.17%. Comparing base (a6f2d15) to head (c63e9c7).
Report is 47 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #422      +/-   ##
==========================================
+ Coverage   94.61%   96.17%   +1.56%     
==========================================
  Files          14       14              
  Lines        4178     4185       +7     
==========================================
+ Hits         3953     4025      +72     
+ Misses        225      160      -65     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[c42f]

It's confusing to know what to do with this part: do we want all build_tree(::Type{TreeType}, ...) methods to need to know about each other for all TreeType? This seems impossible (eg, external packages) which would imply these should just be an error rather than ignored.

But on the other hand making this an error means build_tree() with keywords can't be used generically.

It seems there's two consistent things to do:

• Be generic and ignore unknown keywords (current status quo. not a good user experience, as in MethodError: no method matching isless(::Int64, ::Nothing) #341)
• Throw on unknown keywords (makes parseall etc couple the TreeType with keywords)

This PR takes an awkward middle path between those two options. Which might be ok but seems inconsistent?

[ericphanson]

Yeah, I guess if we want a more decoupled approach, we could copy Makie and add a validate_kwargs function which is called on the tree type and the passed-in kwargs (and throws or returns nothing), and is called in parseall or _parse or something like that. Then custom trees which wish to support additional kwargs would need to define a validate_kwargs method to allow them to pass through.

[c42f]

That sounds promising! So we define validate_parse_args(::Type{GreenNode}; kws...) etc and users with custom trees can overload it?