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

[Marc Weber]

On Sat, Mar 07, 2009 at 03:20:23PM +0100, Eelco Dolstra wrote:
> Some style comments (I want to keep the modular lib functions clean and
> maintainable):
Thanks. Concluseion: misc is not considered beeing a "module" yet :))

> - The comment above "traceWhatis" doesn't actually explain what it does.
> 
> - It should be "whatIs" and "traceWhatIs".  But I prefer something like
> "showType" and "traceType".
showType could be slightly missleading because it dosen't just print the
type but also some information about its contents. Example:

  let val = "foobar"
  in traceWhatis
prints

"is a string" but "is a string, starting characters: foobar"

So what about these renamings?

  debugVal -> traceVal
  debugXMLVal -> traceXMLVal

  whatis -> showVal
  traceWhatis -> traceShowVal
  traceWhatisMarked -> traceShowValMarked
 
> - Please don't use the __XXX builtins, they were never really intended to be
> used directly.  Either say builtins.trace, or use a "with" or "inherit" to add
> them to the scope.  See for instance the use of head/tail in lists.nix.
I like them :-) They are short and convinient. You immediately know
which function you're using without having to look at imports.
I'll fix this.
 
> > +  traceMarked = str: x: __trace (str + (whatis x)) x;
> > +  attrNamesToStr = a : lib.concatStringsSep "; " (map (x : "${x}=") (__attrNames a));
> > +  whatis = x :
> > +      if (__isAttrs x) then
> There are a lot of redundant parentheses in your code, which makes it look too
> much like Lisp :-)  The parentheses around an if condition are unnecessary.
> Likewise, "str + (whatis x)" can just be "str + whatis x".
I'll fix this.

Marc Weber