On Thu, 20 Dec 2018 10:27:32 +0100, Boris FELD wrote: > On 17/12/2018 14:36, Yuya Nishihara wrote: > > On Mon, 17 Dec 2018 12:38:17 +0000, Boris FELD wrote: > >> It is possible to passvarious substitution when passing revset to > >> `repo.revs` (eg: repo.revs("heads(%ld)", [4, 5, 6]). > >> > >> The current implementation handle this in a quick and dirty way. First a > >> new revset string is build: > >> > >> > >> * code: repo.revs("heads(%ld)", [4, 5, 6]) > >> * transformed into (repo.revs("heads(_intlist(4, 5, 6))") > >> * then revset parser build a syntax tree > >> * _intlist turns its argument into a baseset > >> > >> This is clever and simple, it works well for a small number of revision. > >> However, when the set of revision growth, this becomes ineffective. For > >> example, discovery might call revset where %ld is replace by hundred of > >> thousands of revisions. The cost of serializing and serializing them > >> to/from string become significant. > > To avoid formatting/parsing overheads, we could directly build a parse tree > > and feed it to revset.makematcher(). It will be somewhat similar to alias > > expansion. > > The current API is very easy to use and I would rather keep it that way. > Having developer choose between API simplicity and speed seems sub-optimal. > > What does your proposal look like in practice API wise?
repo.revs("heads(%ld)", [4, 5, 6]) rewrite "heads(%ld)" as e.g. "heads($0)" so that it can be parsed as alias tree = parse("heads($0)") replace the placeholder arguments with e.g. ('intlist', 4, 5, 6) analyze, optimize, etc. the tree return revset.makematcher(tree) > > Unlike templater, revset has no context variable. So I had to hack the repo > > instance in the example above, which isn't nice for your use case. > Could we have this done "automatically" with repo.revs? where %ld turned > into special local revsets? If we want to support the % syntax, it's probably better to embed the replacements in the parsed tree. The whole point of using an internal function is that we don't have to touch the parsing stages. > > This wouldn't be simple if we want to implement it cleanly. Maybe we can > > embed a smartset in a parse tree, as say (smartset <smartset>) node, but > > a smartset is mutable and it can be sorted/reordered while evaluating a > > revset. That's probably okay in many cases, but can lead to surprising > > results. > Ha, good point, maybe we need some sort of light smartset copy. Or making smartsets truly immutable by replacing e.g. sort() with sorted(). _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel