[PATCH RFC v3] scmutil: add a simple key-value file helper

Jun Wu quark at fb.com
Tue Dec 6 08:28:04 EST 2016


Excerpts from Kostia Balytskyi's message of 2016-12-06 13:22:40 +0000:
> On 12/6/16 1:04 PM, Yuya Nishihara wrote:
> > On Mon, 5 Dec 2016 15:03:35 -0800, Kostia Balytskyi wrote:
> >> # HG changeset patch
> >> # User Kostia Balytskyi <ikostia at fb.com>
> >> # Date 1480978778 28800
> >> #      Mon Dec 05 14:59:38 2016 -0800
> >> # Node ID bdc49209b0b926214e865f9a98ea987866f64d68
> >> # Parent  243ecbd4f5c9f452275d4435866359cf84dc03ff
> >> scmutil: add a simple key-value file helper
> >>
> >> The purpose of the added class is to serve purposes like save files of shelve
> >> or state files of shelve, rebase and histedit. Keys of these files can be
> >> alphanumeric and start with letters, while values must not contain newlines.
> >> Keys which start with an uppercase letter are required, while other keys
> >> are optional.
> >> +class simplekeyvaluefile(object):
> >> +    """A simple file with key=value lines
> >> +
> >> +    Keys must be alphanumerics and start with a letter, values might not
> >> +    contain '\n' characters
> >> +    """
> >> +    class InvalidKeyValueFile(Exception): pass
> >> +    class InvalidKeyInFileException(Exception): pass
> >> +    class InvalidValueInFileException(Exception): pass
> >> +    class MissingRequiredKeyInFileException(Exception): pass
> >> +
> >> +    # if KEYS is non-empty, read values are validated against it:
> >> +    # each key is a tuple (keyname, required)
> >> +    KEYS = []
> > If we want to define a required key back to older Mercurial versions (like
> > mergestate), we'll need a static rule such as "uppercase is required" seen in
> > your V1 patch.
> I am not sure I understand this one. If you meant that 
> simplekeyvaluefile should be able
> to read state files created by older mercurail versions, I don't think 
> it's possible. I don't think
> simplekeyvaluefile itself should provide a backwards-compatible layer. 
> Rather, when we
> migrate a state file, we should first try to parse it into a 
> simlpekeyvaluefile and if this fails,
> fallback to a traditional file format. New fields should only be added 
> to a simplekeyvaluefile
> code path, while traditional parsing should replace them with some sort 
> of empty/default values.
> 
> Can you clarify what you meant by "define a required key back to older 
> Mercurial versions"?

It means, for example, in hg v5.0, we have keys "a" and "b" as "required".
And we add another key "c" as required in v5.1. What will v5.1 do with the
simplekeyvaluefile written by v5.0 ? (and what will we do if we want to
remove required keys?)

> >
> >> +    def __init__(self, vfs, path):
> >> +        self.vfs = vfs
> >> +        self.path = path
> >> +
> >> +    def validate(self, d):
> >> +        for key, req in self.KEYS:
> >> +            if req and key not in d:
> >> +                e = _("missing a required key: '%s'") % key
> >> +                raise self.MissingRequiredKeyInFileException(e)
> > _() is necessary only for user-facing errors.
> >
> > I don't think nested classes are useful unless we want to give a room for
> > sub classes to override them.
> >
> >> +            lines.append("%s=%s\n" % (k, v))
> >> +        with self.vfs(self.path, mode='wb', atomictemp=True) as fp:
> >> +            fp.write(''.join(lines))
> > BTW, the syntax of this file seems somewhat similar to config.config.
> I wanted something explicitly much simpler than config.config (also with 
> possibility to further limit
> what can be values if needed).
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel 
> 


More information about the Mercurial-devel mailing list