Automated Resyntax fixes

[resyntax-ci]

This is an automated change generated by Resyntax.

Pass 1

Applied 1 fix to drracket/drracket/private/in-irl-namespace.rkt

• Line 256, tidy-require: Keep imports in require sorted and grouped by phase, with collections before files.

Applied 12 fixes to drracket/drracket/private/tools.rkt

• Line 3, tidy-require: Keep imports in require sorted and grouped by phase, with collections before files.
• Line 112, let-to-define: Internal definitions are recommended instead of let expressions, to reduce nesting.
• Line 169, define-let-to-double-define: This let expression can be pulled up into a define expression.
• Line 192, let-to-define: Internal definitions are recommended instead of let expressions, to reduce nesting.
• Line 239, let-to-define: Internal definitions are recommended instead of let expressions, to reduce nesting.
• Line 386, let-to-define: Internal definitions are recommended instead of let expressions, to reduce nesting.
• Line 434, let-to-define: Internal definitions are recommended instead of let expressions, to reduce nesting.
• Line 532, for-each-to-for: This for-each operation can be replaced with a for loop.
• Line 542, let-to-define: Internal definitions are recommended instead of let expressions, to reduce nesting.
• Line 552, let-to-define: Internal definitions are recommended instead of let expressions, to reduce nesting.
• Line 577, let-to-define: Internal definitions are recommended instead of let expressions, to reduce nesting.
• Line 592, let-to-define: Internal definitions are recommended instead of let expressions, to reduce nesting.

Applied 1 fix to drracket/drracket/private/module-browser.rkt

• Line 3, tidy-require: Keep imports in require sorted and grouped by phase, with collections before files.

Pass 2

Applied 2 fixes to drracket/drracket/private/tools.rkt

• Line 131, cond-let-to-cond-define: Internal definitions are recommended instead of let expressions, to reduce nesting.
• Line 445, map-to-for: This map operation can be replaced with a for/list loop.

Summary

Fixed 16 issues in 3 files.

• Fixed 9 occurrences of let-to-define
• Fixed 3 occurrences of tidy-require
• Fixed 1 occurrence of define-let-to-double-define
• Fixed 1 occurrence of for-each-to-for
• Fixed 1 occurrence of cond-let-to-cond-define
• Fixed 1 occurrence of map-to-for

[rfindler]

Looks like (make-object bitmap-dc%) should just go away.

[jackfirth]

Agreed. Not sure how to detect this statically though, since in general constructors can have side effects (and probably occasionally do in practice).

[rfindler]

This should be different; possibly not in the purview of these changes?

[jackfirth]

Hmm. I don't know of a good way to make this code much better, since sort needs to transform the whole list. That sort expression can be made a little simpler though since instead of passing that lambda it could pass (sort ... string<? #:key car). I already have a rule for that - I'm assuming it didn't fire here because the change hit the fix limit.

[jackfirth]

As an aside, that code is much easier to write with transducers:

(transduce installed-tools
           (indexing tool-list-entry)
           (sorting string<=> #:key entry-key)
           #:into
           (into-for-each
             (lambda (entry+it)
               (send listing append (car entry+it) (cdr entry+it)))))

[rfindler]

Oh, I just meant the simple change using sort's #:key argument. Sorry for being so opaque! Glad to hear that it'll get fixed on its own eventually! :)

The transducer code doesn't look much easier to me, but probably I'm missing something?

[jackfirth]

Oh gotcha, yeah there's a rule for that sort fix. I thought you were talking about how indented the whole sort expression got because of it being on the right-hand-side of a for clause.

For the transducer code, I ought to have been clearer. The code I gave is equivalent to the whole for-each expression and for loop. These three snippets are all equivalent:

(for-each
 (lambda (entry+it)
   (send listing append
         (car entry+it)
         (cdr entry+it)))
 (sort (map (lambda (it) (cons (tool-list-entry it) it))
            installed-tools)
       (lambda (a b)
         (string<? (car a) (car b)))))

(for ([entry+it (in-list (sort (map (lambda (it) (cons (tool-list-entry it) it))
                                    installed-tools)
                               (lambda (a b) (string<? (car a) (car b)))))])
  (send listing append (car entry+it) (cdr entry+it)))

(transduce installed-tools
           (indexing tool-list-entry)
           (sorting string<=> #:key entry-key)
           #:into
           (into-for-each
            (lambda (entry+it)
              (send listing append (car entry+it) (cdr entry+it)))))

It's pretty subjective (which I did not make clear in my previous comment) but I find the transducer version much easier to read assuming you know how to use transduce, since it's just a top-to-bottom series of steps applied to the initial list. Whenever I need to sort data and also do some transformation before or after, I usually use transducers because it's pretty hard to work sort into a for loop nicely. Plus sort is frustrating to use on non-lists.

[rfindler]

Overall, looks good to me. Maybe I should make the changes I've suggested after this is merged? How does one best handle that sort of thing, where there are followup fixes that get noticed just by reading the diffs?

[jackfirth]

@rfindler Making them after merging is totally reasonable. Alternatively, if you want to make them before approval you can check out this branch and push them immediately.

[rfindler]

I tried to push and it seemed to do something, but not quite right. How do you do this kind of thing?

$  git checkout pr/696
M	drracket-test/tests/drracket/module-lang-test.rkt
branch 'pr/696' set up to track 'origin/pr/696'.
Switched to a new branch 'pr/696'

# ... removed the bitmap and committed the change

$  git status
On branch pr/696
Your branch is ahead of 'origin/pr/696' by 1 commit.
  (use "git push" to publish your local commits)

$  git push
Enumerating objects: 11, done.
Counting objects: 100% (11/11), done.
Delta compression using up to 8 threads
Compressing objects: 100% (6/6), done.
Writing objects: 100% (6/6), 491 bytes | 491.00 KiB/s, done.
Total 6 (delta 5), reused 0 (delta 0), pack-reused 0
remote: Resolving deltas: 100% (5/5), completed with 5 local objects.
remote: 
remote: Create a pull request for 'pr/696' on GitHub by visiting:
remote:      https://github.com/racket/drracket/pull/new/pr/696
remote: 
To https://github.com/racket/drracket.git
 * [new branch]        pr/696 -> pr/696

[jackfirth]

I think you have to push to the autofix-67-1 branch this PR is based on, not the... origin/pr/696 branch? I didn't realize pull requests had their own special branches like that. Odd.