-
-
Notifications
You must be signed in to change notification settings - Fork 103
Automated Resyntax fixes #1463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Automated Resyntax fixes #1463
Conversation
Specify a line mode of `'any` with `read-line` to avoid differences between Windows and other platforms.
Internal definitions are recommended instead of `let` expressions, to reduce nesting.
This use of `format` does nothing.
Using `cond` instead of `if` here makes `begin` unnecessary
This use of `define-values` is unnecessary.
This `match` expression can be simplified using `match-define`.
Using `when` and `unless` is simpler than a conditional with an always-throwing branch.
`cond` with internal definitions is preferred over `if` with `let`, to reduce nesting
Internal definitions are recommended instead of `let` expressions, to reduce nesting.
This `if`-`else` chain can be converted to a `cond` expression.
The `define` form supports a shorthand for defining functions.
This `if` expression can be refactored to an equivalent expression using `and`.
This list-constructing expression can be simplified
This `let` expression can be pulled up into a `define` expression.
@@ -915,7 +915,8 @@ | |||
[(? variance:const?) S] | |||
[(? variance:co?) S] | |||
[(? variance:contra?) T] | |||
[(? variance:inv?) (let ([gS (generalize S)]) (if (subtype gS T) gS S))])) | |||
[(? variance:inv?) (define gS (generalize S)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the newline here because of the define or because of the overall line length? @jackfirth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. This is a bug in Resyntax's efforts to avoid reformatting unchanged code. It should have produced this:
[(? variance:inv?)
(define gs (generalize S))
(if (subtype gS T) gS S)]
;; +/- (3 args) | ||
[((~var e1 (w/obj+type -Int)) (~var e2 (w/obj+type -Int)) (~var e3 (w/obj+type -Int))) | ||
(define (sign o) | ||
(if plus? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this if
on multiple lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's more context in sorawee/fmt#75, but the gist is that a lot of folks find one-line if
expressions difficult to read even when they're short. The current rule that fmt
uses is to only allow one-line if
s if the true and false branches are atoms.
This one does seem a bit unnecessary. I'd like to get more data before suggesting changes to fmt
's handling of these, however. Could you keep your eyes out for other cases like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few others in this PR, all of which I dislike. In general I think fmt
ended up in a place where it prefers adding a lot more newlines than I like overall (here, in define
, in function application) which makes resyntax PRs harder to be happy about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like the multi-line if
s. Can we figure out a way to configure that?
Resyntax fixed 33 issues in 20 files.
define-lambda-to-define
let-to-define
read-line-any
nested-if-to-cond
always-throwing-if-to-when
consing-onto-static-list
if-begin-to-cond
if-else-false-to-and
single-clause-match-to-match-define
if-let-to-cond
define-values-values-to-define
cond-let-to-cond-define
define-let-to-double-define
format-identity