noprompt / meander

Tools for transparent data transformation
MIT License
923 stars 55 forks source link

Improve docs for `rewrite` #106

Closed d4hines closed 4 years ago

d4hines commented 4 years ago

Looking at @timothypratley's PR #105, that rewrite is really hard for me to follow, because the arity of rewrite isn't immediately obvious. The docstring only expands it into find and subst, but that leaves room for ambiguity on how each parameter is interpreted. The main docs also don't clarify the arity much (though referencing match is a good hint). This should be improved.

I think my main issue is that the docs hint that the arity should be even (for pairs of matches/rewrites), but Timothy's example is odd.

jimmyhmiller commented 4 years ago

I can work on improving the docstring for rewrite. search, find, match, rewrite, and rewrites all have the same arities. They take the data they will operate on as their first argument. Then they take multiple left hand and right hand sides. So it will always be an odd number. So for match it is:

(match x
  pattern_1 expr_1
  ,,,
  pattern_n expr_n)

For rewrite it is the same, but the right hand side are substitution patterns.

(rewrite x
  pattern_1 subst_pattern_1
  ,,,
  pattern_n subst_pattern_n)

This could be confusing because you've seen people post meander.strategy.epsilon/rewrite examples. Strategy is a combinator library, so they return functions instead of taking data as the first argument.

timothypratley commented 4 years ago

I think the signature of this macro and it's siblings are all hiding information and should be changed:

consider instead

(defmacro rewrite [data from-pattern to-pattern & more-from-to-pattern-pairs])

  1. (m/rewrite 1) currently is nil. This is very surprising (I expected 1 or fail). The behavior could be changed to identity but I don't consider that meaningful.

  2. There is no hint that & patterns must be even right now.

Obviously these things can and should be made clearer in the doc-string, but the params is the first port of call and I for one often look at the params without the full docs.

What do you think? Happy to help if this seems reasonable.

noprompt commented 4 years ago

I used the term clauses since thats what cond and case use. match is the only one of the macros that says anything about the number of arguments needing to be odd and the rest of them transitively refer having the same argument structure as match. There is also the fdef definitions.

I think I would be fine with your suggestion @timothypratley, however, only in the :arglists meta data should be updated and not an actual change to the arity. I would prefer the names match-pattern and subst-pattern instead of from-pattern and to-pattern.