[libvirt] [PATCH 0/3] apibuild improvements

While working with the documentation, I found some issues I'd like to improve. Philipp Hahn (3): Align table cells on top Improve tokenizing of linkable terms Fix references to self.warning() docs/apibuild.py | 48 ++++++++++++++++++++++++++++++------------------ docs/libvirt.css | 4 ++++ docs/newapi.xsl | 9 ++++++--- 3 files changed, 40 insertions(+), 21 deletions(-)

When the description of an entry is too long and needs multiple lines, all other table cells of the same row are currently vertically aligned on center. Without row borders or different background colors for alternating rows this is hard to read. Change the style-sheet to align the table cells of a row on top. Signed-off-by: Philipp Hahn <hahn@univention.de> --- docs/libvirt.css | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) --- See <http://libvirt.org/html/libvirt-libvirt.html#virDomainMemoryStatTags> for example. diff --git a/docs/libvirt.css b/docs/libvirt.css index 049e332..6e54b73 100644 --- a/docs/libvirt.css +++ b/docs/libvirt.css @@ -199,6 +199,10 @@ div.api table { whitespace: pre; } +div.api table td, div.variablelist table td { + vertical-align: top; +} + h1 a, h2 a, h3 a, h4 a, h5 a { color: inherit; -- 1.7.1

On 08/11/2011 06:40 AM, Philipp Hahn wrote:
When the description of an entry is too long and needs multiple lines, all other table cells of the same row are currently vertically aligned on center. Without row borders or different background colors for alternating rows this is hard to read.
Change the style-sheet to align the table cells of a row on top.
Seems reasonable.
Signed-off-by: Philipp Hahn<hahn@univention.de> --- docs/libvirt.css | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
--- See <http://libvirt.org/html/libvirt-libvirt.html#virDomainMemoryStatTags> for example.
That was the clincher for me :) ACK and pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Currently only tabs and blanks are used for tokenizing the description, which breaks when a term is at the end of a line or has () appended to it. 1. Use also other white space characters such as new-lines and carriage return for splitting. 2. Remove some common non-word characters from the token before lookup. Signed-off-by: Philipp Hahn <hahn@univention.de> --- docs/newapi.xsl | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/newapi.xsl b/docs/newapi.xsl index 445a48c..6e1c646 100644 --- a/docs/newapi.xsl +++ b/docs/newapi.xsl @@ -54,10 +54,13 @@ because the keys are only defined on the main document --> <xsl:template mode="dumptoken" match='*'> <xsl:param name="token"/> - <xsl:variable name="ref" select="key('symbols', $token)"/> + <xsl:variable name="stem" select="translate($token, '(),.:;@', '')"/> + <xsl:variable name="ref" select="key('symbols', $stem)"/> <xsl:choose> <xsl:when test="$ref"> - <a href="libvirt-{$ref/@file}.html#{$ref/@name}"><xsl:value-of select="$token"/></a> + <xsl:value-of select="substring-before($token, $stem)"/> + <a href="libvirt-{$ref/@file}.html#{$ref/@name}"><xsl:value-of select="$stem"/></a> + <xsl:value-of select="substring-after($token, $stem)"/> </xsl:when> <xsl:otherwise> <xsl:value-of select="$token"/> @@ -70,7 +73,7 @@ <xsl:param name="text"/> <xsl:variable name="ctxt" select='.'/> <!-- <xsl:value-of select="$text"/> --> - <xsl:for-each select="str:tokenize($text, ' ')"> + <xsl:for-each select="str:tokenize($text, ' ')"> <xsl:apply-templates select="$ctxt" mode='dumptoken'> <xsl:with-param name="token" select="string(.)"/> </xsl:apply-templates> -- 1.7.1

On 08/11/2011 06:44 AM, Philipp Hahn wrote:
Currently only tabs and blanks are used for tokenizing the description, which breaks when a term is at the end of a line or has () appended to it. 1. Use also other white space characters such as new-lines and carriage return for splitting. 2. Remove some common non-word characters from the token before lookup.
Signed-off-by: Philipp Hahn<hahn@univention.de> --- docs/newapi.xsl | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)
I am not fluent in reading xsl files. How would I go about testing if the output is more visually appealing? I can ack on the basis of comparison on before vs. after appearance, but only if I figure out which files are affected to compare a view in my browser of the generated html. My gut feel is that this is probably right, but unless someone more familiar with the file format speaks up, or unless I can test the results, I won't push this just yet. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Am Donnerstag 11 August 2011 21:45:18 schrieb Eric Blake:
On 08/11/2011 06:44 AM, Philipp Hahn wrote:
Currently only tabs and blanks are used for tokenizing the description, which breaks when a term is at the end of a line or has () appended to it. 1. Use also other white space characters such as new-lines and carriage return for splitting. 2. Remove some common non-word characters from the token before lookup.
Signed-off-by: Philipp Hahn<hahn@univention.de> --- docs/newapi.xsl | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)
I am not fluent in reading xsl files. How would I go about testing if the output is more visually appealing? I can ack on the basis of comparison on before vs. after appearance, but only if I figure out which files are affected to compare a view in my browser of the generated html.
Go to <http://libvirt.org/html/libvirt-libvirt.html> at search for "()" or strings starting with "VIR_": You'll notice lots of them, which aren't links, but many others are. This hapens because only space characters were used to separate word, which then looked for "virDomainGetVcpus()" instead of just "virDomainGetVcpus". The other case was the keywords were at the end on line, where the "\n" wasn't used for word breaking, so the search went for "VIR_DOMAIN_MEMORY_HARD_LIMIT\nsomething" instead of just "VIR_DOMAIN_MEMORY_HARD_LIMIT". It's still not perfect, because only "_function_()" is clickable and not "function()", but at least it is. It would have been easier with RegExps <http://www.exslt.org/regexp/> for stemming, but that isn't supported by libxslt. Sincerely Philipp Hahn -- Philipp Hahn Open Source Software Engineer hahn@univention.de Univention GmbH Linux for Your Business fon: +49 421 22 232- 0 Mary-Somerville-Str.1 D-28359 Bremen fax: +49 421 22 232-99 http://www.univention.de/

On Thu, Aug 11, 2011 at 02:44:00PM +0200, Philipp Hahn wrote:
Currently only tabs and blanks are used for tokenizing the description, which breaks when a term is at the end of a line or has () appended to it. 1. Use also other white space characters such as new-lines and carriage return for splitting. 2. Remove some common non-word characters from the token before lookup.
Signed-off-by: Philipp Hahn <hahn@univention.de> --- docs/newapi.xsl | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/docs/newapi.xsl b/docs/newapi.xsl index 445a48c..6e1c646 100644 --- a/docs/newapi.xsl +++ b/docs/newapi.xsl @@ -54,10 +54,13 @@ because the keys are only defined on the main document --> <xsl:template mode="dumptoken" match='*'> <xsl:param name="token"/> - <xsl:variable name="ref" select="key('symbols', $token)"/> + <xsl:variable name="stem" select="translate($token, '(),.:;@', '')"/> + <xsl:variable name="ref" select="key('symbols', $stem)"/> <xsl:choose> <xsl:when test="$ref"> - <a href="libvirt-{$ref/@file}.html#{$ref/@name}"><xsl:value-of select="$token"/></a> + <xsl:value-of select="substring-before($token, $stem)"/> + <a href="libvirt-{$ref/@file}.html#{$ref/@name}"><xsl:value-of select="$stem"/></a> + <xsl:value-of select="substring-after($token, $stem)"/> </xsl:when> <xsl:otherwise> <xsl:value-of select="$token"/> @@ -70,7 +73,7 @@ <xsl:param name="text"/> <xsl:variable name="ctxt" select='.'/> <!-- <xsl:value-of select="$text"/> --> - <xsl:for-each select="str:tokenize($text, ' ')"> + <xsl:for-each select="str:tokenize($text, ' ')"> <xsl:apply-templates select="$ctxt" mode='dumptoken'> <xsl:with-param name="token" select="string(.)"/> </xsl:apply-templates>
ACK, that's good, thanks for digging and fixing this :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 08/11/2011 09:18 PM, Daniel Veillard wrote:
On Thu, Aug 11, 2011 at 02:44:00PM +0200, Philipp Hahn wrote:
Currently only tabs and blanks are used for tokenizing the description, which breaks when a term is at the end of a line or has () appended to it. 1. Use also other white space characters such as new-lines and carriage return for splitting. 2. Remove some common non-word characters from the token before lookup.
ACK, that's good, thanks for digging and fixing this :-)
Pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Only the class CParser declares a method warning(), but the other classes still try to call their self.warning() method, which fails. Move the warning() functions to its own base classe and inherit from that in all other classes. Signed-off-by: Philipp Hahn <hahn@univention.de> --- docs/apibuild.py | 48 ++++++++++++++++++++++++++++++------------------ 1 files changed, 30 insertions(+), 18 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 53b3421..d335ab1 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -173,8 +173,30 @@ class identifier: if conditionals != None: self.set_conditionals(conditionals) -class index: +class base(object): + """Base class providing common methods for printing messages.""" + def __init__(self): + self.no_error = False + + def warning(self, msg): + """Print warning message.""" + global warnings + warnings = warnings + 1 + if self.no_error: + return + print msg + + def stop_error(self): + """Stop printing error messages.""" + self.no_error = True + + def start_error(self): + """Re-start printing error messages.""" + self.no_error = False + +class index(base): def __init__(self, name = "noname"): + base.__init__(self) self.name = name self.identifiers = {} self.functions = {} @@ -366,10 +388,11 @@ class index: self.analyze_dict("typedefs", self.typedefs) self.analyze_dict("macros", self.macros) -class CLexer: +class CLexer(base): """A lexer for the C language, tokenize the input by reading and analyzing it line by line""" def __init__(self, input): + base.__init__(self) self.input = input self.tokens = [] self.line = "" @@ -572,9 +595,10 @@ class CLexer: self.last = tok return tok -class CParser: +class CParser(base): """The C module parser""" def __init__(self, filename, idx = None): + base.__init__(self) self.filename = filename if len(filename) > 2 and filename[-2:] == '.h': self.is_header = 1 @@ -590,19 +614,12 @@ class CParser: self.last_comment = "" self.comment = None self.collect_ref = 0 - self.no_error = 0 self.conditionals = [] self.defines = [] def collect_references(self): self.collect_ref = 1 - def stop_error(self): - self.no_error = 1 - - def start_error(self): - self.no_error = 0 - def lineno(self): return self.lexer.getlineno() @@ -623,12 +640,6 @@ class CParser: self.index.add_ref(name, None, module, static, type, self.lineno(), info, extra, self.conditionals) - def warning(self, msg): - global warnings - warnings = warnings + 1 - if self.no_error: - return - print msg def error(self, msg, token=-1): if self.no_error: @@ -1825,9 +1836,10 @@ class CParser: return self.index -class docBuilder: +class docBuilder(base): """A documentation builder""" def __init__(self, name, path='.', directories=['.'], includes=[]): + base.__init__(self) self.name = name self.path = path self.directories = directories @@ -2363,7 +2375,7 @@ def rebuild(): ["src", "src/util", "include/libvirt"], []) else: - self.warning("rebuild() failed, unable to guess the module") + base().warning("rebuild() failed, unable to guess the module") return None builder.scan() builder.analyze() -- 1.7.1

On 08/11/2011 06:50 AM, Philipp Hahn wrote:
Only the class CParser declares a method warning(), but the other classes still try to call their self.warning() method, which fails.
Move the warning() functions to its own base classe and inherit from
s/classe/class/
that in all other classes.
Signed-off-by: Philipp Hahn<hahn@univention.de> --- docs/apibuild.py | 48 ++++++++++++++++++++++++++++++------------------ 1 files changed, 30 insertions(+), 18 deletions(-)
First I claim ignorance with xslt, now I'm claiming ignorance with Python :) However, this one looks easier to review by comparison to the rest of the file, as well as by compilation smoke-tests, which promptly tripped up 'make syntax-check': error_message_period docs/apibuild.py-190- """Stop printing error messages.""" docs/apibuild.py-194- """Re-start printing error messages.""" maint.mk: found error message ending in period But those don't seem to be error messages, so much as self-documentation strings. I don't know if the python needs to be tweaked, or if the real problem is that cfg.mk should be exempting .py files from that particular syntax check. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel Veillard
-
Eric Blake
-
Philipp Hahn