racket / scribble

Other
197 stars 90 forks source link

Replace let forms with definitions in scribble/search #289

Closed jackfirth closed 3 years ago

jackfirth commented 3 years ago

This pull request was generated by an automated tool, with some by-hand postprocessing to preserve comments. The tool replaces uses of the various let forms with equivalent define forms, when doing so would reduce nesting and when it can prove that the binding structure remains unchanged. Parts of this change involved changing some code from using if to using cond, as well as other similar changes needed to introduce a definition context.

The tool produces moderately well-formatted code, but it makes a conscious effort to avoid changing the formatting of any existing subforms that were not refactored.

Note: the tool is called resyntax, and is implemented here.

mflatt commented 3 years ago

This looks cool!

But it looks like there's a problem. I tried building with this patch, and I got need-result?: undefined; cannot use before initialization when raco setup tried to run the first document. I see that the revised code has (define need-result? (and need-result? (not here-result))), probably where the original code was intentionally shadowing need-result? with let.

jackfirth commented 3 years ago

Oops. There's a bit of logic to catch those cases with let, but not let*. I'll fix the tool and repush.

jackfirth commented 3 years ago

@mflatt I've fixed the bug in the tool that produced the buggy need-result? definition and repushed the generated changes.

mflatt commented 3 years ago

Thanks!