This is an archive of the discontinued Mercurial Phabricator instance.

mercurial/cmdutil.py
2007	"cannot specify both --dry-run and --confirm" is the style used in other commands.
mercurial/commands.py
2043	Looks like test-completion.t also needs to be updated.

av6 requested changes to this revision.Mar 22 2018, 11:39 AM

This revision now requires changes to proceed.Mar 22 2018, 11:39 AM

pulkit requested changes to this revision.Mar 22 2018, 12:38 PM

pulkit added a subscriber: pulkit.

pulkit added inline comments.

mercurial/cmdutil.py
2057	This should be before "removing <filename>" message. Also this message should also show filename, something like "forget <filenames> (yn)?"
2060	I am not sure what are you trying do here. Can you look again?
mercurial/commands.py
2079	We don't need r'' prefix here. You can drop one before 'dry_run' too.
tests/test-add.t
290	In such cases, `hg status` will be a better option.
302	Looks like it's not reading the input. Look whether ui.interactive is set or not.

khanchi97 updated this revision to Diff 7254.Mar 22 2018, 3:49 PM

Just curious why we want the --confirm option for such a simple command.

pulkit requested changes to this revision.Mar 23 2018, 12:47 PM

pulkit added inline comments.

mercurial/cmdutil.py
2047	I think the right behavior should be to ask for each file and forget the ones which user confirmed to forget.
tests/test-add.t
293	This is not reading 'n' as an input here and rather going for the default value. You should set `ui.interactive` to True.
300	Please add more tests involving multiple files at once.

This revision now requires changes to proceed.Mar 23 2018, 12:47 PM

In D2934#47327, @yuja wrote:

Just curious why we want the --confirm option for such a simple command.

I agree with you here that this command is simple. I think @khanchi97 is picking easy and simple ones to start with.

mharbison72 added a subscriber: mharbison72.Mar 23 2018, 7:54 PM

mharbison72 added inline comments.

mercurial/cmdutil.py
2047	+1 to the prompt per file. And maybe options for 'all'/'none' to apply to the rest of the files that haven't been processed? Maybe look at --interactive on commit or similar for naming on that.

khanchi97 updated this revision to Diff 7285.Mar 26 2018, 7:13 AM

I have made the requested changes. Now --confirm will prompt per file and also added all/skip option which will either include all the remaining files or skip all the remaining files according the user input.

mercurial/cmdutil.py
2007	sorry, am I supposed to make any change here?
2047	cool :)
mercurial/commands.py
2043	yeah, I will update this.
tests/test-add.t
296	I have set ui.interactive=True but it still not read input.
302	I need some help here. Why it's not passing `n` as a response? I passed `n` in line 299 but as a response from user why it is taking `y` everytime?
302	I have set ui.interactive=True but, the problem is still same. And also by default ui.interactive is always True.

I've touched this up in-flight and will land it, thanks.

(Suggestion: diff the output of hg export on your version vs the one that lands to see what I needed to tweak - there were some oversights in tests during development it looks like.)

Closed by commit rHGe7bf5a73e4e1: forget: add --confirm option (authored by khanchi97). · Explain WhyApr 16 2018, 10:35 PM

This revision was automatically updated to reflect the committed changes.

In D2934#54155, @durin42 wrote:

(Suggestion: diff the output of hg export on your version vs the one that lands to see what I needed to tweak - there were some oversights in tests during development it looks like.)

Alternately if you have the extdiff extension configured, you can diff the two revisions directly and supply --patch to the extdiff command get the same effect. I think it's easier to view graphically, than a diff of a diff.

The change looks good, but new behavior sounds more like --interactive
than --confirm.

I'm fine to rename this to --interactive if we think that makes more sense, even if we do it during the freeze.

In D2934#54256, @yuja wrote:

The change looks good, but new behavior sounds more like --interactive
than --confirm.

I agree. @khanchi97 can you send a follow-up renaming the flag to --interactive?

In D2934#54295, @pulkit wrote:

In D2934#54256, @yuja wrote:

The change looks good, but new behavior sounds more like --interactive
than --confirm.

I agree. @khanchi97 can you send a follow-up renaming the flag to --interactive?

Okay, I will send one. I also think --interactive is better than --confirm.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/cmdutil.py (20 lines)
M			mercurial/commands.py (8 lines)
M			tests/test-add.t (27 lines)
M			tests/test-completion.t (2 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	7252		Mar 22 2018, 10:01 AM	★	★
Diff 2	7254		Mar 22 2018, 3:49 PM	★	★
Diff 3	7285		Mar 26 2018, 7:13 AM	★	★
Diff 4	8362	rHGe7bf5a73e4e1c36ec5d389cf73ee12cac7a16da8	Mar 22 2018, 6:41 AM	★	★

Diff 7254

mercurial/cmdutil.py


	# templates of common command options			# templates of common command options

	dryrunopts = [			dryrunopts = [
	('n', 'dry-run', None,			('n', 'dry-run', None,
	_('do not perform actions, just print output')),			_('do not perform actions, just print output')),
	]			]

				confirmopts = [
				('', 'confirm', None,
				_('ask before applying actions')),
				]

	remoteopts = [			remoteopts = [
	('e', 'ssh', '',			('e', 'ssh', '',
	_('specify ssh command to use'), _('CMD')),			_('specify ssh command to use'), _('CMD')),
	('', 'remotecmd', '',			('', 'remotecmd', '',
	_('specify hg command to run on the remote side'), _('CMD')),			_('specify hg command to run on the remote side'), _('CMD')),
	('', 'insecure', None,			('', 'insecure', None,
	_('do not verify server certificate (ignoring web.cacerts config)')),			_('do not verify server certificate (ignoring web.cacerts config)')),
	]			]
	webconf[serverpath] = repo.root			webconf[serverpath] = repo.root
	repo.ui.debug('adding %s = %s\n' % (serverpath, repo.root))			repo.ui.debug('adding %s = %s\n' % (serverpath, repo.root))

	for r in repo.revs('filelog("path:.hgsub")'):			for r in repo.revs('filelog("path:.hgsub")'):
	ctx = repo[r]			ctx = repo[r]
	for subpath in ctx.substate:			for subpath in ctx.substate:
	ctx.sub(subpath).addwebdirpath(serverpath, webconf)			ctx.sub(subpath).addwebdirpath(serverpath, webconf)

	def forget(ui, repo, match, prefix, explicitonly, dryrun):			def forget(ui, repo, match, prefix, explicitonly, dryrun, confirm):
				if dryrun and confirm:
				raise error.Abort(_("can't specify --dry-run and --confirm"))
				av6Unsubmitted Not Done "cannot specify both --dry-run and --confirm" is the style used in other commands. av6: "cannot specify both --dry-run and --confirm" is the style used in other commands.
				khanchi97AuthorUnsubmitted Not Done sorry, am I supposed to make any change here? khanchi97: sorry, am I supposed to make any change here?
	join = lambda f: os.path.join(prefix, f)			join = lambda f: os.path.join(prefix, f)
	bad = []			bad = []
	badfn = lambda x, y: bad.append(x) or match.bad(x, y)			badfn = lambda x, y: bad.append(x) or match.bad(x, y)
	wctx = repo[None]			wctx = repo[None]
	forgot = []			forgot = []

	s = repo.status(match=matchmod.badmatch(match, badfn), clean=True)			s = repo.status(match=matchmod.badmatch(match, badfn), clean=True)
	forget = sorted(s.modified + s.added + s.deleted + s.clean)			forget = sorted(s.modified + s.added + s.deleted + s.clean)
	if explicitonly:			if explicitonly:
	forget = [f for f in forget if match.exact(f)]			forget = [f for f in forget if match.exact(f)]

	for subpath in sorted(wctx.substate):			for subpath in sorted(wctx.substate):
	sub = wctx.sub(subpath)			sub = wctx.sub(subpath)
	try:			try:
	submatch = matchmod.subdirmatcher(subpath, match)			submatch = matchmod.subdirmatcher(subpath, match)
	subbad, subforgot = sub.forget(submatch, prefix, dryrun=dryrun)			subbad, subforgot = sub.forget(submatch, prefix,
				dryrun=dryrun, confirm=confirm)
	bad.extend([subpath + '/' + f for f in subbad])			bad.extend([subpath + '/' + f for f in subbad])
	forgot.extend([subpath + '/' + f for f in subforgot])			forgot.extend([subpath + '/' + f for f in subforgot])
	except error.LookupError:			except error.LookupError:
	ui.status(_("skipping missing subrepository: %s\n")			ui.status(_("skipping missing subrepository: %s\n")
	% join(subpath))			% join(subpath))

	if not explicitonly:			if not explicitonly:
	for f in match.files():			for f in match.files():
	if f not in repo.dirstate and not repo.wvfs.isdir(f):			if f not in repo.dirstate and not repo.wvfs.isdir(f):
	if f not in forgot:			if f not in forgot:
	if repo.wvfs.exists(f):			if repo.wvfs.exists(f):
	# Don't complain if the exact case match wasn't given.			# Don't complain if the exact case match wasn't given.
	# But don't do this until after checking 'forgot', so			# But don't do this until after checking 'forgot', so
	# that subrepo files aren't normalized, and this op is			# that subrepo files aren't normalized, and this op is
	# purely from data cached by the status walk above.			# purely from data cached by the status walk above.
	if repo.dirstate.normalize(f) in repo.dirstate:			if repo.dirstate.normalize(f) in repo.dirstate:
	continue			continue
	ui.warn(_('not removing %s: '			ui.warn(_('not removing %s: '
	'file is already untracked\n')			'file is already untracked\n')
	% match.rel(f))			% match.rel(f))
	bad.append(f)			bad.append(f)

				if confirm:
				pulkitUnsubmitted Not Done I think the right behavior should be to ask for each file and forget the ones which user confirmed to forget. pulkit: I think the right behavior should be to ask for each file and forget the ones which user…
				khanchi97AuthorUnsubmitted Not Done cool :) khanchi97: cool :)
				mharbison72Unsubmitted Not Done +1 to the prompt per file. And maybe options for 'all'/'none' to apply to the rest of the files that haven't been processed? Maybe look at --interactive on commit or similar for naming on that. mharbison72: +1 to the prompt per file. And maybe options for 'all'/'none' to apply to the rest of the…
				filenames = ' '.join(forget)
				if ui.promptchoice(_('forget %s (yn)?'
				'$$ &Yes $$ &No') % filenames):
				raise error.Abort(_('forget canceled'))

	for f in forget:			for f in forget:
	if ui.verbose or not match.exact(f):			if ui.verbose or not match.exact(f) or confirm:
	ui.status(_('removing %s\n') % match.rel(f))			ui.status(_('removing %s\n') % match.rel(f))

	if not dryrun:			if not dryrun:
				pulkitUnsubmitted Not Done This should be before "removing <filename>" message. Also this message should also show filename, something like "forget <filenames> (yn)?" pulkit: This should be before "removing <filename>" message. Also this message should also show…
	rejected = wctx.forget(forget, prefix)			rejected = wctx.forget(forget, prefix)
	bad.extend(f for f in rejected if f in match.files())			bad.extend(f for f in rejected if f in match.files())
	forgot.extend(f for f in forget if f not in rejected)			forgot.extend(f for f in forget if f not in rejected)
				pulkitUnsubmitted Not Done I am not sure what are you trying do here. Can you look again? pulkit: I am not sure what are you trying do here. Can you look again?
	return bad, forgot			return bad, forgot

	def files(ui, ctx, m, fm, fmt, subrepos):			def files(ui, ctx, m, fm, fmt, subrepos):
	rev = ctx.rev()			rev = ctx.rev()
	ret = 1			ret = 1
	ds = ctx.repo().dirstate			ds = ctx.repo().dirstate

	for f in ctx.matches(m):			for f in ctx.matches(m):

mercurial/commands.py

	('', 'version', None, _('output version information and exit')),			('', 'version', None, _('output version information and exit')),
	('h', 'help', None, _('display help and exit')),			('h', 'help', None, _('display help and exit')),
	('', 'hidden', False, _('consider hidden changesets')),			('', 'hidden', False, _('consider hidden changesets')),
	('', 'pager', 'auto',			('', 'pager', 'auto',
	_("when to paginate (boolean, always, auto, or never)"), _('TYPE')),			_("when to paginate (boolean, always, auto, or never)"), _('TYPE')),
	]			]

	dryrunopts = cmdutil.dryrunopts			dryrunopts = cmdutil.dryrunopts
				confirmopts = cmdutil.confirmopts
	remoteopts = cmdutil.remoteopts			remoteopts = cmdutil.remoteopts
	walkopts = cmdutil.walkopts			walkopts = cmdutil.walkopts
	commitopts = cmdutil.commitopts			commitopts = cmdutil.commitopts
	commitopts2 = cmdutil.commitopts2			commitopts2 = cmdutil.commitopts2
	formatteropts = cmdutil.formatteropts			formatteropts = cmdutil.formatteropts
	templateopts = cmdutil.templateopts			templateopts = cmdutil.templateopts
	logopts = cmdutil.logopts			logopts = cmdutil.logopts
	diffopts = cmdutil.diffopts			diffopts = cmdutil.diffopts

	m = scmutil.match(ctx, pats, opts)			m = scmutil.match(ctx, pats, opts)
	ui.pager('files')			ui.pager('files')
	with ui.formatter('files', opts) as fm:			with ui.formatter('files', opts) as fm:
	return cmdutil.files(ui, ctx, m, fm, fmt, opts.get('subrepos'))			return cmdutil.files(ui, ctx, m, fm, fmt, opts.get('subrepos'))

	@command(			@command(
	'^forget',			'^forget',
	walkopts + dryrunopts,			walkopts + dryrunopts + confirmopts,
				av6Unsubmitted Not Done Looks like test-completion.t also needs to be updated. av6: Looks like test-completion.t also needs to be updated.
				khanchi97AuthorUnsubmitted Not Done yeah, I will update this. khanchi97: yeah, I will update this.
	_('[OPTION]... FILE...'), inferrepo=True)			_('[OPTION]... FILE...'), inferrepo=True)
	def forget(ui, repo, pats, *opts):			def forget(ui, repo, pats, *opts):
	"""forget the specified files on the next commit			"""forget the specified files on the next commit

	Mark the specified files so they will no longer be tracked			Mark the specified files so they will no longer be tracked
	after the next commit.			after the next commit.

	This only removes files from the current branch, not from the			This only removes files from the current branch, not from the
	Returns 0 on success.			Returns 0 on success.
	"""			"""

	opts = pycompat.byteskwargs(opts)			opts = pycompat.byteskwargs(opts)
	if not pats:			if not pats:
	raise error.Abort(_('no files specified'))			raise error.Abort(_('no files specified'))

	m = scmutil.match(repo[None], pats, opts)			m = scmutil.match(repo[None], pats, opts)
	dryrun = opts.get(r'dry_run')			dryrun, confirm = opts.get('dry_run'), opts.get('confirm')
				pulkitUnsubmitted Not Done We don't need r'' prefix here. You can drop one before 'dry_run' too. pulkit: We don't need r'' prefix here. You can drop one before 'dry_run' too.
	rejected = cmdutil.forget(ui, repo, m, prefix="",			rejected = cmdutil.forget(ui, repo, m, prefix="",
	explicitonly=False, dryrun=dryrun)[0]			explicitonly=False, dryrun=dryrun,
				confirm=confirm)[0]
	return rejected and 1 or 0			return rejected and 1 or 0

	@command(			@command(
	'graft',			'graft',
	[('r', 'rev', [], _('revisions to graft'), _('REV')),			[('r', 'rev', [], _('revisions to graft'), _('REV')),
	('c', 'continue', False, _('resume interrupted graft')),			('c', 'continue', False, _('resume interrupted graft')),
	('e', 'edit', False, _('invoke editor on commit messages')),			('e', 'edit', False, _('invoke editor on commit messages')),
	('', 'log', None, _('append graft info to log message')),			('', 'log', None, _('append graft info to log message')),

tests/test-add.t

	$ hg forget foo --dry-run -v			$ hg forget foo --dry-run -v
	removing foo			removing foo
	$ hg diff			$ hg diff
	$ hg forget not_exist -n			$ hg forget not_exist -n
	not_exist: $ENOENT$			not_exist: $ENOENT$
	[1]			[1]

	$ cd ..			$ cd ..

				test --confirm option in forget

				$ hg init forgetconfirm
				$ cd forgetconfirm
				$ echo foo > foo
				$ hg commit -qAm "foo"
				$ hg forget foo --dry-run --confirm
				abort: can't specify --dry-run and --confirm
				[255]
				$ hg forget foo --confirm << EOF
				> y
				> EOF
				forget foo (yn)? y
				removing foo
				$ hg status
				pulkitUnsubmitted Not Done In such cases, `hg status` will be a better option. pulkit: In such cases, `hg status` will be a better option.
				R foo
				$ hg up -qC .
				$ hg forget foo --confirm << EOF
				pulkitUnsubmitted Not Done This is not reading 'n' as an input here and rather going for the default value. You should set `ui.interactive` to True. pulkit: This is not reading 'n' as an input here and rather going for the default value. You should set…
				> n
				> EOF
				forget foo (yn)? y
				khanchi97AuthorUnsubmitted Not Done I have set ui.interactive=True but it still not read input. khanchi97: I have set ui.interactive=True but it still not read input.
				removing foo
				$ hg status
				R foo

				pulkitUnsubmitted Not Done Please add more tests involving multiple files at once. pulkit: Please add more tests involving multiple files at once.
				$ cd ..
				khanchi97AuthorUnsubmitted Not Done I need some help here. Why it's not passing `n` as a response? I passed `n` in line 299 but as a response from user why it is taking `y` everytime? khanchi97: I need some help here. Why it's not passing `n` as a response? I passed `n` in line 299 but as…
				pulkitUnsubmitted Not Done Looks like it's not reading the input. Look whether ui.interactive is set or not. pulkit: Looks like it's not reading the input. Look whether ui.interactive is set or not.
				khanchi97AuthorUnsubmitted Not Done I have set ui.interactive=True but, the problem is still same. And also by default ui.interactive is always True. khanchi97: I have set ui.interactive=True but, the problem is still same. And also by default ui.

tests/test-completion.t

	Show all commands + options			Show all commands + options
	$ hg debugcommands			$ hg debugcommands
	add: include, exclude, subrepos, dry-run			add: include, exclude, subrepos, 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
	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, interactive, include, exclude, message, logfile, date, user, subrepos			commit: addremove, close-branch, amend, secret, edit, interactive, include, exclude, message, logfile, date, user, subrepos
	diff: rev, change, text, git, binary, nodates, noprefix, show-function, reverse, ignore-all-space, ignore-space-change, ignore-blank-lines, ignore-space-at-eol, unified, stat, root, include, exclude, subrepos			diff: rev, change, text, git, binary, nodates, noprefix, show-function, reverse, ignore-all-space, ignore-space-change, ignore-blank-lines, ignore-space-at-eol, unified, stat, root, include, exclude, subrepos
	export: output, switch-parent, rev, text, git, binary, nodates			export: output, switch-parent, rev, text, git, binary, nodates
	forget: include, exclude, dry-run			forget: include, exclude, dry-run, confirm
	init: ssh, remotecmd, insecure			init: ssh, remotecmd, insecure
	log: follow, follow-first, date, copies, keyword, rev, line-range, removed, only-merges, user, only-branch, branch, prune, patch, git, limit, no-merges, stat, graph, style, template, include, exclude			log: follow, follow-first, date, copies, keyword, rev, line-range, removed, only-merges, user, only-branch, branch, prune, patch, git, limit, no-merges, stat, graph, style, template, include, exclude
	merge: force, rev, preview, abort, tool			merge: force, rev, preview, abort, tool
	pull: update, force, rev, bookmark, branch, ssh, remotecmd, insecure			pull: update, force, rev, bookmark, branch, ssh, remotecmd, insecure
	push: force, rev, bookmark, branch, new-branch, pushvars, ssh, remotecmd, insecure			push: force, rev, bookmark, branch, new-branch, pushvars, ssh, remotecmd, insecure
	remove: after, force, subrepos, include, exclude			remove: after, force, subrepos, include, exclude
	serve: accesslog, daemon, daemon-postexec, errorlog, port, address, prefix, name, web-conf, webdir-conf, pid-file, stdio, cmdserver, templates, style, ipv6, certificate, subrepos			serve: accesslog, daemon, daemon-postexec, errorlog, port, address, prefix, name, web-conf, webdir-conf, pid-file, stdio, cmdserver, templates, style, ipv6, certificate, subrepos
	status: all, modified, added, removed, deleted, clean, unknown, ignored, no-status, terse, copies, print0, rev, change, include, exclude, subrepos, template			status: all, modified, added, removed, deleted, clean, unknown, ignored, no-status, terse, copies, print0, rev, change, include, exclude, subrepos, template

This is an archive of the discontinued Mercurial Phabricator instance.

forget: add --confirm optionClosedPublic

Details

Diff Detail

Event Timeline