This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9374

bisect: add a `--rev` flag
Needs RevisionPublic

Authored by SimonSapin on Nov 23 2020, 12:17 PM.

Details

Reviewers
Alphare
martinvonz
Group Reviewers
hg-reviewers
Summary

This is the same as the positional argument, and can be passed more than once.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

SimonSapin created this revision.Nov 23 2020, 12:17 PM
Alphare accepted this revision.Nov 24 2020, 6:17 AM
mharbison72 added a subscriber: mharbison72.Nov 25 2020, 12:38 AM
mharbison72 added inline comments.
tests/test-bisect.t
236

I'm curious what other reviewers think from an API design- the good/bad/skip is a bool, so it feels strange to get the first revspec free, but then need an option for subsequent ones. Can't multiple revspecs be passed in the way a list of files can be passed into file based commands?

Related: https://bz.mercurial-scm.org/show_bug.cgi?id=5530

marmoute added a subscriber: marmoute.Nov 27 2020, 12:24 PM
marmoute added inline comments.
tests/test-bisect.t
236

The addition of --rev is more to mirror other command that take revset as unnamed argument (eg export). I am totally fine with also making multiple revs accepted by this command.

martinvonz requested changes to this revision.Dec 4 2020, 2:49 PM
martinvonz added a subscriber: martinvonz.
martinvonz added inline comments.
mercurial/commands.py
894

I fear that this will increase the likelihood that the user runs hg bisect -r abc123, so could you insert a patch before this one to make sure that that is an error?

913

nit: while you're touching this code, could you also rename this variable to revs to make it clear that it's a collection?

This revision now requires changes to proceed.Dec 4 2020, 2:49 PM

Revision Contents
Changeset List

PathPackages
Mmercurial/commands.py (3 lines)
Mtests/test-bisect.t (4 lines)
Mtests/test-completion.t (2 lines)

Diff 23631

mercurial/commands.py

@command( @command(
b'bisect', b'bisect',
[ [
(b'r', b'reset', False, _(b'reset bisect state')), (b'r', b'reset', False, _(b'reset bisect state')),
(b'g', b'good', False, _(b'mark changeset good')), (b'g', b'good', False, _(b'mark changeset good')),
(b'b', b'bad', False, _(b'mark changeset bad')), (b'b', b'bad', False, _(b'mark changeset bad')),
(b's', b'skip', False, _(b'skip testing changeset')), (b's', b'skip', False, _(b'skip testing changeset')),
(b'', b'rev', [], _(b'select the changesets to mark')),
martinvonzUnsubmitted
Not Done

I fear that this will increase the likelihood that the user runs hg bisect -r abc123, so could you insert a patch before this one to make sure that that is an error?

martinvonz: I fear that this will increase the likelihood that the user runs `hg bisect -r abc123`, so…
(b'e', b'extend', False, _(b'extend the bisect range')), (b'e', b'extend', False, _(b'extend the bisect range')),
( (
b'c', b'c',
b'command', b'command',
b'', b'',
_(b'use command to check changeset state'), _(b'use command to check changeset state'),
_(b'CMD'), _(b'CMD'),
), ),
(b'U', b'noupdate', False, _(b'do not update to target')), (b'U', b'noupdate', False, _(b'do not update to target')),
], ],
_(b"[-gbsr] [-U] [-c CMD] [REV]"), _(b"[-gbsr] [-U] [-c CMD] [REV]"),
helpcategory=command.CATEGORY_CHANGE_NAVIGATION, helpcategory=command.CATEGORY_CHANGE_NAVIGATION,
) )
def bisect( def bisect(
ui, ui,
repo, repo,
positional_1=None, positional_1=None,
positional_2=None, positional_2=None,
rev=None,
martinvonzUnsubmitted
Not Done

nit: while you're touching this code, could you also rename this variable to revs to make it clear that it's a collection?

martinvonz: nit: while you're touching this code, could you also rename this variable to `revs` to make it…
command=None, command=None,
reset=None, reset=None,
good=None, good=None,
bad=None, bad=None,
skip=None, skip=None,
extend=None, extend=None,
noupdate=None, noupdate=None,
): ):
- you can even get a nice graph:: - you can even get a nice graph::
hg log --graph -r "bisect(range)" hg log --graph -r "bisect(range)"
See :hg:`help revisions.bisect` for more about the `bisect()` predicate. See :hg:`help revisions.bisect` for more about the `bisect()` predicate.
Returns 0 on success. Returns 0 on success.
""" """
rev = []
# backward compatibility # backward compatibility
if positional_1 in (b"good", b"bad", b"reset", b"init"): if positional_1 in (b"good", b"bad", b"reset", b"init"):
ui.warn(_(b"(use of 'hg bisect <cmd>' is deprecated)\n")) ui.warn(_(b"(use of 'hg bisect <cmd>' is deprecated)\n"))
cmd = positional_1 cmd = positional_1
rev.append(positional_2) rev.append(positional_2)
if cmd == b"good": if cmd == b"good":
good = True good = True
elif cmd == b"bad": elif cmd == b"bad":

tests/test-bisect.t

date: Thu Jan 01 00:00:29 1970 +0000 date: Thu Jan 01 00:00:29 1970 +0000
summary: msg 29 summary: msg 29
mark revsets instead of single revs mark revsets instead of single revs
$ hg bisect -r $ hg bisect -r
$ hg bisect -b "0::3" $ hg bisect -b "0::3"
$ hg bisect -s "13::16" $ hg bisect -s --rev "13::16"
$ hg bisect -g "26::tip" $ hg bisect -g --rev "26::28" --rev "29" "30::tip"
mharbison72Unsubmitted
Not Done

I'm curious what other reviewers think from an API design- the good/bad/skip is a bool, so it feels strange to get the first revspec free, but then need an option for subsequent ones. Can't multiple revspecs be passed in the way a list of files can be passed into file based commands?

Related: https://bz.mercurial-scm.org/show_bug.cgi?id=5530

mharbison72: I'm curious what other reviewers think from an API design- the good/bad/skip is a bool, so it…
marmouteUnsubmitted
Not Done

The addition of --rev is more to mirror other command that take revset as unnamed argument (eg export). I am totally fine with also making multiple revs accepted by this command.

marmoute: The addition of `--rev` is more to mirror other command that take revset as unnamed argument…
Testing changeset 12:1941b52820a5 (23 changesets remaining, ~4 tests) Testing changeset 12:1941b52820a5 (23 changesets remaining, ~4 tests)
1 files updated, 0 files merged, 0 files removed, 0 files unresolved 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ cat .hg/bisect.state $ cat .hg/bisect.state
bad b99c7b9c8e11558adef3fad9af211c58d46f325b bad b99c7b9c8e11558adef3fad9af211c58d46f325b
bad 5cd978ea51499179507ee7b6f340d2dbaa401185 bad 5cd978ea51499179507ee7b6f340d2dbaa401185
bad db07c04beaca44cf24832541e7f4a2346a95275b bad db07c04beaca44cf24832541e7f4a2346a95275b
bad b53bea5e2fcb30d3e00bd3409507a5659ce0fd8b bad b53bea5e2fcb30d3e00bd3409507a5659ce0fd8b
current 1941b52820a544549596820a8ae006842b0e2c64 current 1941b52820a544549596820a8ae006842b0e2c64

tests/test-completion.t

Show all commands + options Show all commands + options
$ hg debugcommands $ hg debugcommands
abort: dry-run abort: dry-run
add: include, exclude, subrepos, dry-run add: include, exclude, subrepos, dry-run
addremove: similarity, subrepos, include, exclude, dry-run addremove: similarity, subrepos, include, exclude, dry-run
annotate: rev, follow, no-follow, text, user, file, date, number, changeset, line-number, skip, ignore-all-space, ignore-space-change, ignore-blank-lines, ignore-space-at-eol, include, exclude, template annotate: rev, follow, no-follow, text, user, file, date, number, changeset, line-number, skip, ignore-all-space, ignore-space-change, ignore-blank-lines, ignore-space-at-eol, include, exclude, template
archive: no-decode, prefix, rev, type, subrepos, include, exclude archive: no-decode, prefix, rev, type, subrepos, include, exclude
backout: merge, commit, no-commit, parent, rev, edit, tool, include, exclude, message, logfile, date, user backout: merge, commit, no-commit, parent, rev, edit, tool, include, exclude, message, logfile, date, user
bisect: reset, good, bad, skip, extend, command, noupdate bisect: reset, good, bad, skip, rev, extend, command, noupdate
bookmarks: force, rev, delete, rename, inactive, list, template bookmarks: force, rev, delete, rename, inactive, list, template
branch: force, clean, rev branch: force, clean, rev
branches: active, closed, rev, template branches: active, closed, rev, template
bundle: force, rev, branch, base, all, type, ssh, remotecmd, insecure bundle: force, rev, branch, base, all, type, ssh, remotecmd, insecure
cat: output, rev, decode, include, exclude, template cat: output, rev, decode, include, exclude, template
clone: noupdate, updaterev, rev, branch, pull, uncompressed, stream, ssh, remotecmd, insecure clone: noupdate, updaterev, rev, branch, pull, uncompressed, stream, ssh, remotecmd, insecure
commit: addremove, close-branch, amend, secret, edit, force-close-branch, interactive, include, exclude, message, logfile, date, user, subrepos commit: addremove, close-branch, amend, secret, edit, force-close-branch, interactive, include, exclude, message, logfile, date, user, subrepos
config: untrusted, edit, local, shared, non-shared, global, template config: untrusted, edit, local, shared, non-shared, global, template