On Thu, 11 May 2017 21:09:35 +0000, Kostia Balytskyi wrote: > > -----Original Message----- > > From: Yuya Nishihara [mailto:you...@gmail.com] On Behalf Of Yuya > > Nishihara > > Sent: Thursday, 11 May, 2017 14:48 > > To: Kostia Balytskyi <ikos...@fb.com> > > Cc: mercurial-devel@mercurial-scm.org > > Subject: Re: [PATCH 4 of 4 shelve-ext v4] shelve: make shelvestate use > > simplekeyvaluefile > > > > On Sun, 7 May 2017 06:07:06 -0700, Kostia Balytskyi wrote: > > > # HG changeset patch > > > # User Kostia Balytskyi <ikos...@fb.com> # Date 1494162279 25200 > > > # Sun May 07 06:04:39 2017 -0700 > > > # Node ID 9f24868156f15473b08a418765411341c96e892b > > > # Parent 497904bddbaa75b9086c168ab2e03381dfaff165 > > > shelve: make shelvestate use simplekeyvaluefile > > > > This looks good to me. > > > > > + @classmethod > > > + def _getversion(cls, repo): > > > + """Read version information from shelvestate file""" > > > + fp = repo.vfs(cls._filename) > > > + try: > > > + version = int(fp.readline().strip()) > > > + except ValueError as err: > > > + raise error.CorruptedState(str(err)) > > > + finally: > > > + fp.close() > > > + return version > > > + > > > + @classmethod > > > + def _readold(cls, repo): > > > + """Read the old position-based version of a shelvestate file""" > > > + # Order is important, because old shelvestate file uses it > > > + # to detemine values of fields (i.g. name is on the second line, > > > + # originalwctx is on the third and so forth). Please do not > > > change. > > > + keys = ['version', 'name', 'originalwctx', 'pendingctx', > > > 'parents', > > > + 'nodestoremove', 'branchtorestore', 'keep', 'activebook'] > > > + # this is executed only seldomly, so it is not a big deal > > > + # that we open this file twice > > > + fp = repo.vfs(cls._filename) > > > + d = {} > > > + try: > > > + for key in keys: > > > + d[key] = fp.readline().strip() > > > + finally: > > > + fp.close() > > > + return d > > > + > > > + @classmethod > > > + def load(cls, repo): > > > + version = cls._getversion(repo) > > > + if version < cls._version: > > > + d = cls._readold(repo) > > > + elif version == cls._version: > > > + d = scmutil.simplekeyvaluefile(repo.vfs, cls._filename)\ > > > + .read(firstlinenonkeyval=True) > > > > This is okay, but I really don't understand why the simplekeyvaluefile has > > to: > > > > - be a class instead of load/dump functions (like json) > > - take a vfs instead of a file object > > > > IMHO, these design decisions just make things complicated. > > I like it being a class because it allows inheritance, which allows > simplekeyvalue users > to add validation. Also, it can serve as a container for firstlinekey. > vfs vs a file object - I don't have an answer to this, maybe file object > would've been better, but I'd prefer to not rewrite it.
Thanks for clarification. I'm not a fan of using inheritance excessively, but yeah that's one way. _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel