Re: Fix error messages in website build (issue4428077)

[Phil Holmes]

----- Original Message ----- 
From: "Graham Percival" <address@hidden>
To: "Phil Holmes" <address@hidden>
Cc: <address@hidden>; <address@hidden>; 
<address@hidden>; <address@hidden>
Sent: Monday, May 02, 2011 6:28 PM
Subject: Re: Fix error messages in website build (issue4428077)


> On Mon, May 02, 2011 at 05:29:27PM +0100, Phil Holmes wrote:
> > >http://codereview.appspot.com/4428077/diff/1/scripts/build/extract_texi_filenames.py#newcode65
> > >scripts/build/extract_texi_filenames.py:65: known_missing_files = ''
> > >could this be a list instead of a string?
> >
> > I thought about whether a string was the _best_ solution, but
> > decided that using a read() and find() was simplest and probably
> > quickest.
>
> python has .readlines() for a file, and generally it's
> "pythonesque" to use lists.

You'll be trying to convert me to Lisp next :-)

> > >http://codereview.appspot.com/4428077/diff/1/scripts/build/extract_texi_filenames.py#newcode87
> > >scripts/build/extract_texi_filenames.py:87: known_missing_files_file = a
> > >known_missing_files.append(a)
> >
> > Again, I'm assuming that this is changing from a simple string for
> > the filename to a list?
>
> Yes.  wait... no -- we're still use a string for the filename, but
> using a list of strings for the names of files.
>
>
> um, let me get back to you on this.  I've just confused myself.

No worries.  You'd put that comment in the place where we extract the 
missing files list filename from the options.  If we agree that there's only 
one list, I'll ignore the placement of the comment.

> >  Strikes me we don't want lots of lists of
> > missing files we can pass to the script - that'll just get confusing
> > - I'd suggest just using a single file for any call.
>
> It's not like that -- we still pass in a single filename.  But the
> contents of that file are then read into a list variable, rather
> than leaving them in a string variable.

OK.

> > >http://codereview.appspot.com/4428077/diff/1/scripts/build/extract_texi_filenames.py#newcode94
> > >scripts/build/extract_texi_filenames.py:94: missing_files = open
> > >(known_missing_files_file, 'r')
> > >missing_Files = open(...).readlines()
> > >
> > >then you don't need the next two lines.
> >
> > So with:
> >
> > string = open("filename").read()
> >
> > You don't need the close() ?
>
> That is not correct.  With .readlines(), you don't need the
> close().  (at least, that's what I was told, but I don't see it in
> the docs.  hmm, maybe I should change all my files!)
>
> oh wait, maybe it was a special case where you don't assign the
> file to a variable?

My bet is that you don't really need file.close at all, since it's closed 
when the script exits (probably - although C# doesn't do this).  I think 
it's best always to close explicitly.

--
Phil Holmes