Across my career, I’ve enjoyed doing tree-wide refactors that move the state of the world further just a little bit. With top-level with statements, there’s an opportunity to do that in nixpkgs , so I’ve assigned myself the following issue.
Awesome, this would be a huge improvement in ease of readability! I vividly remember being thoroughly confused where names come from, and that made Nixpkgs feel like way too much magic.
My commitment is to produce patches which are pure refactors, with no behavior changes. What will kill this effort is nit-picking to try to get to the smallest possible set of changes in each file, or “one-more-thinging”.
In particular, I’m going to unify lib.something into just something if there’s an inherit (lib) something up top, as this is both more performant (which usually doesn’t matter) and is more clear that this is the same symbol (which usually does matter for readability.)
My call to action for you (and any other Nixpkgs committers) is that I could use reviews and merges on the flurry of PRs to come.
The script is neat, but do make sure that scope checking errors aren’t masked by a different with, which could be anywhere in the file, disabling scope checking in its particular subtree.
Checking that by hand (using find in $EDITOR) might be the most effective way to do that.
masked by a different with, which could be anywhere in the file, disabling scope checking in its particular subtree.
Don’t I know it. My workflow relies on having vim open with (at least) two panes, one of which is running refactor-top-level-with-away.sh, and the other one has the file being edited open.
I try to do the following, and any advice you have on gotchas is welcome.
Read the file for other with statements. As you mention, that’s a key way to accidentally introduce a semantic change. Thankfully I find there’s not too much shadowing of the lib names, and lib is far and away the top contributor to top-level with. I also look for rec uses, though I’m less clear on what bad looks like there.
Look through the list of inherit statements for false positives. I tried to make the script susceptible to false positives and immune to false negatives. Since the script thinks that every word in a string is a possible identifier, it’ll bring those into scope. Usually this is something like and, all, any, or, modules, optional or the like. I haven’t found a place yet in nixpkgs where this extra symbol does damage: it’s just … extra. But let’s have it gone if we see it.
