[Nix-dev] Re: [Nix-commits] SVN commit: nix - 16412 - eelco - nixpkgs/trunk/pkgs/lib

[Nicolas Pierron]

On Fri, Jul 17, 2009 at 14:04, Eelco Dolstra<e.dolstra at tudelft.nl> wrote:
> Modified: nixpkgs/trunk/pkgs/lib/types.nix
> ===================================================================
> +    # !!! this should be a type constructor that takes the options as
> +    # an argument.
>     optionSet = mkOptionType {
>       name = "option set";
>       check = x: lib.traceValIfNot builtins.isAttrs x;

If you set it as an argument of the type then you have to merge types
before obtaining the list of option sets.  One other argument against
that is that you will have to repeat the type of the option each time
you want to extend it with another declaration.  Look at
mergeOptionDecls and addOptionMakeUp and deduce the extra complexity
to handle this.  By the way, just imagine what have been done with the
fileSystem attribute:

// in modules/tasks/filesystems.nix
fileSystems = mkOption {
  type = types.nullOr (types.list (types.optionSet {
     mountpoint = ..; label = ..; ..
  }));
}

// in modules/system/boot/stage-1.nix
fileSystems = mkOption {
  type = types.nullOr (types.list (types.optionSet {
     neededForBoot = ..;
  }));
}


instead of

// in modules/tasks/filesystems.nix
fileSystems = mkOption {
  types = types.nullOr (types.list types.optionSet);
  options = {
     mountpoint = ..; label = ..; ..
  };
}

// in modules/system/boot/stage-1.nix
fileSystems = mkOption {
  options = {
     neededForBoot = ..;
  };
}


I don't like the idea of adding "mkOption" and "options" either.  I
would prefer something like:

// in modules/system/boot/stage-1.nix
fileSystems.neededForBoot = ..;


-- 
Nicolas Pierron
http://www.linkedin.com/in/nicolasbpierron
A borderless war is along the way! Do you copy?