Automated Resyntax fixes #1438
Automated Resyntax fixes #1438
Conversation
![resyntax-ci[bot]](https://avatars.githubusercontent.com/in/999945?s=60&v=4){kind=small}
The `define-simple-macro` form has been renamed to `define-syntax-parse-rule`.
This expression is equivalent to calling the `positive?` predicate.
The `syntax-disarm` function is a legacy function that does nothing.
Internal definitions are recommended instead of `let` expressions, to reduce nesting.
This `if` expression can be refactored to an equivalent expression using `and`.
This `match` expression can be simplified using `match-define`.
This `map` operation can be replaced with a `for/list` loop.
The `define` form supports a shorthand for defining functions.
Providing the same identifier multiple times is unnecessary.
Using `cond` instead of `if` here makes `begin` unnecessary
The `let` expression in this `begin0` form can be extracted into the surrounding definition context.
This `case-lambda` form only has one case. Use a regular lambda instead.
This use of `define-values` is unnecessary.
This negated `unless` expression can be replaced by a `when` expression.
This `sort` expression can be replaced with a simpler, equivalent expression.
This negated `when` expression can be replaced by an `unless` expression.
Internal definitions are recommended instead of `let` expressions, to reduce nesting.
Internal definitions are recommended instead of `let` expressions, to reduce nesting.
| (raise-arguments-error | ||
| (string->symbol (format "typed/racket/~a" (keyword->string (syntax-e te-attr)))) |
There was a problem hiding this comment.
@jackfirth this could use format-symbol from racket/syntax
There was a problem hiding this comment.
Added a new rule for this in jackfirth/resyntax#442.
| @@ -143,8 +143,7 @@ | |||
| [#:for-each (f) (for-each f ps)] | |||
| [#:custom-constructor/contract | |||
| (-> (listof (or/c TypeProp? NotTypeProp? LeqProp?)) OrProp?) | |||
| (let ([ps (sort ps (λ (p q) (unsafe-fx<= (eq-hash-code p) | |||
There was a problem hiding this comment.
@jackfirth my guess is that this change is slower
There was a problem hiding this comment.
If it is, that seems like a missed optimization opportunity in sort.
There was a problem hiding this comment.
It's really an issue of inlining.
| (map f* vals) | ||
| (and call-cc (map f* call-cc)))])) | ||
| (match-define (pt-seq vals call-cc) seq) | ||
| (define (f* a) |
There was a problem hiding this comment.
@sorawee why is this on two lines?
There was a problem hiding this comment.
It's a "community"'s preference to always format function definition this way. It used to support one-line format, but IIRC, either @jackfirth or @Metaxal gathered supporters to NOT do the one-line format, and I made the change accordingly.
There was a problem hiding this comment.
I would be interested to find that discussion; I strongly disagree.
There was a problem hiding this comment.
If it was me, I don't remember having a strong opinion about this. Perhaps it was more of a pragmatic choice at that time(?). But again, perhaps it wasn't me 😁
There was a problem hiding this comment.
It's @Metaxal. I'm very lucky that I wrote your name in the commit message 😂
There was a problem hiding this comment.
Haha indeed! Though "Suggestion by" sounds a bit different from "gathered supporters to NOT do" ;)
Again, I don't feel particularly strongly about this, although I do have a preference for two lines so that the function header stands out. I'm not going to die on this hill though.
There was a problem hiding this comment.
It was also me. I was (and still am) of the strong opinion that function definitions should never be on a single line. Otherwise it's hard to tell them apart from variable definitions.
There was a problem hiding this comment.
Functions are just another kind of variable. ;)
| (let-values ([(l c p) (port-next-location port)]) | ||
| (raise-read-error (format "typed expression ~a not properly terminated" | ||
| (syntax->datum name)) | ||
| src |
There was a problem hiding this comment.
What's your preferred format?
There was a problem hiding this comment.
having all the additional arguments on the same line. I realize that this preference of mine is not widely shared but I'm still grumpy.
There was a problem hiding this comment.
FWIW, I also find the new indentation jarring: there's a big empty space with just a few letters. It's quite wasteful information-wise.
There was a problem hiding this comment.
I agree with this particular case, but I'm not sure how to turn this into code in a way that makes other cases also look good.
E.g.: compare:
(format "This is a very long string"
first-long-string second-long-string third-long-string fourth-long-string))
vs:
(format "This is a very long string"
first-long-string
second-long-string
third-long-string
fourth-long-string))
I think the second one is preferable.
There was a problem hiding this comment.
Yes, I agree with that. I bet you can make resyntax do it though. :)
There was a problem hiding this comment.
I could attempt something. Do you think this case with raise-read-error comes up enough to be worth it?
There was a problem hiding this comment.
No it would have to be more general, but I bet the case where format is involved happens a lot.
There was a problem hiding this comment.
More general is trickier because it makes it difficult to pick a good name for the introduced variable.
There was a problem hiding this comment.
Good excuse to use LLM to find a name?
Resyntax fixed 41 issues in 20 files.
single-clause-match-to-match-definelet-to-definedefine-simple-macro-to-define-syntax-parse-ruleif-else-false-to-andzero-comparison-to-positive?map-to-forsort-with-keyed-comparator-to-sort-by-keycase-lambda-with-single-case-to-lambdaif-begin-to-conddefine-lambda-to-defineinverted-unlessprovide-deduplicationbegin0-let-to-define-begin0inverted-whendefine-values-values-to-definecond-let-to-cond-definesyntax-disarm-migration