On Thu, May 19, 2011 at 06:44:36PM +0100, Michael Hanselmann wrote:
> Am 19. Mai 2011 17:56 schrieb Iustin Pop <[email protected]>:
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > +htools/Ganeti/Constants.hs: htools/Ganeti/Constants.hs.in \
> > +       lib/constants.py lib/_autoconf.py $(CONVERT_CONSTANTS)
> > +       set -e; \
> > +       cat $< > $@; \
> > +       $(CONVERT_CONSTANTS) >> $@
> 
> Can you change this to “{ cat $<; $(CONVERT…); } > $@” to match the
> generation of .rst files?

Yep.

> > +
> > +
> >  lib/_autoconf.py: Makefile vcs-version | lib/.dir
> 
> One empty line only, please.

Ack.

> > --- /dev/null
> > +++ b/autotools/convert-constants
> > @@ -0,0 +1,78 @@
> > +import re
> > +
> > +from ganeti import constants
> > +
> > +CONSTANT_RE = re.compile("^[A-Z][A-Z0-9_]+$")
> > +
> > +def nameRules(name):
> 
> Please change the names to match Ganeti's Python style (“NameRules”),
> here and elsewhere.

Uh, will do.

> > +  """Converts the upper-cased Python name to Haskell camelCase.
> > +
> > +  """
> > +  elems = name.split("_")
> > +  return elems[0].lower() + "".join(e.capitalize() for e in elems[1:])
> > +
> > +def stringValueRules(value):
> 
> Two empty lines at module level. Here and elsewhere.

Ack.

> > +  """Converts a string value from Python to Haskell.
> > +
> > +  """
> > +  value = value.encode("string_escape") # escapes backslashes
> > +  value = value.replace("\"", "\\\"")
> > +  return value
> > +
> > +def convert():
> > +  """Converts the constants to Haskell.
> > +
> > +  """
> > +  lines = [""]
> > +
> > +  all_names = dir(constants)
> > +
> > +  for name in all_names:
> > +    value = constants.__getattribute__(name)
> 
> Just wondering, why __getattribute__ instead of getattr()?

Because I was more focused on making the generated code compile :)

Will fix.

> > +    hs_name = nameRules(name)
> > +    if not CONSTANT_RE.match(name):
> > +      lines.append("-- Skipped %s, not constant" % name)
> > +    elif isinstance(value, basestring):
> > +      lines.append("%s :: String" % hs_name)
> > +      lines.append("%s = \"%s\"" % (hs_name, stringValueRules(value)))
> > +    elif isinstance(value, int):
> 
> (int, long)?

Not quite, but good suggestion. Long is the equivalent of Integer
(variable-length), so I'll add a separate clause for that.

thanks,
iustin

Reply via email to