On Thursday 23 July 2009 08:04:11 Jürgen Spitzmüller wrote:
> Vincent van Ravesteijn wrote:
> > I would especially like to ask whether someone is willing to check the
> > lyx2lyx part ?
>
> Everything untested, and I'm not really a Pythonist either. Just some
> thoughts on how I would have done it.
Using the same remark as Jürgen.
> > Vincent
> > author_ids2.patch
> >
> > Index: lib/lyx2lyx/lyx_2_0.py
> > ===================================================================
> > --- lib/lyx2lyx/lyx_2_0.py (revision 30748)
> > +++ lib/lyx2lyx/lyx_2_0.py (working copy)
> > @@ -947,7 +947,65 @@
> > document.body[i:i+3] = subst
> > i = i + 2
> >
> > +def convert_author_id(document):
> > + " Add the author_id to the \\author definition and make sure 0 is
> > not used" + i = 0
> > + j = 1
> > + while True:
> > + i = find_token(document.header, "\\author", i)
> > + if i == -1:
> > + break
> > + document.header[i] = document.header[i][:8] \
> > + + str(j) + document.header[i][7:]
> > + j = j + 1
> > + i = i + 1
>
> + i = find_token(document.header, "\\author", i)
> + if i == -1:
> + break
> + name = get_value(document.header, '\\author', i)
> + document.header[i] = "\\author " + str(j) + ' ' + name
> + j = j + 1
> + i = i + 1
I agree with Jürgen here, no need to be clever. I had to count the number of
characters in "\\author" to understand what document.header[i][:8] meant. FWIW
I am guilty of the same mistake earlier so you are in good company here. :-)
The other advantage is that by explicitly naming the remaining part as "name"
you making the code easier to read. I like it because of this. Since the
assignment work as a reference here there is no copy involved so there is no
associated cost in terms of performance.
Another point, regarding
document.header[i] = "\\author " + str(j) + ' ' + name
for me it is easier to read this as
document.header[i] = "\\author %i %s" % (j, name)
This syntax is reminiscent of the C printf where %i stands for an integer and
%s for a string.
Again this is a personal style, the line above is legal and does what you
intend to.
> > + k = 0
> > + while True:
> > + k = find_token(document.body, "\\change_", k)
> > +
> > + if k == -1:
> > + break
> > + l = document.body[k].find(" ")
> > + m = document.body[k].rfind(" ")
> > +
> > + if (l > -1) and (m > -1) and (l != m):
> > + val = string.atoi(document.body[k][l:m])
> > + document.body[k] = document.body[k][:l] \
> > + + ' ' + str(val + 1) \
> > + + document.body[k][m:]
> > + k = k + 1
>
> + k = find_token(document.body, "\\change_", k)
> +
> + if k == -1:
> + break
> + change = document.body[k].split(' ')
> + if len(change) == 3:
> + val = string.atoi(change[1])
> + document.body[k] = change[0] \
> + + ' ' + str(val + 1) \
> + + ' ' + change[2]
> + k = k + 1
One note here, there is no need to use string.atoi here int() is enough
val = int(change[1])
We use string.atoi because historically that was not available in
python 1.5
the first required version. This is clearly an accident. :-)
Again I would prefer
document.body[k] = "%s %i %s" % (change[0], val + 1, change[2])
>
> Jürgen
--
José Abílio
