llakala / nixos

My NixOS config
3 stars 1 forks source link

Make PR for mkEnableOption #30

Open llakala opened 1 month ago

llakala commented 1 month ago

Once laptop display is fixed, a PR should be made to nixpkgs that replaces all appropriate instances with mkEnableOption.

Be careful of:

mkPackageOption should also be done across the board, but that may be more complicated.

llakala commented 1 month ago

Update, we have this ripgrep command:

rg -l -U "default = false;\n\s+description = ''\n\s+Whether to enable " $DIRECTORY -g '*.nix'

This isn't perfect, since some instances use a different order of default, type, and description. Ideally we would check that all three of these strings are found within nearby lines (maybe within a range of 3). Should look into this further.

For replacement, we have a few options. For really basic replacements on a single line, sed seemed to do the trick:

sed -i 's/Whether to enable //' file.nix

This can be used with for-looping over the rg files. However, we quickly run into issues with this approach, since sed doesn't really support multi-line replacements. There's a great stackoverflow post on this here . They recommend some sed alternatives. Perl seems really simple, and I want to look into using it for this purpose.

Here's our full workflow, written out:

Or, a little more abstracted:

  1. Select proper files
  2. Modify proper section of files with knowledge of surrounding string context
  3. Make our automated changes
  4. Make manual changes for things that are difficult to automate

2 is really what's worrying me. We'll see if it's feasible. This may also do well as a Draft PR in Nixpkgs, or as a www.discourse.nixos.org post.

llakala commented 1 month ago

Here's a new, MUCH better ripgrep regex:

mkOption\s*\{[^}]*default\s*=\s*false;[^}]*Whether to enable [^}]*\}

Let's walk through what it does, step by step:

mkOption\s*\{[^}]*

This checks for "mkOption {", followed by ANYTHING up until the next ending curly bracket. This means that it'll grab everything until the mkOption ends, which is TREMENDOUSLY helpful. This little [^}]* is used whenever we know that a line will come AFTER, but we don't know WHEN it'll come after.

default\s*=\s*false;[^}]*

This is similar to the previous snippet. It checks for "default = false;", and then goes up until the next ending curcly bracket. Spacing is irrelevant thanks to all the uses of \s*, which allows any amount of spaces, zero inclusive.

Whether to enable [^}]*\}

Final snippet. We check for the string "Whether to enable ", space-sensitively. We need this exact string to appear for mkEnableOption to be a valid replacement. Then, we use our little [^}]* symbol one more time, before finally using a literal ending curly, to wrap it all up!

There's still a lot to do with creating the proper replacements, but this is a huge step.

(Also, key note: ripgrep NEEDS the -U option to work multiline. If it's not included, ripgrep will literally find no results.)

llakala commented 1 month ago

Here's the full ripgrep command right now:

rg -U -g "*.nix" "mkOption\s*\{[^}]*default\s*=\s*false;[^}]*Whether to enable [^}]*\};" -l --stats

-U makes it multiline, and -g "*.nix" only selects nix files. We'll always want these options, so they're at the beginning.

Other options are at the end, since we may want to add or remove them. --stats shows how many files will be changed, but if we're piping output to something else, we can't use it. -l only shows the filenames rather than the selected part, but this should be removed if we want to preview our changes.

llakala commented 1 month ago

Also, a small change has been made to the regex to ensure we don't select ANY }. Now, we only stop once we find a curly bracket followed by a semicolon (ex: };). If these come up inside the multiline string, we're screwed, but i find this hard to imagine.

llakala commented 1 month ago

Currently working on a perl script for replacement, and it's revealed that no regex I have currently is perfect.

Our ripgrep regex has a problem: the description actually CAN come before default = false;. There's at least one instance of this with nixos/modules/services/computing/slurm/slurm.nix. There must be a way to solve this, but I don't have the brain to write it today, so I'll brainstorm about it here.

[^}]* is our strongest soldier. Currently, it's only checking for the next }, though, and not };. If we could make it do that, we could select ALL mkOption's. Then, we could pipe that output into another ripgrep command, that checks for the string "Whether to enable " in the input. Finally, we check for default = false. This completely ignores order, and seems to be the best way to do this.

(I started this comment with no clue of what to do, and ended it with a game plan!)

llakala commented 1 month ago

Seems relevant: https://stackoverflow.com/questions/7124778/how-can-i-match-anything-up-until-this-sequence-of-characters-in-a-regular-exp

llakala commented 1 month ago

I've found the new way to grab an entire mkOption, going until the next ending curly bracket: mkOption[\s\S]*?};

This is pretty simple: after mkOption is found, we check ahead, to see if the next character is in our character set. In this case, every character is in our character set, including newlines, so this will always be true. Then, we check for our "termination condition", aka if we're about to run into };. If we're not, we repeat this. Here's that in pseudocode after we find mkOption:

terminationString = "};"
matchedText = "mkOption" 

textLength = length of text
index = 0

while index < textLength:
    currentCharacter = text[index]

    if text at index matches terminationString: 
        matchedText += terminationString
        return matchedText
    else: 
        matchedText += currentChar
        index += 1

Important thing to remember here, is that we need this to be lazy, to grab multiple mkOptions within the same file. Otherwise, it'll grab from the first mkOption, and stop at the last };. We can accomplish this using ?, so don't remove that.

llakala commented 1 month ago

My current plan is to:

1: Write a list of files we MIGHT modify (ones that contain the string "Whether to enable ").

This is simple, and already done. This also includes the small manual step of removing the actual options.nix file, since it defines mkEnableOption, which we definitely shouldn't modify. We've already done this! I think of this as the ripgrep filter.

2: Have a bash script that passes each of these files, one by one, into a perl script.

Perl seems magic for the regex side, but it'll be much simpler to have the actual file management logic within a bash script.

3: Have the perl script tell the bash script whether a file is ACTUALLY valid.

Just a simple return true/false. From what I've seen, perl makes it super easy to check multiple conditions within a block.

4: Have the bash script make a new list of files that passed the perl filter.

These should be the ones we ACTUALLY plan on modifying.

5: Make the actual modifications to the files that passed the perl filter.

We're not ready to work on this yet, though. We should start by actually getting that list of valid files.

llakala commented 1 month ago

Making lots of progress: we now extract the description, and place it into the correct place within the file to replace the option. Lots of things still need to be done, but first we need to fix a strange bug.

Given this input file:

    programs.bash.vteIntegration = lib.mkOption {
      default = false;
      type = lib.types.bool;
      description = ''
        Whether to enable Bash integration for VTE terminals.
        This allows it to preserve the current directory of the shell
        across terminals.
      '';
    };

    programs.zsh.vteIntegration = lib.mkOption {
      default = false;
      type = lib.types.bool;
      description = ''
        Whether to enable Zsh integration for VTE terminals.
        This allows it to preserve the current directory of the shell
        across terminals.
      '';
    };

For some reason, we get this output for the options:

programs.bash.vteIntegration = lib.mkOption {
      default = false;
      type = lib.types.bool;
      description = ''
        Whether to enable Bash integration for VTE terminals.
        This allows it to preserve the current directory of the shell
        across terminals.
      '';
    };

    programs.zsh.vteIntegration = lib.''
        Whether to enable Zsh integration for VTE terminals.
        This allows it to preserve the current directory of the shell
        across terminals.
      '';

It's only modifying the second one. No clue why this is happening. It modifies the first one fine when it's alone. It also never modifies the first one when there's two, even if we swap the order.