Fix #460; for introduces a bound variable #462
Conversation
|
@albertnetymk I would suggest that you add a bit of info to your PRs. A link to the issue is good, but what did you do to fix it? What is the error? How did you fix it? It feels like I need to look into the code to understand what is going on |
|
I agree. I have no clues with your PR. |
|
this commit could have been super-simple to review if it had been split into two, meaningful changes and removing one level of indentation 😒 . The fact that indentation was removed, even though it's nice, adds lots of noise to the whole function, which looks as if it was completely new. |
|
@kikofernandez: Once again I highly recommend hiding changes to whitespace: https://github.com/parapluu/encore/pull/462/files?w=1 |
|
@EliasC I always forget about that flag. In any case, if you use that you would not be able to see any change when programming in the Whitespace language... 😶 |
| fvDecls (x, expr) (free, bound) = | ||
| (freeVariables' (x:bound) expr ++ free, x:bound) | ||
| freeVariables' bound e@For{name, step, src, body} = | ||
| concatMap (freeVariables' (name:bound)) $ getChildren e |
There was a problem hiding this comment.
may I suggest this other form (I think it looks more Haskelly and cleaner):
freeVariables' (name:bound) =<< getChildren e
|
@EliasC I just saw that, while using the |
|
waiting for @albertnetymk or any other person to reply the only comment and merging afterwards |
|
@kikofernandez: That is a downside, but at least it makes browsing simpler! |
|
Replaced |
|
yes! I noticed that as well! |
No description provided.