( )⚙ D422 mdiff: add a --ignore-space-at-eol option

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D422

mdiff: add a --ignore-space-at-eol option
ClosedPublic

Authored by dsp on Aug 16 2017, 6:13 PM.

Details

Reviewers
quark
yuja
Group Reviewers
hg-reviewers
Commits
rHGda07367d683b: mdiff: add a --ignore-space-at-eol option
rHGae7308d7604e: mdiff: add a --ignore-space-at-eol option
Summary

Add an option that only ignores whitespaces at EOL. The name of the option is
the same as Git.

.. feature::

Added `--ignore-space-at-eol` diff option to ignore whitespace differences at line endings.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

dsp created this revision.Aug 16 2017, 6:13 PM
quark accepted this revision.Aug 16 2017, 8:25 PM
quark added a subscriber: quark.

Looks good to me. It seems to be a significant feature that is worth a release note. Maybe check https://www.mercurial-scm.org/wiki/ReleasenotesExtension and amend the commit message to include something like:

.. feature::

   Added `--ignore-space-at-eol` diff option to ignore whitespace differences at line endings.
mercurial/help/config.txt
317

Maybe remove only since otherwise it sounds ignorewseol must be used together with ignorews.

dsp edited the summary of this revision. (Show Details)Aug 16 2017, 9:15 PM
dsp updated this revision to Diff 1022.
dsp added a comment.Aug 16 2017, 9:17 PM

Updated the diff with your suggested changed

martinvonz added a subscriber: martinvonz.Aug 23 2017, 1:30 PM

Looks like you didn't run test-help.t. I'll fix it in flight, but please run all tests next time.

martinvonz added a comment.Aug 23 2017, 1:33 PM
In D422#7795, @martinvonz wrote:

Looks like you didn't run test-help.t. I'll fix it in flight, but please run all tests next time.

...and test-completion.t and test-qrecord.t

martinvonz added a comment.Aug 23 2017, 1:35 PM
In D422#7797, @martinvonz wrote:
In D422#7795, @martinvonz wrote:

Looks like you didn't run test-help.t. I'll fix it in flight, but please run all tests next time.

...and test-completion.t and test-qrecord.t

...and test-record.t and test-duplicateoptions.py. This is getting a bit too many, especially since the last one is not trivial, so please send an update instead.

quark requested changes to this revision.Aug 23 2017, 2:06 PM

Maybe we should set up CI on Phabricator.

This revision now requires changes to proceed.Aug 23 2017, 2:06 PM
martinvonz added a comment.Aug 23 2017, 2:19 PM
In D422#7804, @quark wrote:

Maybe we should set up CI on Phabricator.

Yes, I've also suggested that. The concern was sandboxing, so we can run tests from untrusted people. I have no experience setting something like that up, but I'd be very happy if someone else could do it. It would be really nice to not have to run tests after applying a patch (if we're willing to live with the test failures we'd get every now and then due to conflicting changes).

akushner added a subscriber: akushner.Aug 23 2017, 2:26 PM

@martinvonz - a reasonable first pass would be to whitelist known contributors for CI runs.

martinvonz added a comment.Aug 23 2017, 4:08 PM
In D422#7818, @akushner wrote:

@martinvonz - a reasonable first pass would be to whitelist known contributors for CI runs.

That sounds reasonable. Still not my area of expertise, but I'd be happy to see it done.

dsp edited edge metadata.Aug 23 2017, 10:39 PM
dsp updated this revision to Diff 1240.
dsp marked an inline comment as done.Aug 23 2017, 10:39 PM
dsp updated this revision to Diff 1241.Aug 23 2017, 10:40 PM
morisgi added a subscriber: morisgi.Aug 24 2017, 2:49 AM
morisgi added inline comments.
mercurial/cmdutil.py
126–127

Regular diff chose -Z for the short option

yuja added a subscriber: yuja.Aug 24 2017, 11:10 AM
yuja added inline comments.
mercurial/cmdutil.py
126–127

-Z sounds good, which doesn't conflict with any existing short
options.

FWIW, the long option is called as --ignore-trailing-space in
GNU diff.

dsp added inline comments.Aug 24 2017, 1:55 PM
mercurial/cmdutil.py
126–127

I am happy to change '-Z'. The used the long opt name from git. I am happy either way, but I personally would prefer the git name.

dsp updated this revision to Diff 1266.Aug 24 2017, 6:38 PM
yuja requested changes to this revision.Aug 27 2017, 7:01 AM
yuja added inline comments.
mercurial/mdiff.py
102

This matches empty lines because \s includes \n.
Perhaps it should be r'[ \t\r]+\n'.

Can you add a test for this bug?

This revision now requires changes to proceed.Aug 27 2017, 7:01 AM
dsp edited edge metadata.Aug 29 2017, 9:21 PM
dsp updated this revision to Diff 1415.
yuja accepted this revision.Aug 30 2017, 9:26 AM

Queued, thanks.

Closed by commit rHGae7308d7604e: mdiff: add a --ignore-space-at-eol option (authored by dsp). · Explain WhyAug 30 2017, 9:53 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 1435

mercurial/cmdutil.py

diffwsopts = [ diffwsopts = [
('w', 'ignore-all-space', None, ('w', 'ignore-all-space', None,
_('ignore white space when comparing lines')), _('ignore white space when comparing lines')),
('b', 'ignore-space-change', None, ('b', 'ignore-space-change', None,
_('ignore changes in the amount of white space')), _('ignore changes in the amount of white space')),
('B', 'ignore-blank-lines', None, ('B', 'ignore-blank-lines', None,
_('ignore changes whose lines are all blank')), _('ignore changes whose lines are all blank')),
('Z', 'ignore-space-at-eol', None,
_('ignore changes in whitespace at EOL')),
morisgiUnsubmitted
Not Done

Regular diff chose -Z for the short option

morisgi: Regular diff chose -Z for the short option
yujaUnsubmitted
Not Done

-Z sounds good, which doesn't conflict with any existing short
options.

FWIW, the long option is called as --ignore-trailing-space in
GNU diff.

yuja: `-Z` sounds good, which doesn't conflict with any existing short options. FWIW, the long…
dspAuthorUnsubmitted
Not Done

I am happy to change '-Z'. The used the long opt name from git. I am happy either way, but I personally would prefer the git name.

dsp: I am happy to change '-Z'. The used the long opt name from git. I am happy either way, but I…
] ]
diffopts2 = [ diffopts2 = [
('', 'noprefix', None, _('omit a/ and b/ prefixes from filenames')), ('', 'noprefix', None, _('omit a/ and b/ prefixes from filenames')),
('p', 'show-function', None, _('show which function each change is in')), ('p', 'show-function', None, _('show which function each change is in')),
('', 'reverse', None, _('produce a diff that undoes the changes')), ('', 'reverse', None, _('produce a diff that undoes the changes')),
] + diffwsopts + [ ] + diffwsopts + [
('U', 'unified', '', ('U', 'unified', '',

mercurial/help/config.txt

Settings used when displaying file annotations. All values are Settings used when displaying file annotations. All values are
Booleans and default to False. See :hg:`help config.diff` for Booleans and default to False. See :hg:`help config.diff` for
related options for the diff command. related options for the diff command.
``ignorews`` ``ignorews``
Ignore white space when comparing lines. Ignore white space when comparing lines.
``ignorewseol``
Ignore white space at the end of a line when comparing lines.
quarkUnsubmitted
Done

Maybe remove only since otherwise it sounds ignorewseol must be used together with ignorews.

quark: Maybe remove `only` since otherwise it sounds `ignorewseol` must be used together with…
``ignorewsamount`` ``ignorewsamount``
Ignore changes in the amount of white space. Ignore changes in the amount of white space.
``ignoreblanklines`` ``ignoreblanklines``
Ignore changes whose lines are all blank. Ignore changes whose lines are all blank.
``auth`` ``auth``

mercurial/mdiff.py

'showfunc': False, 'showfunc': False,
'git': False, 'git': False,
'nodates': False, 'nodates': False,
'nobinary': False, 'nobinary': False,
'noprefix': False, 'noprefix': False,
'index': 0, 'index': 0,
'ignorews': False, 'ignorews': False,
'ignorewsamount': False, 'ignorewsamount': False,
'ignorewseol': False,
'ignoreblanklines': False, 'ignoreblanklines': False,
'upgrade': False, 'upgrade': False,
'showsimilarity': False, 'showsimilarity': False,
} }
def __init__(self, **opts): def __init__(self, **opts):
opts = pycompat.byteskwargs(opts) opts = pycompat.byteskwargs(opts)
for k in self.defaults.keys(): for k in self.defaults.keys():
def wsclean(opts, text, blank=True): def wsclean(opts, text, blank=True):
if opts.ignorews: if opts.ignorews:
text = bdiff.fixws(text, 1) text = bdiff.fixws(text, 1)
elif opts.ignorewsamount: elif opts.ignorewsamount:
text = bdiff.fixws(text, 0) text = bdiff.fixws(text, 0)
if blank and opts.ignoreblanklines: if blank and opts.ignoreblanklines:
text = re.sub('\n+', '\n', text).strip('\n') text = re.sub('\n+', '\n', text).strip('\n')
if opts.ignorewseol:
text = re.sub(r'[ \t\r\f]+\n', r'\n', text)
yujaUnsubmitted
Not Done

This matches empty lines because \s includes \n.
Perhaps it should be r'[ \t\r]+\n'.

Can you add a test for this bug?

yuja: This matches empty lines because `\s` includes `\n`. Perhaps it should be `r'[ \t\r]+\n'`. Can…
return text return text
def splitblock(base1, lines1, base2, lines2, opts): def splitblock(base1, lines1, base2, lines2, opts):
# The input lines matches except for interwoven blank lines. We # The input lines matches except for interwoven blank lines. We
# transform it into a sequence of matching blocks and blank blocks. # transform it into a sequence of matching blocks and blank blocks.
lines1 = [(wsclean(opts, l) and 1 or 0) for l in lines1] lines1 = [(wsclean(opts, l) and 1 or 0) for l in lines1]
lines2 = [(wsclean(opts, l) and 1 or 0) for l in lines2] lines2 = [(wsclean(opts, l) and 1 or 0) for l in lines2]
s1, e1 = 0, len(lines1) s1, e1 = 0, len(lines1)
line entry. type is '=' for blocks matching exactly one another line entry. type is '=' for blocks matching exactly one another
(bdiff blocks), '!' for non-matching blocks and '~' for blocks (bdiff blocks), '!' for non-matching blocks and '~' for blocks
matching only after having filtered blank lines. matching only after having filtered blank lines.
line1 and line2 are text1 and text2 split with splitnewlines() if line1 and line2 are text1 and text2 split with splitnewlines() if
they are already available. they are already available.
""" """
if opts is None: if opts is None:
opts = defaultopts opts = defaultopts
if opts.ignorews or opts.ignorewsamount: if opts.ignorews or opts.ignorewsamount or opts.ignorewseol:
text1 = wsclean(opts, text1, False) text1 = wsclean(opts, text1, False)
text2 = wsclean(opts, text2, False) text2 = wsclean(opts, text2, False)
diff = bdiff.blocks(text1, text2) diff = bdiff.blocks(text1, text2)
for i, s1 in enumerate(diff): for i, s1 in enumerate(diff):
# The first match is special. # The first match is special.
# we've either found a match starting at line 0 or a match later # we've either found a match starting at line 0 or a match later
# in the file. If it starts later, old and new below will both be # in the file. If it starts later, old and new below will both be
# empty and we'll continue to the next match. # empty and we'll continue to the next match.

mercurial/patch.py

buildopts['index'] = hlen buildopts['index'] = hlen
if whitespace: if whitespace:
buildopts['ignorews'] = get('ignore_all_space', 'ignorews') buildopts['ignorews'] = get('ignore_all_space', 'ignorews')
buildopts['ignorewsamount'] = get('ignore_space_change', buildopts['ignorewsamount'] = get('ignore_space_change',
'ignorewsamount') 'ignorewsamount')
buildopts['ignoreblanklines'] = get('ignore_blank_lines', buildopts['ignoreblanklines'] = get('ignore_blank_lines',
'ignoreblanklines') 'ignoreblanklines')
buildopts['ignorewseol'] = get('ignore_space_at_eol', 'ignorewseol')
if formatchanging: if formatchanging:
buildopts['text'] = opts and opts.get('text') buildopts['text'] = opts and opts.get('text')
binary = None if opts is None else opts.get('binary') binary = None if opts is None else opts.get('binary')
buildopts['nobinary'] = (not binary if binary is not None buildopts['nobinary'] = (not binary if binary is not None
else get('nobinary', forceplain=False)) else get('nobinary', forceplain=False))
buildopts['noprefix'] = get('noprefix', forceplain=False) buildopts['noprefix'] = get('noprefix', forceplain=False)
return mdiff.diffopts(**pycompat.strkwargs(buildopts)) return mdiff.diffopts(**pycompat.strkwargs(buildopts))

tests/test-completion.t

$ hg debugcomplete --options s $ hg debugcomplete --options s
hg: command 's' is ambiguous: hg: command 's' is ambiguous:
serve showconfig status summary serve showconfig status summary
[255] [255]
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, 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, ssh, remotecmd, insecure clone: noupdate, updaterev, rev, branch, pull, uncompressed, 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, 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 forget: include, exclude
init: ssh, remotecmd, insecure init: ssh, remotecmd, insecure
log: follow, follow-first, date, copies, keyword, rev, 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, removed, only-merges, user, only-branch, branch, prune, patch, git, limit, no-merges, stat, graph, style, template, include, exclude
merge: force, rev, preview, tool merge: force, rev, preview, 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

tests/test-diff-ignore-whitespace.t

@@ -1,2 +1,3 @@ @@ -1,2 +1,3 @@
-hello world -hello world
-goodbye world -goodbye world
+hello world\r (esc) +hello world\r (esc)
+\r (esc) +\r (esc)
+goodbye\r (no-eol) (esc) +goodbye\r (no-eol) (esc)
world world
Test \r (carriage return) as used in "DOS" line endings:
$ printf 'hello world \r\n\t\ngoodbye world\n' >foo
$ hg ndiff --ignore-space-at-eol
diff -r 540c40a65b78 foo
--- a/foo
+++ b/foo
@@ -1,2 +1,3 @@
hello world
+\t (esc)
goodbye world
No completely blank lines to ignore: No completely blank lines to ignore:
$ printf 'hello world\r\n\r\ngoodbye\rworld\n' >foo
$ hg ndiff --ignore-blank-lines $ hg ndiff --ignore-blank-lines
diff -r 540c40a65b78 foo diff -r 540c40a65b78 foo
--- a/foo --- a/foo
+++ b/foo +++ b/foo
@@ -1,2 +1,3 @@ @@ -1,2 +1,3 @@
-hello world -hello world
-goodbye world -goodbye world
+hello world\r (esc) +hello world\r (esc)

tests/test-help.t

--binary generate binary diffs in git mode (default) --binary generate binary diffs in git mode (default)
--nodates omit dates from diff headers --nodates omit dates from diff headers
--noprefix omit a/ and b/ prefixes from filenames --noprefix omit a/ and b/ prefixes from filenames
-p --show-function show which function each change is in -p --show-function show which function each change is in
--reverse produce a diff that undoes the changes --reverse produce a diff that undoes the changes
-w --ignore-all-space ignore white space when comparing lines -w --ignore-all-space ignore white space when comparing lines
-b --ignore-space-change ignore changes in the amount of white space -b --ignore-space-change ignore changes in the amount of white space
-B --ignore-blank-lines ignore changes whose lines are all blank -B --ignore-blank-lines ignore changes whose lines are all blank
-Z --ignore-space-at-eol ignore changes in whitespace at EOL
-U --unified NUM number of lines of context to show -U --unified NUM number of lines of context to show
--stat output diffstat-style summary of changes --stat output diffstat-style summary of changes
--root DIR produce diffs relative to subdirectory --root DIR produce diffs relative to subdirectory
-I --include PATTERN [+] include names matching the given patterns -I --include PATTERN [+] include names matching the given patterns
-X --exclude PATTERN [+] exclude names matching the given patterns -X --exclude PATTERN [+] exclude names matching the given patterns
-S --subrepos recurse into subrepositories -S --subrepos recurse into subrepositories
(some details hidden, use --verbose to show complete help) (some details hidden, use --verbose to show complete help)

tests/test-qrecord.t

-m --message TEXT use text as commit message -m --message TEXT use text as commit message
-l --logfile FILE read commit message from file -l --logfile FILE read commit message from file
-d --date DATE record the specified date as commit date -d --date DATE record the specified date as commit date
-u --user USER record the specified user as committer -u --user USER record the specified user as committer
-S --subrepos recurse into subrepositories -S --subrepos recurse into subrepositories
-w --ignore-all-space ignore white space when comparing lines -w --ignore-all-space ignore white space when comparing lines
-b --ignore-space-change ignore changes in the amount of white space -b --ignore-space-change ignore changes in the amount of white space
-B --ignore-blank-lines ignore changes whose lines are all blank -B --ignore-blank-lines ignore changes whose lines are all blank
-Z --ignore-space-at-eol ignore changes in whitespace at EOL
(some details hidden, use --verbose to show complete help) (some details hidden, use --verbose to show complete help)
help (no mq, so no qrecord) help (no mq, so no qrecord)
$ hg help qrecord $ hg help qrecord
hg qrecord [OPTION]... PATCH [FILE]... hg qrecord [OPTION]... PATCH [FILE]...
-d --date DATE add "Date: <DATE>" to patch -d --date DATE add "Date: <DATE>" to patch
-I --include PATTERN [+] include names matching the given patterns -I --include PATTERN [+] include names matching the given patterns
-X --exclude PATTERN [+] exclude names matching the given patterns -X --exclude PATTERN [+] exclude names matching the given patterns
-m --message TEXT use text as commit message -m --message TEXT use text as commit message
-l --logfile FILE read commit message from file -l --logfile FILE read commit message from file
-w --ignore-all-space ignore white space when comparing lines -w --ignore-all-space ignore white space when comparing lines
-b --ignore-space-change ignore changes in the amount of white space -b --ignore-space-change ignore changes in the amount of white space
-B --ignore-blank-lines ignore changes whose lines are all blank -B --ignore-blank-lines ignore changes whose lines are all blank
-Z --ignore-space-at-eol ignore changes in whitespace at EOL
--mq operate on patch repository --mq operate on patch repository
(some details hidden, use --verbose to show complete help) (some details hidden, use --verbose to show complete help)
$ cd a $ cd a
Base commit Base commit

tests/test-record.t

-m --message TEXT use text as commit message -m --message TEXT use text as commit message
-l --logfile FILE read commit message from file -l --logfile FILE read commit message from file
-d --date DATE record the specified date as commit date -d --date DATE record the specified date as commit date
-u --user USER record the specified user as committer -u --user USER record the specified user as committer
-S --subrepos recurse into subrepositories -S --subrepos recurse into subrepositories
-w --ignore-all-space ignore white space when comparing lines -w --ignore-all-space ignore white space when comparing lines
-b --ignore-space-change ignore changes in the amount of white space -b --ignore-space-change ignore changes in the amount of white space
-B --ignore-blank-lines ignore changes whose lines are all blank -B --ignore-blank-lines ignore changes whose lines are all blank
-Z --ignore-space-at-eol ignore changes in whitespace at EOL
(some details hidden, use --verbose to show complete help) (some details hidden, use --verbose to show complete help)
Select no files Select no files
$ touch empty-rw $ touch empty-rw
$ hg add empty-rw $ hg add empty-rw