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

programs.zsh.syntax-highlighting: refactor highlighters option for proper validation #25210

Merged
merged 1 commit into from Apr 27, 2017
Merged

programs.zsh.syntax-highlighting: refactor highlighters option for proper validation #25210

merged 1 commit into from Apr 27, 2017

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Apr 25, 2017

Motivation for this change

Today after updating my nix-channel (which included the changes from #25153) I updated my personal NixOS setup to configure the zsh-syntax-highlighting properly, but I mistyped one of the highlighters which caused my shell to do odd things.

At first I feared that I broke NixOS, but in the end I've seen that it was just a stupid typo. However this showed me that it might be a good idea to replace the types.list(types.str) declaration of programs.zsh.syntax-highlighting.highlighters with a types.list(types.enum([/* available highlighters */])) to ensure stricter validation.

Although the validation is restricted here this is IMHO not a BC break due to the fact that everything will continue to work if no invalid highlighters were given (but if that happened the shell would be almost unusable).

I validated the patch by using two configurations that were deployed in a test VM:

# aborts with an evaluation failure because of the invalid highlighter
{
  programs.zsh = {
    enable = true;
    syntax-highlighting = {
      enable = true;
      highlighters = [ "main" "foobar" ];
    };
  };
}
# still works
{
  programs.zsh = {
    enable = true;
    syntax-highlighting = {
      enable = true;
      highlighters = [ "main" "brackets" ];
    };
  };
}

I booted the VMs using the following command:

NIXOS_CONFIG=`pwd`/vmtest.nix nixos-rebuild -I nixpkgs=$HOME/Projects/nixpkgs/ build-vm
Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

…proper validation

Right now the `programs.zsh.syntax-highlighting.highlighters` option
lacks appropriate validation which can cause confusing things when
mistyping a higlighter for zsh-syntax-highlighting.
@fpletz
Copy link
Member

fpletz commented Apr 27, 2017

Thanks, looks useful!

@fpletz fpletz merged commit dab5f92 into NixOS:master Apr 27, 2017
@Ma27 Ma27 deleted the zsh/refactor-syntax-highlighting branch April 27, 2017 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants