This is an archive of the discontinued Mercurial Phabricator instance.

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

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

Download Raw Diff

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

Herald added a reviewer: hg-reviewers. · View Herald TranscriptNov 23 2020, 12:17 PM

Herald added a subscriber: mercurial-patches. · View Herald Transcript

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

			Path	Packages
M			mercurial/commands.py (3 lines)
M			tests/test-bisect.t (4 lines)
M			tests/test-completion.t (2 lines)

Commit	Parents	Author	Summary	Date
358d5d3cab3d	26695d9fda66	Simon Sapin		Nov 23 2020, 6:56 AM

Status	Author	Revision
Needs Revision	SimonSapin	D9374 bisect: add a `--rev` flag
Closed	SimonSapin	D9373 bisect: add `-G` to an `hg log` command in a test
Closed	SimonSapin	D9372 bisect: refactor to work on a list of revspecs
Closed	SimonSapin	D9371 bisect: use tuple literal instead of split on string literal
Closed	SimonSapin	D9370 hgignore: add VS Code config

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

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	23631			Nov 23 2020, 12:17 PM	★	★