[PATCH 4 of 8] opener: add read & write utility methods

Matt Mackall mpm at selenic.com
Mon Mar 7 11:37:21 CST 2011


On Mon, 2011-03-07 at 17:51 +0100, Adrian Buehlmann wrote:
> On 2010-12-27 19:49, Matt Mackall wrote:
> > On Mon, 2010-12-27 at 17:02 +0100, Dan Villiom Podlaski Christiansen
> > wrote:
> >> On 27 Dec 2010, at 16:05, Adrian Buehlmann wrote:
> >>
> >>> On 2010-12-26 12:51, Dan Villiom Podlaski Christiansen wrote:
> >>>> A similar method for reading is added to the opener used by static
> >>>> HTTP repositories. It doesn't do any actual closing, as the opener
> >>>> doesn't have access to any underlying files.
> >>>>
> >>>> diff --git a/mercurial/statichttprepo.py b/mercurial/ 
> >>>> statichttprepo.py
> >>>> --- a/mercurial/statichttprepo.py
> >>>> +++ b/mercurial/statichttprepo.py
> >>>> @@ -75,6 +75,8 @@ def build_opener(ui, authinfo):
> >>>>                 raise IOError('Permission denied')
> >>>>             f = "/".join((p, urllib.quote(path)))
> >>>>             return httprangereader(f, urlopener)
> >>>> +        # for compatibility with regular file openers
> >>>> +        o.read = lambda *args, **kwargs: o(*args, **kwargs).read()
> >>>>         return o
> >>>>
> >>>>     opener.options = {'nonlazy': 1}
> >>>
> >>> is needed, then I'm fairly convinced that having to add read and write
> >>> member functions to every kind of opener is fishy.
> >>
> >> I've identified four kinds of opener in the Mercurial codebase:
> >>
> >> - regular openers that use the class
> >> - store openers for preprocessing the filename with a function
> >> - static HTTP openers — read-only
> >> - urllib openers
> > 
> > Seems to me that we should be able to clean up the opener code so that
> > this becomes a simple matter of inheritance, especially given how
> > trivial these functions should be.
> > 
> > Our choices appear to be between something like:
> > 
> > self.opener.read(file)
> > 
> > and
> > 
> > util.readopener(self.opener, file)
> > 
> > 
> > It seems we have a lot more users of the opener read/write pattern than
> > openers so this looks like a net win over independent utility functions.
> > 
> 
> Can you shed some light on in what way you would like to see the opener
> code "cleaned up"?

Right now "opener" is a duck type. There are lots of different types
with different functionality that get passed to things expecting an
"opener". It's a bit of a mess.

Also the util:opener code itself is long and scary. Make it less long
and scary.

> What was the idea here? Derive util.opener from something?

A base class might be helpful, but I don't think it's essential. But if
we're going to add read() and write() helper methods, putting them in a
base class seem the obvious way.

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list