[PATCH 02 of 10] opener: add read & write utility methods
Dan Villiom Podlaski Christiansen
danchr at gmail.com
Fri Dec 3 09:06:34 CST 2010
On 2 Dec 2010, at 00:42, Adrian Buehlmann wrote:
> On 2010-12-01 22:34, Dan Villiom Podlaski Christiansen wrote:
>> # HG changeset patch
>> # User Dan Villiom Podlaski Christiansen <danchr at gmail.com>
>> # Date 1291227679 -3600
>> # Node ID 8c47f61bd78b2bdcbd088e0b2fed3bedc4e995eb
>> # Parent 446c177fbf75f7134eec7671998fd112fbe90bed
>> opener: add read & write utility methods
>>
>> The two new methods are useful for quickly opening a file for reading
>> or writing. Unlike 'opener(...).read()', they ensure they the file is
> ----
>
>> immediately closed without relying on CPython reference counting.
>>
>> Similar methods are added to custom openers.
>>
>> diff --git a/mercurial/statichttprepo.py b/mercurial/
>> statichttprepo.py
>> --- a/mercurial/statichttprepo.py
>> +++ b/mercurial/statichttprepo.py
>> @@ -75,6 +75,7 @@ def build_opener(ui, authinfo):
>> raise IOError('Permission denied')
>> f = "/".join((p, urllib.quote(path)))
>> return httprangereader(f, urlopener)
>> + o.read = lambda *args, **kwargs: o(*args, **kwargs).read()
>> return o
>>
>> opener.options = {'nonlazy': 1}
>> diff --git a/mercurial/store.py b/mercurial/store.py
>> --- a/mercurial/store.py
>> +++ b/mercurial/store.py
>> @@ -176,6 +176,8 @@ class basicstore(object):
>> op = opener(self.path)
>> op.createmode = self.createmode
>> self.opener = lambda f, *args, **kw: op(encodedir(f),
>> *args, **kw)
>> + self.opener.read = (lambda f, *args, **kw:
>> + op(encodedir(f), *args, **kw).read())
>>
>> def join(self, f):
>> return self.pathjoiner(self.path, encodedir(f))
>> @@ -221,6 +223,8 @@ class encodedstore(basicstore):
>> op = opener(self.path)
>> op.createmode = self.createmode
>> self.opener = lambda f, *args, **kw: op(encodefilename(f),
>> *args, **kw)
>> + self.opener.read = (lambda f, *args, **kw:
>> + op(encodefilename(f), *args, **kw).read
>> ())
>
> IMHO, this is too intrusive. You shouldn't have to add a read member
> function to
> every opener.
>
> Consider creating free functions util.read and write instead, taking
> an opener
> as a parameter.
My main concern was writing the code in a maintainable fashion.
Putting read() and write() methods on the opener seemed the simplest
to me. As the store doesn't use the shared opener class, I had to fake
it. An alternate approach would be to create a ‘filteropener’ class
for the stores.
> in util.py:
>
> def read(op, path, text=False):
> fp = op(path, 'r', text)
> try:
> return fp.read()
> finally:
> fp.close()
>
> def write(data, op, path, mode='w', text=False, atomictemp=False):
> fp = op(path, mode, text, atomictemp)
> try:
> fp.write(data)
> if atomictemp:
> fp.rename()
> finally:
> fp.close()
…
> and then use it as (example):
>
>> diff --git a/hgext/mq.py b/hgext/mq.py
>> --- a/hgext/mq.py
>> +++ b/hgext/mq.py
>> @@ -272,14 +272,14 @@ class queue(object):
>> def parse(l):
>> n, name = l.split(':', 1)
>> return statusentry(bin(n), name)
>> - lines = self.opener(self.status_path).read().splitlines
>> ()
>> + lines = self.opener.read(self.status_path).splitlines()
>
> lines = util.read(self.opener,
> self.status_path).splitlines()
>
>> return [parse(l) for l in lines]
>> return []
I don't really see the point to this. I find it logical that
operations on openers should be part of the opener class. A util.read
() function might be useful for cases where an opener isn't used; I'll
look into adding that.
> and you probably need:
>
>
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -822,7 +822,7 @@ class atomictempfile(object):
> self._fp.close()
> rename(self.temp, localpath(self.__name))
>
> - def __del__(self):
> + def close(self):
> if not self._fp:
> return
> if not self._fp.closed:
> @@ -831,6 +831,9 @@ class atomictempfile(object):
> except: pass
> self._fp.close()
>
> + def __del__(self):
> + self.close()
> +
> def makedirs(name, mode=None):
> """recursive directory creation with parent mode inheritance"""
> try:
I actually had code similar to this in my patch queue, but deleted it
as it didn't seem to be used. I've restored it for consistency.
--
Dan Villiom Podlaski Christiansen
danchr at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 1943 bytes
Desc: not available
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20101203/69c047db/attachment.bin>
More information about the Mercurial-devel
mailing list