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