This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5834

commit/revert: if interactive, look elsewhere for whitespace settings (BC)
ClosedPublic

Authored by spectral on Feb 4 2019, 3:39 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG66399f2e92aa: commit: if interactive, look elsewhere for whitespace settings (BC)
Summary

Previously, when doing commit -i, we respected diff.ignorews and other
whitespace-related settings, which is probably unexpected. The primary reason
for this is to support hgext.record's commandline options, it's probably
accidental that the [diff] settings were also considered. See comments on
issue6042 and D5490. This can cause problems (issue5839, issue6042).

It is assumed by the author that the [diff] section is primarily for *viewing*
diffs, and that it is unlikely what people intend when attempting to commit or
revert.

With this change, if a user wants the behavior, they can clone their [diff]
settings to commands.commit.interactive.<setting>. This is thus a mild BC
change, but one I suspect is not going to be relied on by anyone.

Note: while doing a partial commit/revert, we do not know what command the user
is actually running. This means that the split extension, which ends up calling
into this code, will respect the commands.commit.interactive.<setting>
settings, and not a hypothetical commands.split.interactive.<setting>. This
*also* means that setting commands.commit.interactive.ignoreblanklines, for
example, will still cause issue5839. Considering the highly unlikely chance that
a user actually sets commands.commit.interactive.<setting>, the author deems
this risk acceptable.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

spectral created this revision.Feb 4 2019, 3:39 PM
yuja added a subscriber: yuja.Feb 5 2019, 7:03 AM

Updated the tests added at d1d3094b54f9 and queued, thanks.

Closed by commit rHG66399f2e92aa: commit: if interactive, look elsewhere for whitespace settings (BC) (authored by spectral). · Explain WhyFeb 5 2019, 7:12 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/cmdutil.py (8 lines)
Mmercurial/configitems.py (2 lines)

Diff 13761

mercurial/cmdutil.py

if not force: if not force:
vdirs = [] vdirs = []
match.explicitdir = vdirs.append match.explicitdir = vdirs.append
match.bad = fail match.bad = fail
status = repo.status(match=match) status = repo.status(match=match)
if not force: if not force:
repo.checkcommitpatterns(wctx, vdirs, match, status, fail) repo.checkcommitpatterns(wctx, vdirs, match, status, fail)
diffopts = patch.difffeatureopts(ui, opts=opts) diffopts = patch.difffeatureopts(ui, opts=opts, whitespace=True,
section='commands',
configprefix='commit.interactive.')
diffopts.nodates = True diffopts.nodates = True
diffopts.git = True diffopts.git = True
diffopts.showfunc = True diffopts.showfunc = True
originaldiff = patch.diff(repo, changes=status, opts=diffopts) originaldiff = patch.diff(repo, changes=status, opts=diffopts)
originalchunks = patch.parsepatch(originaldiff) originalchunks = patch.parsepatch(originaldiff)
# 1. filter patch, since we are intending to apply subset of it # 1. filter patch, since we are intending to apply subset of it
try: try:
else: else:
normal = repo.dirstate.normal normal = repo.dirstate.normal
newlyaddedandmodifiedfiles = set() newlyaddedandmodifiedfiles = set()
if interactive: if interactive:
# Prompt the user for changes to revert # Prompt the user for changes to revert
torevert = [f for f in actions['revert'][0] if f not in excluded_files] torevert = [f for f in actions['revert'][0] if f not in excluded_files]
m = scmutil.matchfiles(repo, torevert) m = scmutil.matchfiles(repo, torevert)
diffopts = patch.difffeatureopts(repo.ui) diffopts = patch.difffeatureopts(repo.ui, whitespace=True,
section='commands',
configprefix='revert.interactive.')
diffopts.nodates = True diffopts.nodates = True
diffopts.git = True diffopts.git = True
operation = 'discard' operation = 'discard'
reversehunks = True reversehunks = True
if node != parent: if node != parent:
operation = 'apply' operation = 'apply'
reversehunks = False reversehunks = False
if reversehunks: if reversehunks:

mercurial/configitems.py

generic=True, generic=True,
) )
coreconfigitem('color', 'mode', coreconfigitem('color', 'mode',
default='auto', default='auto',
) )
coreconfigitem('color', 'pagermode', coreconfigitem('color', 'pagermode',
default=dynamicdefault, default=dynamicdefault,
) )
_registerdiffopts(section='commands', configprefix='commit.interactive.')
coreconfigitem('commands', 'grep.all-files', coreconfigitem('commands', 'grep.all-files',
default=False, default=False,
) )
coreconfigitem('commands', 'resolve.confirm', coreconfigitem('commands', 'resolve.confirm',
default=False, default=False,
) )
coreconfigitem('commands', 'resolve.explicit-re-merge', coreconfigitem('commands', 'resolve.explicit-re-merge',
default=False, default=False,
) )
coreconfigitem('commands', 'resolve.mark-check', coreconfigitem('commands', 'resolve.mark-check',
default='none', default='none',
) )
_registerdiffopts(section='commands', configprefix='revert.interactive.')
coreconfigitem('commands', 'show.aliasprefix', coreconfigitem('commands', 'show.aliasprefix',
default=list, default=list,
) )
coreconfigitem('commands', 'status.relative', coreconfigitem('commands', 'status.relative',
default=False, default=False,
) )
coreconfigitem('commands', 'status.skipstates', coreconfigitem('commands', 'status.skipstates',
default=[], default=[],