Automated Resyntax fixes #698
Conversation
![resyntax-ci[bot]](https://avatars.githubusercontent.com/in/999945?s=60&v=4){kind=small}
Keep imports in `require` sorted and grouped by phase, with collections before files.
Use the `#:when` keyword instead of `when` to reduce loop body indentation.
…t-lambda` The lambda can be removed from the failure result in this `hash-ref` expression.
Internal definitions are recommended instead of `let` expressions, to reduce nesting.
This expression can be replaced with a simpler, equivalent `hash-update!` expression.
This `if` expression can be refactored to an equivalent expression using `and`.
`cond` with internal definitions is preferred over `if` with `let`, to reduce nesting
These nested `for` loops can be replaced by a single `for*` loop.
Internal definitions are recommended instead of `let` expressions, to reduce nesting.
| (parameterize ([current-eventspace progress-eventspace]) | ||
| (queue-callback (λ () (send progress-frame show #f)))) | ||
| (when success? | ||
| (let () |
There was a problem hiding this comment.
This let was in the original, but shouldn't it have gone away?
There was a problem hiding this comment.
Yup. I just checked using #lang resyntax/test that Resyntax can refactor this code, so I think the only reason it didn't happen was the fix limit.
| frame)] | ||
| [alignment '(left center)])) | ||
| (define root-message | ||
| (instantiate message% () |
There was a problem hiding this comment.
Maybe it just didn't kick in, but this could be new.
There was a problem hiding this comment.
I don't have a rule for that, actually. Filed jackfirth/resyntax#418.
| ;; note: the preference drracket:module-browser:name-length is also used for | ||
| ;; the View|Show Module Browser version of the module browser | ||
| ;; here we just treat any pref value except '3' as if it were for the long names. | ||
| (let ([selection (send module-browser-name-length-choice get-selection)]) |
There was a problem hiding this comment.
probably another one that didn't kick in, but it feels like it would be nice to limit the changes by region instead of by number of changes maybe?
There was a problem hiding this comment.
I'm considering redoing how the limiting works. It's hard to tell when something isn't being refactored because it hit the limit or because the refactoring rule didn't catch it.
| @@ -582,7 +574,7 @@ | |||
| [parent menu] | |||
| [label (choice->label-string choice)] | |||
| [callback (λ (item evt) | |||
| (hash-set! selected choice (not (hash-ref selected choice))) | |||
| (hash-update! selected choice (λ (v) (not v))) | |||
There was a problem hiding this comment.
(I don't really like hash-update and hash-update!, for the same reason that I really like for loops over map and friends. Not a big deal.)
There was a problem hiding this comment.
Filed jackfirth/resyntax#419.
|
LGTM, overall. |
| frame)] | ||
| [alignment '(left center)])) | ||
| (define root-message | ||
| (instantiate message% () |
There was a problem hiding this comment.
I don't have a rule for that, actually. Filed jackfirth/resyntax#418.
| @@ -582,7 +574,7 @@ | |||
| [parent menu] | |||
| [label (choice->label-string choice)] | |||
| [callback (λ (item evt) | |||
| (hash-set! selected choice (not (hash-ref selected choice))) | |||
| (hash-update! selected choice (λ (v) (not v))) | |||
There was a problem hiding this comment.
Filed jackfirth/resyntax#419.
| (cond | ||
| [(null? levels) (void)] | ||
| [else |
There was a problem hiding this comment.
This is equivalent to (unless (null? levels) ...). Filed jackfirth/resyntax#420.
| ;; note: the preference drracket:module-browser:name-length is also used for | ||
| ;; the View|Show Module Browser version of the module browser | ||
| ;; here we just treat any pref value except '3' as if it were for the long names. | ||
| (let ([selection (send module-browser-name-length-choice get-selection)]) |
There was a problem hiding this comment.
I'm considering redoing how the limiting works. It's hard to tell when something isn't being refactored because it hit the limit or because the refactoring rule didn't catch it.
| (parameterize ([current-eventspace progress-eventspace]) | ||
| (queue-callback (λ () (send progress-frame show #f)))) | ||
| (when success? | ||
| (let () |
There was a problem hiding this comment.
Yup. I just checked using #lang resyntax/test that Resyntax can refactor this code, so I think the only reason it didn't happen was the fix limit.
Resyntax fixed 20 issues in 1 file.
let-to-definehash-set!-ref-to-hash-update!if-let-to-condif-else-false-to-andnested-for-to-for*cond-let-to-cond-definehash-ref-with-constant-lambda-to-hash-ref-without-lambdawhen-expression-in-for-loop-to-when-keywordtidy-require