comparison mercurial/revsetlang.py @ 41218:24a1f67bb75a

revset: enforce "%d" to be interpreted as literal revision number (API) (BC) Before this change, `formatspec("%d", x)` results in `"%d" % int(x)`. This seems simple and correct until you consider `nullrev`. In revset, a direct "-1" symbol is equivalent to `tip` not `nullrev`. This is a subtle error that went undetected for a while. Wrapping the revision number inside 'rev()' remove the ambiguity, preserving nullrev value passed to formatspec. It got caught by the rebase code, were the following wrongly returned `[1]`: repo.revs("children(%d) and ancestors(%ld)", 0, [nullrev]) This is flagged as API, because `%d` can be used for non-revision integer argument of revset function. We probably need to introduce a new '%?' substitution to allow literal integer (maybe `%i`). However, the `%d` usage is currently widespread for revision number so it is important to fix this issue for `%d`. This choice is reinforced by the fact _intlist is implemented as revisions only. Restricting `%d` to revision only makes things more consistent. This bug can become especially tricky since `_intlist` recognize `nullrev` right. So `revs('%ld', [-1, 0])` ? select `[nullrev, 0]` but `revs('%ld', [-1])` is simplified and treated as `%d` selecting `[tip]`. Another side effect is that "%d" of an unknown revision simply match nothing. It was previously raising and error. This is consistent with what "%ld" (and `_intlist`) is doing, so it seems like a good move.
author Boris Feld <boris.feld@octobus.net>
date Thu, 10 Jan 2019 15:23:58 +0100
parents 4aa04d009167
children e5b227f41e4a
comparison
equal deleted inserted replaced
41217:afa884015e66 41218:24a1f67bb75a
581 """ 581 """
582 return "'%s'" % stringutil.escapestr(pycompat.bytestr(s)) 582 return "'%s'" % stringutil.escapestr(pycompat.bytestr(s))
583 583
584 def _formatargtype(c, arg): 584 def _formatargtype(c, arg):
585 if c == 'd': 585 if c == 'd':
586 return '%d' % int(arg) 586 return 'rev(%d)' % int(arg)
587 elif c == 's': 587 elif c == 's':
588 return _quote(arg) 588 return _quote(arg)
589 elif c == 'r': 589 elif c == 'r':
590 if not isinstance(arg, bytes): 590 if not isinstance(arg, bytes):
591 raise TypeError 591 raise TypeError
636 so that intended expression behavior isn't accidentally subverted. 636 so that intended expression behavior isn't accidentally subverted.
637 637
638 Supported arguments: 638 Supported arguments:
639 639
640 %r = revset expression, parenthesized 640 %r = revset expression, parenthesized
641 %d = int(arg), no quoting 641 %d = rev(int(arg)), no quoting
642 %s = string(arg), escaped and single-quoted 642 %s = string(arg), escaped and single-quoted
643 %b = arg.branch(), escaped and single-quoted 643 %b = arg.branch(), escaped and single-quoted
644 %n = hex(arg), single-quoted 644 %n = hex(arg), single-quoted
645 %% = a literal '%' 645 %% = a literal '%'
646 646
648 and 'p' specifies a list of function parameters of that type. 648 and 'p' specifies a list of function parameters of that type.
649 649
650 >>> formatspec(b'%r:: and %lr', b'10 or 11', (b"this()", b"that()")) 650 >>> formatspec(b'%r:: and %lr', b'10 or 11', (b"this()", b"that()"))
651 '(10 or 11):: and ((this()) or (that()))' 651 '(10 or 11):: and ((this()) or (that()))'
652 >>> formatspec(b'%d:: and not %d::', 10, 20) 652 >>> formatspec(b'%d:: and not %d::', 10, 20)
653 '10:: and not 20::' 653 'rev(10):: and not rev(20)::'
654 >>> formatspec(b'%ld or %ld', [], [1]) 654 >>> formatspec(b'%ld or %ld', [], [1])
655 "_list('') or 1" 655 "_list('') or rev(1)"
656 >>> formatspec(b'keyword(%s)', b'foo\\xe9') 656 >>> formatspec(b'keyword(%s)', b'foo\\xe9')
657 "keyword('foo\\\\xe9')" 657 "keyword('foo\\\\xe9')"
658 >>> b = lambda: b'default' 658 >>> b = lambda: b'default'
659 >>> b.branch = b 659 >>> b.branch = b
660 >>> formatspec(b'branch(%b)', b) 660 >>> formatspec(b'branch(%b)', b)