Skip to content
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

Run Resyntax on various files #433

Merged
merged 2 commits into from
Sep 18, 2024
Merged

Conversation

jackfirth
Copy link
Contributor

Similar to #432, this pull request runs Resyntax on various files. Each commit is a single pass from Resyntax, and the commit message lists the refactoring rules that were applied.

Fixed 9 issues in 4 files.

  * Fixed 6 occurences of `let-to-define`
  * Fixed 2 occurences of `arrow-contract-with-rest-to-arrow-contract-with-ellipses`
  * Fixed 1 occurence of `nested-for-to-for*`
Fixed 1 issues in 1 files.

  * Fixed 1 occurence of `if-begin-to-cond`
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resyntax analyzed 4 files in this pull request and found no issues.

(cond
[(or (eof-object? c) (char=? c #\newline)) null]
[(char=? c #\@) (cons (recur) (loop))]
[else (cons (string c) (loop))])))))))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, this is a rare place where I find the refomatting suboptimal. I often prefer to write cond "questions" and "answers" on separate lines so that I can see all the possible answers more clearly. I don't object enough to stand in the way of the overall improvements, though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree w/ Matthew here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @sorawee, since this is a fmt choice.

In this case I disagree, since everything is so short. But I can understand the reasoning.

Copy link
Contributor Author

@jackfirth jackfirth Aug 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you also prefer that the else and its accompanying expression be on opposite lines? The cond expression on line 23 above has the regular branch split across lines, but the else clause is a single line.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, yes. I often go back and forth on this while editing code, and usually end up happiest after I've added newlines consistently, but probably not always.

@mflatt mflatt merged commit 3fde986 into racket:master Sep 18, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants