D2619: revsetlang: add a hint for more useful parse errors

[ryanmce (Ryan McElroy)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHG2a258985ffeb: revsetlang: add a hint for more useful parse 
errors (authored by ryanmce, committed by ).

CHANGED PRIOR TO COMMIT
  https://phab.mercurial-scm.org/D2619?vs=6507=6540#toc

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2619?vs=6507=6540

REVISION DETAIL
  https://phab.mercurial-scm.org/D2619

AFFECTED FILES
  mercurial/revsetlang.py
  tests/test-annotate.t
  tests/test-default-push.t
  tests/test-fileset.t
  tests/test-revset.t
  tests/test-revset2.t

CHANGE DETAILS

diff --git a/tests/test-revset2.t b/tests/test-revset2.t
--- a/tests/test-revset2.t
+++ b/tests/test-revset2.t
@@ -690,6 +690,8 @@
 
   $ log '1 OR 2'
   hg: parse error at 2: invalid token
+  (1 OR 2
+ ^ here)
   [255]
 
 or operator should preserve ordering:
@@ -1562,6 +1564,8 @@
 test error message of bad revset
   $ hg log -r 'foo\\'
   hg: parse error at 3: syntax error in revset 'foo\\'
+  (foo\\
+  ^ here)
   [255]
 
   $ cd ..
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -399,6 +399,8 @@
   4
   $ log 'date(this is a test)'
   hg: parse error at 10: unexpected token: symbol
+  (date(this is a test)
+ ^ here)
   [255]
   $ log 'date()'
   hg: parse error: date requires a string
@@ -408,6 +410,8 @@
   [255]
   $ log 'date('
   hg: parse error at 5: not a prefix: end
+  (date(
+^ here)
   [255]
   $ log 'date("\xy")'
   hg: parse error: invalid \x escape* (glob)
@@ -614,18 +618,28 @@
 
   $ hg debugrevspec '[0]'
   hg: parse error at 0: not a prefix: [
+  ([0]
+   ^ here)
   [255]
   $ hg debugrevspec '.#'
   hg: parse error at 2: not a prefix: end
+  (.#
+ ^ here)
   [255]
   $ hg debugrevspec '#rel'
   hg: parse error at 0: not a prefix: #
+  (#rel
+   ^ here)
   [255]
   $ hg debugrevspec '.#rel[0'
   hg: parse error at 7: unexpected token: end
+  (.#rel[0
+  ^ here)
   [255]
   $ hg debugrevspec '.]'
   hg: parse error at 1: invalid token
+  (.]
+^ here)
   [255]
 
   $ hg debugrevspec '.#generations[a]'
@@ -1330,6 +1344,8 @@
   6
   $ try 'grep(r"\")'
   hg: parse error at 7: unterminated string
+  (grep(r"\")
+  ^ here)
   [255]
   $ log 'head()'
   0
@@ -2774,3 +2790,14 @@
 
   $ cd ..
   $ cd repo
+
+test multiline revset with errors
+
+  $ echo > multiline-revset
+  $ echo '. +' >> multiline-revset
+  $ echo '.^ +' >> multiline-revset
+  $ hg log -r "`cat multiline-revset`"
+  hg: parse error at 9: not a prefix: end
+  ( . + .^ +
+^ here)
+  [255]
diff --git a/tests/test-fileset.t b/tests/test-fileset.t
--- a/tests/test-fileset.t
+++ b/tests/test-fileset.t
@@ -666,7 +666,11 @@
 
   $ fileset "status(' ', '4', added())"
   hg: parse error at 1: not a prefix: end
+  ( 
+^ here)
   [255]
   $ fileset "status('2', ' ', added())"
   hg: parse error at 1: not a prefix: end
+  ( 
+^ here)
   [255]
diff --git a/tests/test-default-push.t b/tests/test-default-push.t
--- a/tests/test-default-push.t
+++ b/tests/test-default-push.t
@@ -142,6 +142,8 @@
   $ hg --config 'paths.default:pushrev=(' push
   pushing to file:/*/$TESTTMP/pushurlsource/../pushurldest (glob)
   hg: parse error at 1: not a prefix: end
+  ((
+^ here)
   [255]
 
   $ cd ..
diff --git a/tests/test-annotate.t b/tests/test-annotate.t
--- a/tests/test-annotate.t
+++ b/tests/test-annotate.t
@@ -814,6 +814,8 @@
   [255]
   $ hg log -r 'followlines(baz, 2:4, startrev=20, descend=[1])'
   hg: parse error at 43: not a prefix: [
+  (followlines(baz, 2:4, startrev=20, descend=[1])
+  ^ here)
   [255]
   $ hg log -r 'followlines(baz, 2:4, startrev=20, descend=a)'
   hg: parse error: descend argument must be a boolean
diff --git a/mercurial/revsetlang.py b/mercurial/revsetlang.py
--- a/mercurial/revsetlang.py
+++ b/mercurial/revsetlang.py
@@ -539,7 +539,21 @@
 return tuple(foldconcat(t) for t in tree)
 
 def parse(spec, lookup=None):
-return _parsewith(spec, lookup=lookup)
+try:
+return _parsewith(spec, lookup=lookup)
+except error.ParseError as inst:
+if len(inst.args) > 1:  # has location
+# Add 1 to location because unlike templates, revset parse errors
+# point to the char where the error happened, not the char after.
+loc = inst.args[1] + 1
+# Remove newlines -- spaces are equivalent whitespace.
+spec = spec.replace('\n', ' ')
+# We want the caret to point to the place in the template that
+# failed to parse, but in a hint we get a open paren at the
+# start. Therefore, we print "loc + 1" spaces (instead of "loc")
+# to line up the caret with the location of the error.
+inst.hint = spec + '\n' + ' ' * loc + '^ ' + _('here')
+raise
 
 def _quote(s):
 r"""Quote a value in order to make it 

D2589: setup: ignore extension load failures when finding working hg

[ryanmce (Ryan McElroy)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHG5f41e3418407: setup: ignore extension load failures when 
finding working hg (authored by ryanmce, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2589?vs=6440=6539

REVISION DETAIL
  https://phab.mercurial-scm.org/D2589

AFFECTED FILES
  setup.py

CHANGE DETAILS

diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -255,6 +255,7 @@
if (not e.startswith(b'not trusting file')
and not e.startswith(b'warning: Not importing')
and not e.startswith(b'obsolete feature not enabled')
+   and not e.startswith(b'*** failed to import extension')
and not e.startswith(b'devel-warn:'))]
 return b'\n'.join(b'  ' + e for e in err)
 



To: ryanmce, #hg-reviewers, lothiraldan
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2608: templater: add hint to template parse errors to help locate issues

[ryanmce (Ryan McElroy)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHG8df20b7a39d2: templater: add hint to template parse errors 
to help locate issues (authored by ryanmce, committed by ).

CHANGED PRIOR TO COMMIT
  https://phab.mercurial-scm.org/D2608?vs=6485=6509#toc

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2608?vs=6485=6509

REVISION DETAIL
  https://phab.mercurial-scm.org/D2608

AFFECTED FILES
  mercurial/templater.py
  tests/test-command-template.t
  tests/test-export.t
  tests/test-log.t

CHANGE DETAILS

diff --git a/tests/test-log.t b/tests/test-log.t
--- a/tests/test-log.t
+++ b/tests/test-log.t
@@ -2289,6 +2289,23 @@
   $ hg --config extensions.names=../names.py log -r 0 --template '{bars}\n'
   foo
 
+Templater parse errors:
+
+simple error
+  $ hg log -r . -T '{shortest(node}'
+  hg: parse error at 15: unexpected token: end
+  ({shortest(node}
+ ^ here)
+  [255]
+
+multi-line template with error
+  $ hg log -r . -T 'line 1
+  > line2
+  > {shortest(node}
+  > line4\nline5'
+  hg: parse error at 28: unexpected token: end
+  [255]
+
   $ cd ..
 
 hg log -f dir across branches
diff --git a/tests/test-export.t b/tests/test-export.t
--- a/tests/test-export.t
+++ b/tests/test-export.t
@@ -218,6 +218,8 @@
   [255]
   $ hg export -o '%m{' tip
   hg: parse error at 3: unterminated template expansion
+  (%m{
+ ^ here)
   [255]
   $ hg export -o '%\' tip
   abort: invalid format spec '%\' in output filename
diff --git a/tests/test-command-template.t b/tests/test-command-template.t
--- a/tests/test-command-template.t
+++ b/tests/test-command-template.t
@@ -2766,19 +2766,29 @@
 
   $ hg log -T '{date'
   hg: parse error at 1: unterminated template expansion
+  ({date
+   ^ here)
   [255]
   $ hg log -T '{date(}'
   hg: parse error at 7: not a prefix: end
+  ({date(}
+ ^ here)
   [255]
   $ hg log -T '{date)}'
   hg: parse error at 5: invalid token
+  ({date)}
+   ^ here)
   [255]
   $ hg log -T '{date date}'
   hg: parse error at 6: invalid token
+  ({date date}
+^ here)
   [255]
 
   $ hg log -T '{}'
   hg: parse error at 2: not a prefix: end
+  ({}
+^ here)
   [255]
   $ hg debugtemplate -v '{()}'
   (template
@@ -2827,10 +2837,14 @@
 
   $ hg log -T '{"date'
   hg: parse error at 2: unterminated string
+  ({"date
+^ here)
   [255]
 
   $ hg log -T '{"foo{date|?}"}'
   hg: parse error at 11: syntax error
+  ({"foo{date|?}"}
+ ^ here)
   [255]
 
 Thrown an error if a template function doesn't exist
@@ -3362,6 +3376,8 @@
   -4
   $ hg debugtemplate '{(-)}\n'
   hg: parse error at 3: not a prefix: )
+  ({(-)}\n
+ ^ here)
   [255]
   $ hg debugtemplate '{(-a)}\n'
   hg: parse error: negation needs an integer argument
@@ -3527,6 +3543,8 @@
   foo
   $ hg log -r 2 -T '{if(rev, "{if(rev, \")}")}\n'
   hg: parse error at 21: unterminated string
+  ({if(rev, "{if(rev, \")}")}\n
+   ^ here)
   [255]
   $ hg log -r 2 -T '{if(rev, \"\\"")}\n'
   hg: parse error: trailing \ in string
diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -213,35 +213,48 @@
 unescape = [parser.unescapestr, pycompat.identity][raw]
 pos = start
 p = parser.parser(elements)
-while pos < stop:
-n = min((tmpl.find(c, pos, stop) for c in sepchars),
-key=lambda n: (n < 0, n))
-if n < 0:
-yield ('string', unescape(tmpl[pos:stop]), pos)
-pos = stop
-break
-c = tmpl[n:n + 1]
-bs = 0  # count leading backslashes
-if not raw:
-bs = (n - pos) - len(tmpl[pos:n].rstrip('\\'))
-if bs % 2 == 1:
-# escaped (e.g. '\{', '\\\{', but not '\\{')
-yield ('string', unescape(tmpl[pos:n - 1]) + c, pos)
-pos = n + 1
-continue
-if n > pos:
-yield ('string', unescape(tmpl[pos:n]), pos)
-if c == quote:
-yield ('end', None, n + 1)
-return
+try:
+while pos < stop:
+n = min((tmpl.find(c, pos, stop) for c in sepchars),
+key=lambda n: (n < 0, n))
+if n < 0:
+yield ('string', unescape(tmpl[pos:stop]), pos)
+pos = stop
+break
+c = tmpl[n:n + 1]
+bs = 0  # count leading backslashes
+if not raw:
+bs = (n - pos) - len(tmpl[pos:n].rstrip('\\'))
+if bs % 2 == 1:
+# escaped (e.g. '\{', '\\\{', but not '\\{')
+yield ('string', unescape(tmpl[pos:n - 1]) + c, pos)
+pos = n + 1
+continue
+if n > pos:
+yield ('string', unescape(tmpl[pos:n]), pos)
+if c == quote:
+yield ('end', None, n + 1)
+return
 
-parseres, pos = 

D2609: templater: provide hint for multi-line templates with parse errors

[ryanmce (Ryan McElroy)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHG5d1bae328b7d: templater: provide hint for multi-line 
templates with parse errors (authored by ryanmce, committed by ).

CHANGED PRIOR TO COMMIT
  https://phab.mercurial-scm.org/D2609?vs=6487=6510#toc

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2609?vs=6487=6510

REVISION DETAIL
  https://phab.mercurial-scm.org/D2609

AFFECTED FILES
  mercurial/templater.py
  tests/test-log.t

CHANGE DETAILS

diff --git a/tests/test-log.t b/tests/test-log.t
--- a/tests/test-log.t
+++ b/tests/test-log.t
@@ -2304,6 +2304,8 @@
   > {shortest(node}
   > line4\nline5'
   hg: parse error at 28: unexpected token: end
+  (line 1\nline2\n{shortest(node}\nline4\nline5
+^ here)
   [255]
 
   $ cd ..
diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -246,14 +246,16 @@
 except error.ParseError as inst:
 if len(inst.args) > 1:  # has location
 loc = inst.args[1]
-# TODO: Opportunity for improvement! If there is a newline in the
-# template, this hint does not point to the right place, so skip.
-if '\n' not in tmpl:
-# We want the caret to point to the place in the template that
-# failed to parse, but in a hint we get a open paren at the
-# start. Therefore, we print "loc" spaces (instead of "loc - 
1")
-# to line up the caret with the location of the error.
-inst.hint = tmpl + '\n' + ' ' * (loc) + '^ ' + _('here')
+# Offset the caret location by the number of newlines before the
+# location of the error, since we will replace one-char newlines
+# with the two-char literal r'\n'.
+offset = tmpl[:loc].count('\n')
+tmpl = tmpl.replace('\n', br'\n')
+# We want the caret to point to the place in the template that
+# failed to parse, but in a hint we get a open paren at the
+# start. Therefore, we print "loc" spaces (instead of "loc - 1")
+# to line up the caret with the location of the error.
+inst.hint = tmpl + '\n' + ' ' * (loc + offset) + '^ ' + _('here')
 raise
 yield ('end', None, pos)
 



To: ryanmce, #hg-reviewers, durin42
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2619: revsetlang: add a hint for more useful parse errors

[ryanmce (Ryan McElroy)]

ryanmce created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This logic is largely based on the similar logic added to template error
  messages in https://phab.mercurial-scm.org/D2608 and 
https://phab.mercurial-scm.org/D2609, but with a few tweaks based on how revsets
  actually work.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2619

AFFECTED FILES
  mercurial/revsetlang.py
  tests/test-annotate.t
  tests/test-default-push.t
  tests/test-fileset.t
  tests/test-revset.t
  tests/test-revset2.t

CHANGE DETAILS

diff --git a/tests/test-revset2.t b/tests/test-revset2.t
--- a/tests/test-revset2.t
+++ b/tests/test-revset2.t
@@ -690,6 +690,8 @@
 
   $ log '1 OR 2'
   hg: parse error at 2: invalid token
+  (1 OR 2
+ ^ here)
   [255]
 
 or operator should preserve ordering:
@@ -1562,6 +1564,8 @@
 test error message of bad revset
   $ hg log -r 'foo\\'
   hg: parse error at 3: syntax error in revset 'foo\\'
+  (foo\\
+  ^ here)
   [255]
 
   $ cd ..
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -399,6 +399,8 @@
   4
   $ log 'date(this is a test)'
   hg: parse error at 10: unexpected token: symbol
+  (date(this is a test)
+ ^ here)
   [255]
   $ log 'date()'
   hg: parse error: date requires a string
@@ -408,6 +410,8 @@
   [255]
   $ log 'date('
   hg: parse error at 5: not a prefix: end
+  (date(
+^ here)
   [255]
   $ log 'date("\xy")'
   hg: parse error: invalid \x escape
@@ -614,18 +618,28 @@
 
   $ hg debugrevspec '[0]'
   hg: parse error at 0: not a prefix: [
+  ([0]
+   ^ here)
   [255]
   $ hg debugrevspec '.#'
   hg: parse error at 2: not a prefix: end
+  (.#
+ ^ here)
   [255]
   $ hg debugrevspec '#rel'
   hg: parse error at 0: not a prefix: #
+  (#rel
+   ^ here)
   [255]
   $ hg debugrevspec '.#rel[0'
   hg: parse error at 7: unexpected token: end
+  (.#rel[0
+  ^ here)
   [255]
   $ hg debugrevspec '.]'
   hg: parse error at 1: invalid token
+  (.]
+^ here)
   [255]
 
   $ hg debugrevspec '.#generations[a]'
@@ -1330,6 +1344,8 @@
   6
   $ try 'grep(r"\")'
   hg: parse error at 7: unterminated string
+  (grep(r"\")
+  ^ here)
   [255]
   $ log 'head()'
   0
@@ -2774,3 +2790,13 @@
 
   $ cd ..
   $ cd repo
+
+test multiline revset with errors
+
+  $ hg log -r '
+  > . +
+  > .^ +'
+  hg: parse error at 9: not a prefix: end
+  ( . + .^ +
+^ here)
+  [255]
diff --git a/tests/test-fileset.t b/tests/test-fileset.t
--- a/tests/test-fileset.t
+++ b/tests/test-fileset.t
@@ -666,7 +666,11 @@
 
   $ fileset "status(' ', '4', added())"
   hg: parse error at 1: not a prefix: end
+  ( 
+^ here)
   [255]
   $ fileset "status('2', ' ', added())"
   hg: parse error at 1: not a prefix: end
+  ( 
+^ here)
   [255]
diff --git a/tests/test-default-push.t b/tests/test-default-push.t
--- a/tests/test-default-push.t
+++ b/tests/test-default-push.t
@@ -142,6 +142,8 @@
   $ hg --config 'paths.default:pushrev=(' push
   pushing to file:/*/$TESTTMP/pushurlsource/../pushurldest (glob)
   hg: parse error at 1: not a prefix: end
+  ((
+^ here)
   [255]
 
   $ cd ..
diff --git a/tests/test-annotate.t b/tests/test-annotate.t
--- a/tests/test-annotate.t
+++ b/tests/test-annotate.t
@@ -814,6 +814,8 @@
   [255]
   $ hg log -r 'followlines(baz, 2:4, startrev=20, descend=[1])'
   hg: parse error at 43: not a prefix: [
+  (followlines(baz, 2:4, startrev=20, descend=[1])
+  ^ here)
   [255]
   $ hg log -r 'followlines(baz, 2:4, startrev=20, descend=a)'
   hg: parse error: descend argument must be a boolean
diff --git a/mercurial/revsetlang.py b/mercurial/revsetlang.py
--- a/mercurial/revsetlang.py
+++ b/mercurial/revsetlang.py
@@ -539,7 +539,21 @@
 return tuple(foldconcat(t) for t in tree)
 
 def parse(spec, lookup=None):
-return _parsewith(spec, lookup=lookup)
+try:
+return _parsewith(spec, lookup=lookup)
+except error.ParseError as inst:
+if len(inst.args) > 1:  # has location
+# Add 1 to location because unlike templates, revset parse errors
+# point to the char where the error happened, not the char after.
+loc = inst.args[1] + 1
+# Remove newlines -- spaces are equivalent whitespace.
+spec = spec.replace('\n', ' ')
+# We want the caret to point to the place in the template that
+# failed to parse, but in a hint we get a open paren at the
+# start. Therefore, we print "loc + 1" spaces (instead of "loc")
+# to line up the caret with the location of the error.
+inst.hint = spec + '\n' + ' ' * loc + '^ ' + _('here')
+raise
 
 def _quote(s):
 r"""Quote a value in order to make it safe for the revset engine.



To: ryanmce, #hg-reviewers
Cc: mercurial-devel

D2609: templater: provide hint for multi-line templates with parse errors

[ryanmce (Ryan McElroy)]

ryanmce updated this revision to Diff 6487.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2609?vs=6486=6487

REVISION DETAIL
  https://phab.mercurial-scm.org/D2609

AFFECTED FILES
  mercurial/templater.py
  tests/test-parse-errors.t

CHANGE DETAILS

diff --git a/tests/test-parse-errors.t b/tests/test-parse-errors.t
--- a/tests/test-parse-errors.t
+++ b/tests/test-parse-errors.t
@@ -16,4 +16,7 @@
   > {shortest(node}
   > line4\nline5'
   hg: parse error at 28: unexpected token: end
+  (line 1\nline2\n{shortest(node}\nline4
+^ here)
   [255]
+
diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -245,14 +245,16 @@
 except error.ParseError as inst:
 if len(inst.args) > 1:  # has location
 loc = inst.args[1]
-# TODO: Opportunity for improvement! If there is a newline in the
-# template, this hint does not point to the right place, so skip.
-if '\n' not in tmpl:
-# We want the caret to point to the place in the template that
-# failed to parse, but in a hint we get a open paren at the
-# start. Therefore, we print "loc" spaces (instead of "loc - 
1")
-# to line up the caret with the location of the error.
-inst.hint = tmpl + '\n' + ' ' * (loc) + '^ ' + _('here')
+# Offset the caret location by the number of newlines before the
+# location of the error, since we will replace one-char newlines
+# with the two-char literal r'\n'.
+offset = tmpl[:loc].count('\n')
+tmpl = tmpl.replace('\n', r'\n')
+# We want the caret to point to the place in the template that
+# failed to parse, but in a hint we get a open paren at the
+# start. Therefore, we print "loc" spaces (instead of "loc - 1")
+# to line up the caret with the location of the error.
+inst.hint = tmpl + '\n' + ' ' * (loc + offset) + '^ ' + _('here')
 raise
 yield ('end', None, pos)
 



To: ryanmce, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2608: templater: add hint to template parse errors to help locate issues

[ryanmce (Ryan McElroy)]

ryanmce updated this revision to Diff 6485.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2608?vs=6484=6485

REVISION DETAIL
  https://phab.mercurial-scm.org/D2608

AFFECTED FILES
  mercurial/templater.py
  tests/test-command-template.t
  tests/test-export.t
  tests/test-parse-errors.t

CHANGE DETAILS

diff --git a/tests/test-parse-errors.t b/tests/test-parse-errors.t
new file mode 100644
--- /dev/null
+++ b/tests/test-parse-errors.t
@@ -0,0 +1,19 @@
+  $ hg init repo
+  $ cd repo
+  $ echo a > a
+  $ hg ci -qAm aa
+
+simple error
+  $ hg log -r . -T '{shortest(node}'
+  hg: parse error at 15: unexpected token: end
+  ({shortest(node}
+ ^ here)
+  [255]
+
+multi-line
+  $ hg log -r . -T 'line 1
+  > line2
+  > {shortest(node}
+  > line4\nline5'
+  hg: parse error at 28: unexpected token: end
+  [255]
diff --git a/tests/test-export.t b/tests/test-export.t
--- a/tests/test-export.t
+++ b/tests/test-export.t
@@ -218,6 +218,8 @@
   [255]
   $ hg export -o '%m{' tip
   hg: parse error at 3: unterminated template expansion
+  (%m{
+ ^ here)
   [255]
   $ hg export -o '%\' tip
   abort: invalid format spec '%\' in output filename
diff --git a/tests/test-command-template.t b/tests/test-command-template.t
--- a/tests/test-command-template.t
+++ b/tests/test-command-template.t
@@ -2766,19 +2766,29 @@
 
   $ hg log -T '{date'
   hg: parse error at 1: unterminated template expansion
+  ({date
+   ^ here)
   [255]
   $ hg log -T '{date(}'
   hg: parse error at 7: not a prefix: end
+  ({date(}
+ ^ here)
   [255]
   $ hg log -T '{date)}'
   hg: parse error at 5: invalid token
+  ({date)}
+   ^ here)
   [255]
   $ hg log -T '{date date}'
   hg: parse error at 6: invalid token
+  ({date date}
+^ here)
   [255]
 
   $ hg log -T '{}'
   hg: parse error at 2: not a prefix: end
+  ({}
+^ here)
   [255]
   $ hg debugtemplate -v '{()}'
   (template
@@ -2827,10 +2837,14 @@
 
   $ hg log -T '{"date'
   hg: parse error at 2: unterminated string
+  ({"date
+^ here)
   [255]
 
   $ hg log -T '{"foo{date|?}"}'
   hg: parse error at 11: syntax error
+  ({"foo{date|?}"}
+ ^ here)
   [255]
 
 Thrown an error if a template function doesn't exist
@@ -3362,6 +3376,8 @@
   -4
   $ hg debugtemplate '{(-)}\n'
   hg: parse error at 3: not a prefix: )
+  ({(-)}\n
+ ^ here)
   [255]
   $ hg debugtemplate '{(-a)}\n'
   hg: parse error: negation needs an integer argument
@@ -3527,6 +3543,8 @@
   foo
   $ hg log -r 2 -T '{if(rev, "{if(rev, \")}")}\n'
   hg: parse error at 21: unterminated string
+  ({if(rev, "{if(rev, \")}")}\n
+   ^ here)
   [255]
   $ hg log -r 2 -T '{if(rev, \"\\"")}\n'
   hg: parse error: trailing \ in string
diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -212,35 +212,48 @@
 unescape = [parser.unescapestr, pycompat.identity][raw]
 pos = start
 p = parser.parser(elements)
-while pos < stop:
-n = min((tmpl.find(c, pos, stop) for c in sepchars),
-key=lambda n: (n < 0, n))
-if n < 0:
-yield ('string', unescape(tmpl[pos:stop]), pos)
-pos = stop
-break
-c = tmpl[n:n + 1]
-bs = 0  # count leading backslashes
-if not raw:
-bs = (n - pos) - len(tmpl[pos:n].rstrip('\\'))
-if bs % 2 == 1:
-# escaped (e.g. '\{', '\\\{', but not '\\{')
-yield ('string', unescape(tmpl[pos:n - 1]) + c, pos)
-pos = n + 1
-continue
-if n > pos:
-yield ('string', unescape(tmpl[pos:n]), pos)
-if c == quote:
-yield ('end', None, n + 1)
-return
+try:
+while pos < stop:
+n = min((tmpl.find(c, pos, stop) for c in sepchars),
+key=lambda n: (n < 0, n))
+if n < 0:
+yield ('string', unescape(tmpl[pos:stop]), pos)
+pos = stop
+break
+c = tmpl[n:n + 1]
+bs = 0  # count leading backslashes
+if not raw:
+bs = (n - pos) - len(tmpl[pos:n].rstrip('\\'))
+if bs % 2 == 1:
+# escaped (e.g. '\{', '\\\{', but not '\\{')
+yield ('string', unescape(tmpl[pos:n - 1]) + c, pos)
+pos = n + 1
+continue
+if n > pos:
+yield ('string', unescape(tmpl[pos:n]), pos)
+if c == quote:
+yield ('end', None, n + 1)
+return
 
-parseres, pos = p.parse(tokenize(tmpl, n + 1, stop, '}'))
-if not tmpl.endswith('}', n + 1, pos):
-raise error.ParseError(_("invalid token"), pos)
-yield ('template', parseres, n)
+parseres, pos = p.parse(tokenize(tmpl, n + 1, stop, '}'))
+if not tmpl.endswith('}', n + 1, pos):
+raise 

D2609: templater: provide hint for multi-line templates with parse errors

[ryanmce (Ryan McElroy)]

ryanmce created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Previously, we punted. Now we "rewrite" the template's newlines to r'\n' and
  offset the hint appropriately.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2609

AFFECTED FILES
  mercurial/templater.py
  tests/test-parse-errors.t

CHANGE DETAILS

diff --git a/tests/test-parse-errors.t b/tests/test-parse-errors.t
--- a/tests/test-parse-errors.t
+++ b/tests/test-parse-errors.t
@@ -16,4 +16,7 @@
   > {shortest(node}
   > line4\nline5'
   hg: parse error at 28: unexpected token: end
+  (line 1\nline2\n{shortest(node}\nline4
+^ here)
   [255]
+
diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -245,14 +245,16 @@
 except error.ParseError as inst:
 if len(inst.args) > 1:  # has location
 loc = inst.args[1]
-# TODO: Opportunity for improvement! If there is a newline in the
-# template, this hint does not point to the right place, so skip.
-if '\n' not in tmpl:
-# We want the caret to point to the place in the template that
-# failed to parse, but in a hint we get a open paren at the
-# start. Therefore, we print "loc" spaces (instead of "loc - 
1")
-# to line up the caret with the location of the error.
-inst.hint = tmpl + '\n' + ' ' * (loc) + '^ ' + _('here')
+# if there are newlines in front of the location of the error,
+# offset the caret location by that number, since we will next
+# replace newlines with the two-char literal r'\n'.
+offset = tmpl[:loc].count('\n')
+tmpl = tmpl.replace('\n', r'\n')
+# We want the caret to point to the place in the template that
+# failed to parse, but in a hint we get a open paren at the
+# start. Therefore, we print "loc" spaces (instead of "loc - 1")
+# to line up the caret with the location of the error.
+inst.hint = tmpl + '\n' + ' ' * (loc + offset) + '^ ' + _('here')
 raise
 yield ('end', None, pos)
 



To: ryanmce, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2608: templater: add hint to template parse errors to help locate issues

[ryanmce (Ryan McElroy)]

ryanmce updated this revision to Diff 6484.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2608?vs=6483=6484

REVISION DETAIL
  https://phab.mercurial-scm.org/D2608

AFFECTED FILES
  mercurial/templater.py
  tests/test-command-template.t
  tests/test-export.t
  tests/test-parse-errors.t

CHANGE DETAILS

diff --git a/tests/test-parse-errors.t b/tests/test-parse-errors.t
new file mode 100644
--- /dev/null
+++ b/tests/test-parse-errors.t
@@ -0,0 +1,21 @@
+  $ hg init repo
+  $ cd repo
+  $ echo a > a
+  $ hg ci -qAm aa
+
+simple error
+
+  $ hg log -r . -T '{shortest(node}'
+  hg: parse error at 15: unexpected token: end
+  ({shortest(node}
+ ^ here)
+  [255]
+
+multi-line
+
+  $ hg log -r . -T 'line 1
+  > line2
+  > {shortest(node}
+  > line4'
+  hg: parse error at 28: unexpected token: end
+  [255]
diff --git a/tests/test-export.t b/tests/test-export.t
--- a/tests/test-export.t
+++ b/tests/test-export.t
@@ -218,6 +218,8 @@
   [255]
   $ hg export -o '%m{' tip
   hg: parse error at 3: unterminated template expansion
+  (%m{
+ ^ here)
   [255]
   $ hg export -o '%\' tip
   abort: invalid format spec '%\' in output filename
diff --git a/tests/test-command-template.t b/tests/test-command-template.t
--- a/tests/test-command-template.t
+++ b/tests/test-command-template.t
@@ -2766,19 +2766,29 @@
 
   $ hg log -T '{date'
   hg: parse error at 1: unterminated template expansion
+  ({date
+   ^ here)
   [255]
   $ hg log -T '{date(}'
   hg: parse error at 7: not a prefix: end
+  ({date(}
+ ^ here)
   [255]
   $ hg log -T '{date)}'
   hg: parse error at 5: invalid token
+  ({date)}
+   ^ here)
   [255]
   $ hg log -T '{date date}'
   hg: parse error at 6: invalid token
+  ({date date}
+^ here)
   [255]
 
   $ hg log -T '{}'
   hg: parse error at 2: not a prefix: end
+  ({}
+^ here)
   [255]
   $ hg debugtemplate -v '{()}'
   (template
@@ -2827,10 +2837,14 @@
 
   $ hg log -T '{"date'
   hg: parse error at 2: unterminated string
+  ({"date
+^ here)
   [255]
 
   $ hg log -T '{"foo{date|?}"}'
   hg: parse error at 11: syntax error
+  ({"foo{date|?}"}
+ ^ here)
   [255]
 
 Thrown an error if a template function doesn't exist
@@ -3362,6 +3376,8 @@
   -4
   $ hg debugtemplate '{(-)}\n'
   hg: parse error at 3: not a prefix: )
+  ({(-)}\n
+ ^ here)
   [255]
   $ hg debugtemplate '{(-a)}\n'
   hg: parse error: negation needs an integer argument
@@ -3527,6 +3543,8 @@
   foo
   $ hg log -r 2 -T '{if(rev, "{if(rev, \")}")}\n'
   hg: parse error at 21: unterminated string
+  ({if(rev, "{if(rev, \")}")}\n
+   ^ here)
   [255]
   $ hg log -r 2 -T '{if(rev, \"\\"")}\n'
   hg: parse error: trailing \ in string
diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -212,35 +212,48 @@
 unescape = [parser.unescapestr, pycompat.identity][raw]
 pos = start
 p = parser.parser(elements)
-while pos < stop:
-n = min((tmpl.find(c, pos, stop) for c in sepchars),
-key=lambda n: (n < 0, n))
-if n < 0:
-yield ('string', unescape(tmpl[pos:stop]), pos)
-pos = stop
-break
-c = tmpl[n:n + 1]
-bs = 0  # count leading backslashes
-if not raw:
-bs = (n - pos) - len(tmpl[pos:n].rstrip('\\'))
-if bs % 2 == 1:
-# escaped (e.g. '\{', '\\\{', but not '\\{')
-yield ('string', unescape(tmpl[pos:n - 1]) + c, pos)
-pos = n + 1
-continue
-if n > pos:
-yield ('string', unescape(tmpl[pos:n]), pos)
-if c == quote:
-yield ('end', None, n + 1)
-return
+try:
+while pos < stop:
+n = min((tmpl.find(c, pos, stop) for c in sepchars),
+key=lambda n: (n < 0, n))
+if n < 0:
+yield ('string', unescape(tmpl[pos:stop]), pos)
+pos = stop
+break
+c = tmpl[n:n + 1]
+bs = 0  # count leading backslashes
+if not raw:
+bs = (n - pos) - len(tmpl[pos:n].rstrip('\\'))
+if bs % 2 == 1:
+# escaped (e.g. '\{', '\\\{', but not '\\{')
+yield ('string', unescape(tmpl[pos:n - 1]) + c, pos)
+pos = n + 1
+continue
+if n > pos:
+yield ('string', unescape(tmpl[pos:n]), pos)
+if c == quote:
+yield ('end', None, n + 1)
+return
 
-parseres, pos = p.parse(tokenize(tmpl, n + 1, stop, '}'))
-if not tmpl.endswith('}', n + 1, pos):
-raise error.ParseError(_("invalid token"), pos)
-yield ('template', parseres, n)
+parseres, pos = p.parse(tokenize(tmpl, n + 1, stop, '}'))
+if not tmpl.endswith('}', n + 1, pos):
+raise 

D2608: templater: add hint to template parse errors to help locate issues

[ryanmce (Ryan McElroy)]

ryanmce created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Previously, we would print the error name and location, but this isn't as
  helpful as we can be. Let's add a hint that shows the location where we
  encountered the parse error.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2608

AFFECTED FILES
  mercurial/templater.py
  tests/test-command-template.t
  tests/test-export.t

CHANGE DETAILS

diff --git a/tests/test-export.t b/tests/test-export.t
--- a/tests/test-export.t
+++ b/tests/test-export.t
@@ -218,6 +218,8 @@
   [255]
   $ hg export -o '%m{' tip
   hg: parse error at 3: unterminated template expansion
+  (%m{
+ ^)
   [255]
   $ hg export -o '%\' tip
   abort: invalid format spec '%\' in output filename
diff --git a/tests/test-command-template.t b/tests/test-command-template.t
--- a/tests/test-command-template.t
+++ b/tests/test-command-template.t
@@ -2766,19 +2766,29 @@
 
   $ hg log -T '{date'
   hg: parse error at 1: unterminated template expansion
+  ({date
+   ^)
   [255]
   $ hg log -T '{date(}'
   hg: parse error at 7: not a prefix: end
+  ({date(}
+ ^)
   [255]
   $ hg log -T '{date)}'
   hg: parse error at 5: invalid token
+  ({date)}
+   ^)
   [255]
   $ hg log -T '{date date}'
   hg: parse error at 6: invalid token
+  ({date date}
+^)
   [255]
 
   $ hg log -T '{}'
   hg: parse error at 2: not a prefix: end
+  ({}
+^)
   [255]
   $ hg debugtemplate -v '{()}'
   (template
@@ -2827,10 +2837,14 @@
 
   $ hg log -T '{"date'
   hg: parse error at 2: unterminated string
+  ({"date
+^)
   [255]
 
   $ hg log -T '{"foo{date|?}"}'
   hg: parse error at 11: syntax error
+  ({"foo{date|?}"}
+ ^)
   [255]
 
 Thrown an error if a template function doesn't exist
@@ -3362,6 +3376,8 @@
   -4
   $ hg debugtemplate '{(-)}\n'
   hg: parse error at 3: not a prefix: )
+  ({(-)}\n
+ ^)
   [255]
   $ hg debugtemplate '{(-a)}\n'
   hg: parse error: negation needs an integer argument
@@ -3527,6 +3543,8 @@
   foo
   $ hg log -r 2 -T '{if(rev, "{if(rev, \")}")}\n'
   hg: parse error at 21: unterminated string
+  ({if(rev, "{if(rev, \")}")}\n
+   ^)
   [255]
   $ hg log -r 2 -T '{if(rev, \"\\"")}\n'
   hg: parse error: trailing \ in string
diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -212,35 +212,45 @@
 unescape = [parser.unescapestr, pycompat.identity][raw]
 pos = start
 p = parser.parser(elements)
-while pos < stop:
-n = min((tmpl.find(c, pos, stop) for c in sepchars),
-key=lambda n: (n < 0, n))
-if n < 0:
-yield ('string', unescape(tmpl[pos:stop]), pos)
-pos = stop
-break
-c = tmpl[n:n + 1]
-bs = 0  # count leading backslashes
-if not raw:
-bs = (n - pos) - len(tmpl[pos:n].rstrip('\\'))
-if bs % 2 == 1:
-# escaped (e.g. '\{', '\\\{', but not '\\{')
-yield ('string', unescape(tmpl[pos:n - 1]) + c, pos)
-pos = n + 1
-continue
-if n > pos:
-yield ('string', unescape(tmpl[pos:n]), pos)
-if c == quote:
-yield ('end', None, n + 1)
-return
+try:
+while pos < stop:
+n = min((tmpl.find(c, pos, stop) for c in sepchars),
+key=lambda n: (n < 0, n))
+if n < 0:
+yield ('string', unescape(tmpl[pos:stop]), pos)
+pos = stop
+break
+c = tmpl[n:n + 1]
+bs = 0  # count leading backslashes
+if not raw:
+bs = (n - pos) - len(tmpl[pos:n].rstrip('\\'))
+if bs % 2 == 1:
+# escaped (e.g. '\{', '\\\{', but not '\\{')
+yield ('string', unescape(tmpl[pos:n - 1]) + c, pos)
+pos = n + 1
+continue
+if n > pos:
+yield ('string', unescape(tmpl[pos:n]), pos)
+if c == quote:
+yield ('end', None, n + 1)
+return
 
-parseres, pos = p.parse(tokenize(tmpl, n + 1, stop, '}'))
-if not tmpl.endswith('}', n + 1, pos):
-raise error.ParseError(_("invalid token"), pos)
-yield ('template', parseres, n)
+parseres, pos = p.parse(tokenize(tmpl, n + 1, stop, '}'))
+if not tmpl.endswith('}', n + 1, pos):
+raise error.ParseError(_("invalid token"), pos)
+yield ('template', parseres, n)
 
-if quote:
-raise error.ParseError(_("unterminated string"), start)
+if quote:
+raise error.ParseError(_("unterminated string"), start)
+except error.ParseError as inst:
+if len(inst.args) > 1:  # has location
+loc = inst.args[1]
+# We want the caret to 

D2572: xdiff: vendor xdiff library from git

[ryanmce (Ryan McElroy)]

ryanmce planned changes to this revision.
ryanmce added a comment.


  This existing directory is called `thirdparty` not `third-party`, 
unfortunately.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2572

To: ryanmce, #hg-reviewers
Cc: quark, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2068: revlog: do not use delta for lfs revisions

[ryanmce (Ryan McElroy)]

ryanmce added inline comments.

INLINE COMMENTS

> quark wrote in revlog.py:2110
> `candelta` takes two revisions. Here there is only one revision. It's 
> possible to pass a useless revision but that has unnecessary overhead.
> 
> `candelta` also fetches `flags`, here we already know the values `flags` so 
> it's faster to avoid fetching it again.

Then why is this `REVIDX_KNOWN_FLAGS` and not `REVIDX_RAWTEXT_CHANGING_FLAGS`?

Given the number of questions here from me and others, I think the comment 
could use extension at least so future readers understand why this is the way 
it is.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2068

To: quark, indygreg, #hg-reviewers, ryanmce
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2067: changegroup: do not delta lfs revisions

[ryanmce (Ryan McElroy)]

ryanmce added inline comments.

INLINE COMMENTS

> revlog.py:746
> +def candelta(self, baserev, rev):
> +"""whether two revisions (prev, rev) can be delta-ed or not"""
> +# Disable delta if either rev requires a content-changing flag

`prev` should be `baserev`?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2067

To: quark, indygreg, #hg-reviewers, ryanmce
Cc: martinvonz, durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2068: revlog: do not use delta for lfs revisions

[ryanmce (Ryan McElroy)]

ryanmce added inline comments.

INLINE COMMENTS

> revlog.py:2110
> +# no delta for flag processor revision (see "candelta" for why)
> +if flags & REVIDX_KNOWN_FLAGS:
> +deltainfo = None

Why is this not `candelta()`?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2068

To: quark, indygreg, #hg-reviewers, ryanmce
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2577: mdiff: prefer xdiff for diff calculation

[ryanmce (Ryan McElroy)]

ryanmce added inline comments.

INLINE COMMENTS

> quark wrote in mdiff.py:34
> The diff functions below do not have an `ui` object. Would you be okay if the 
> config is global? ex. something like
> 
>   # mdiff.py
>   def setdiffalgo(name):
>   global blocks
>   if name == 'xdiff':
>   blocks = ...
>   else: ...
> 
> 
> 
>   # dispatch.py
>   ui = 
>   mdiff.setdiffalgo(...)

I chatted with people here at the sprint. I'm going to see if we can pass in 
the diff algo in from calling functions, falling back to bdiff if none is 
specified.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2577

To: ryanmce, #hg-reviewers
Cc: quark, durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2589: setup: ignore extension load failures when finding working hg

[ryanmce (Ryan McElroy)]

ryanmce created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Previously, `make local` would fail if any extension was not properly loading.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2589

AFFECTED FILES
  setup.py

CHANGE DETAILS

diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -255,6 +255,7 @@
if (not e.startswith(b'not trusting file')
and not e.startswith(b'warning: Not importing')
and not e.startswith(b'obsolete feature not enabled')
+   and not e.startswith(b'*** failed to import extension')
and not e.startswith(b'devel-warn:'))]
 return b'\n'.join(b'  ' + e for e in err)
 



To: ryanmce, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2572: xdiff: vendor xdiff library from git

[ryanmce (Ryan McElroy)]

ryanmce added a subscriber: quark.
ryanmce added a comment.


  To be clear, this patch series was created by @quark, but phabricator did not 
keep the author information from the patches.
  
  I've ported it here to generate discussion about this as a path forward.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2572

To: ryanmce, #hg-reviewers
Cc: quark, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2573: xdiff: remove patience and histogram diffs

[ryanmce (Ryan McElroy)]

ryanmce created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  They are greedy algorithms that yields suboptimal results. Patience diff has
  been advertised as "slower, but generating better results sometimes" for a
  long time. But it's in theory "faster if there are common prefix/suffix
  and/or unique lines, could generate suboptimal results". As demonstrated by
  this test case:
  
open('a', 'w').write('\n'.join(list('a' + 'x' * 300 + 'u' + 'x' * 700 + 
'a\n')))
open('b', 'w').write('\n'.join(list('b' + 'x' * 700 + 'u' + 'x' * 300 + 
'b\n')))
  
  For generating "better" results, the diff sliding heuristic [1] is a better
  solution in general.
  
  So let's just remove patience diff and its variant histogram diff to
  simplify the code.
  
  [1]: 
https://github.com/git/git/commit/433860f3d0beb0c6f205290bd16cda413148f098

TEST PLAN
  `gcc -fPIC *.c --shared -o xdiff.so` still builds.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2573

AFFECTED FILES
  mercurial/thirdparty/xdiff/xdiff.h
  mercurial/thirdparty/xdiff/xdiffi.c
  mercurial/thirdparty/xdiff/xprepare.c

CHANGE DETAILS

diff --git a/mercurial/thirdparty/xdiff/xprepare.c 
b/mercurial/thirdparty/xdiff/xprepare.c
--- a/mercurial/thirdparty/xdiff/xprepare.c
+++ b/mercurial/thirdparty/xdiff/xprepare.c
@@ -27,7 +27,6 @@
 #define XDL_MAX_EQLIMIT 1024
 #define XDL_SIMSCAN_WINDOW 100
 #define XDL_GUESS_NLINES1 256
-#define XDL_GUESS_NLINES2 20
 
 
 typedef struct s_xdlclass {
@@ -181,9 +180,7 @@
if (!(recs = (xrecord_t **) xdl_malloc(narec * sizeof(xrecord_t *
goto abort;
 
-   if (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF)
-   hbits = hsize = 0;
-   else {
+   {
hbits = xdl_hashbits((unsigned int) narec);
hsize = 1 << hbits;
if (!(rhash = (xrecord_t **) xdl_malloc(hsize * 
sizeof(xrecord_t *
@@ -209,8 +206,7 @@
crec->ha = hav;
recs[nrec++] = crec;
 
-   if ((XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF) &&
-   xdl_classify_record(pass, cf, rhash, hbits, crec) < 
0)
+   if (xdl_classify_record(pass, cf, rhash, hbits, crec) < 
0)
goto abort;
}
}
@@ -266,21 +262,12 @@
 
memset(, 0, sizeof(cf));
 
-   /*
-* For histogram diff, we can afford a smaller sample size and
-* thus a poorer estimate of the number of lines, as the hash
-* table (rhash) won't be filled up/grown. The number of lines
-* (nrecs) will be updated correctly anyway by
-* xdl_prepare_ctx().
-*/
-   sample = (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF
- ? XDL_GUESS_NLINES2 : XDL_GUESS_NLINES1);
+   sample = XDL_GUESS_NLINES1;
 
enl1 = xdl_guess_lines(mf1, sample) + 1;
enl2 = xdl_guess_lines(mf2, sample) + 1;
 
-   if (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF &&
-   xdl_init_classifier(, enl1 + enl2 + 1, xpp->flags) < 0)
+   if (xdl_init_classifier(, enl1 + enl2 + 1, xpp->flags) < 0)
return -1;
 
if (xdl_prepare_ctx(1, mf1, enl1, xpp, , >xdf1) < 0) {
@@ -295,18 +282,14 @@
return -1;
}
 
-   if ((XDF_DIFF_ALG(xpp->flags) != XDF_PATIENCE_DIFF) &&
-   (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF) &&
-   xdl_optimize_ctxs(, >xdf1, >xdf2) < 0) {
-
+   if (xdl_optimize_ctxs(, >xdf1, >xdf2) < 0) {
xdl_free_ctx(>xdf2);
xdl_free_ctx(>xdf1);
xdl_free_classifier();
return -1;
}
 
-   if (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF)
-   xdl_free_classifier();
+   xdl_free_classifier();
 
return 0;
 }
diff --git a/mercurial/thirdparty/xdiff/xdiffi.c 
b/mercurial/thirdparty/xdiff/xdiffi.c
--- a/mercurial/thirdparty/xdiff/xdiffi.c
+++ b/mercurial/thirdparty/xdiff/xdiffi.c
@@ -328,12 +328,6 @@
xdalgoenv_t xenv;
diffdata_t dd1, dd2;
 
-   if (XDF_DIFF_ALG(xpp->flags) == XDF_PATIENCE_DIFF)
-   return xdl_do_patience_diff(mf1, mf2, xpp, xe);
-
-   if (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF)
-   return xdl_do_histogram_diff(mf1, mf2, xpp, xe);
-
if (xdl_prepare_env(mf1, mf2, xpp, xe) < 0) {
 
return -1;
diff --git a/mercurial/thirdparty/xdiff/xdiff.h 
b/mercurial/thirdparty/xdiff/xdiff.h
--- a/mercurial/thirdparty/xdiff/xdiff.h
+++ b/mercurial/thirdparty/xdiff/xdiff.h
@@ -27,6 +27,8 @@
 extern "C" {
 #endif /* #ifdef __cplusplus */
 
+#include  /* size_t */
+
 /* xpparm_t.flags */
 #define XDF_NEED_MINIMAL (1 << 0)
 
@@ -41,11 +43,6 @@
 
 #define XDF_IGNORE_BLANK_LINES (1 << 7)
 
-#define XDF_PATIENCE_DIFF (1 << 14)
-#define XDF_HISTOGRAM_DIFF (1 

D2575: xdiff: add a bdiff hunk mode

[ryanmce (Ryan McElroy)]

ryanmce created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  xdiff generated hunks for the differences (ex. questionmarks in the `@@ -?,?
  +?,? @@` part from `diff --git` output). However, bdiff generates matched
  hunks instead.
  
  This patch adds a `XDL_EMIT_BDIFFHUNK` flag used by the output function
  `xdl_call_hunk_func`.  Once set, xdiff will generate bdiff-like hunks
  instead.
  
  Note that since `bdiff('', '')` returns `[(0, 0, 0, 0)]`, the shortcut path
  `if (xscr)` is removed. I have checked functions called with `xscr` argument
  (`xdl_mark_ignorable`, `xdl_call_hunk_func`, `xdl_emit_diff`,
  `xdl_free_script`) work just fine with `xscr = NULL`.

TEST PLAN
  `cd contrib/xdiff; make` - still builds. Actual logic tested in the next
  patch.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2575

AFFECTED FILES
  mercurial/thirdparty/xdiff/xdiff.h
  mercurial/thirdparty/xdiff/xdiffi.c

CHANGE DETAILS

diff --git a/mercurial/thirdparty/xdiff/xdiffi.c 
b/mercurial/thirdparty/xdiff/xdiffi.c
--- a/mercurial/thirdparty/xdiff/xdiffi.c
+++ b/mercurial/thirdparty/xdiff/xdiffi.c
@@ -975,15 +975,32 @@
  xdemitconf_t const *xecfg)
 {
xdchange_t *xch, *xche;
-
-   for (xch = xscr; xch; xch = xche->next) {
-   xche = xdl_get_hunk(, xecfg);
-   if (!xch)
-   break;
-   if (xecfg->hunk_func(xch->i1, xche->i1 + xche->chg1 - xch->i1,
-xch->i2, xche->i2 + xche->chg2 - xch->i2,
-ecb->priv) < 0)
+   if ((xecfg->flags & XDL_EMIT_BDIFFHUNK) != 0) {
+   long i1 = 0, i2 = 0, n1 = xe->xdf1.nrec, n2 = xe->xdf2.nrec;
+   for (xch = xscr; xch; xch = xche->next) {
+   xche = xdl_get_hunk(, xecfg);
+   if (!xch)
+   break;
+   if (xch->i1 > i1 || xch->i2 > i2) {
+   if (xecfg->hunk_func(i1, xch->i1, i2, xch->i2, 
ecb->priv) < 0)
+   return -1;
+   }
+   i1 = xche->i1 + xche->chg1;
+   i2 = xche->i2 + xche->chg2;
+   }
+   if (xecfg->hunk_func(i1, n1, i2, n2, ecb->priv) < 0)
return -1;
+   } else {
+   for (xch = xscr; xch; xch = xche->next) {
+   xche = xdl_get_hunk(, xecfg);
+   if (!xch)
+   break;
+   if (xecfg->hunk_func(
+   xch->i1, xche->i1 + xche->chg1 - 
xch->i1,
+   xch->i2, xche->i2 + xche->chg2 - 
xch->i2,
+   ecb->priv) < 0)
+   return -1;
+   }
}
return 0;
 }
@@ -1026,18 +1043,15 @@
xdl_free_env();
return -1;
}
-   if (xscr) {
-   if (xpp->flags & XDF_IGNORE_BLANK_LINES)
-   xdl_mark_ignorable(xscr, , xpp->flags);
 
-   if (ef(, xscr, ecb, xecfg) < 0) {
-
-   xdl_free_script(xscr);
-   xdl_free_env();
-   return -1;
-   }
+   if (xpp->flags & XDF_IGNORE_BLANK_LINES)
+   xdl_mark_ignorable(xscr, , xpp->flags);
+   if (ef(, xscr, ecb, xecfg) < 0) {
xdl_free_script(xscr);
+   xdl_free_env();
+   return -1;
}
+   xdl_free_script(xscr);
xdl_free_env();
 
return 0;
diff --git a/mercurial/thirdparty/xdiff/xdiff.h 
b/mercurial/thirdparty/xdiff/xdiff.h
--- a/mercurial/thirdparty/xdiff/xdiff.h
+++ b/mercurial/thirdparty/xdiff/xdiff.h
@@ -48,6 +48,9 @@
 /* xdemitconf_t.flags */
 #define XDL_EMIT_FUNCNAMES (1 << 0)
 #define XDL_EMIT_FUNCCONTEXT (1 << 2)
+/* emit bdiff-style "matched" (a1, a2, b1, b2) hunks instead of "different"
+ * (a1, a2 - a1, b1, b2 - b1) hunks */
+#define XDL_EMIT_BDIFFHUNK (1 << 4)
 
 #define XDL_MMB_READONLY (1 << 0)
 



To: ryanmce, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2578: xdiff: enable indent heuristic unconditionally

[ryanmce (Ryan McElroy)]

ryanmce created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Enable the indent heuristic feature unconditionally, since it provides nice
  visual improvements. See the added test, and [1].
  
  [1]: 
https://github.com/git/git/commit/433860f3d0beb0c6f205290bd16cda413148f098

TEST PLAN
  Added a test case.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2578

AFFECTED FILES
  mercurial/thirdparty/xdiff.c
  tests/test-diff-indent-heuristic.t

CHANGE DETAILS

diff --git a/tests/test-diff-indent-heuristic.t 
b/tests/test-diff-indent-heuristic.t
new file mode 100644
--- /dev/null
+++ b/tests/test-diff-indent-heuristic.t
@@ -0,0 +1,227 @@
+#require xdiff
+
+Test cases are most from git t/t4061-diff-indent.sh, and commit messages.
+
+  $ hg init
+
+  $ cat > a.c <<'EOF'
+  > int a()
+  > {
+  >   return 1;
+  > }
+  > 
+  > int c()
+  > {
+  >   return 3;
+  > }
+  > EOF
+
+  $ cat > partial.pl <<'EOF'
+  > }
+  > 
+  > if (!$smtp_server) {
+  >foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
+  >if (-x $_) {
+  >$smtp_server = $_;
+  > EOF
+
+  $ cat > c.rb <<'EOF'
+  > ["foo", "bar", "baz"].map do |i|
+  >   i.upcase
+  > end
+  > EOF
+
+  $ cat > d.rb <<'EOF'
+  > def foo
+  >   do_foo_stuff()
+  > 
+  >   common_ending()
+  > end
+  > EOF
+
+  $ cat > spaces.txt <<'EOF'
+  > 1
+  > 2
+  > a
+  > 
+  > b
+  > 3
+  > 4
+  > EOF
+
+  $ cat > functions.c <<'EOF'
+  > 1
+  > 2
+  > /* function */
+  > foo() {
+  > foo
+  > }
+  > 
+  > 3
+  > 4
+  > EOF
+
+  $ hg commit -m 1 -A . -q
+
+  $ cat > a.c <<'EOF'
+  > int a()
+  > {
+  >   return 1;
+  > }
+  > 
+  > int b()
+  > {
+  >   return 2;
+  > }
+  > 
+  > int c()
+  > {
+  >   return 3;
+  > }
+  > EOF
+
+  $ cat > partial.pl <<'EOF'
+  > }
+  > 
+  > if (!$smtp_server) {
+  >$smtp_server = $repo->config('sendemail.smtpserver');
+  > }
+  > if (!$smtp_server) {
+  >foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
+  >if (-x $_) {
+  >$smtp_server = $_;
+  > EOF
+
+  $ cat > c.rb <<'EOF'
+  > ["foo", "bar", "baz"].map do |i|
+  >   i
+  > end
+  >  
+  > ["foo", "bar", "baz"].map do |i|
+  >   i.upcase
+  > end
+  > EOF
+
+  $ cat > d.rb <<'EOF'
+  > def foo
+  >   do_foo_stuff()
+  > 
+  >   common_ending()
+  > end
+  > 
+  > def bar
+  >   do_bar_stuff()
+  > 
+  >   common_ending()
+  > end
+  > EOF
+
+  $ cat > spaces.txt <<'EOF'
+  > 1
+  > 2
+  > a
+  > 
+  > b
+  > a
+  > 
+  > b
+  > 3
+  > 4
+  > EOF
+
+  $ cat > functions.c <<'EOF'
+  > 1
+  > 2
+  > /* function */
+  > bar() {
+  > foo
+  > }
+  > 
+  > /* function */
+  > foo() {
+  > foo
+  > }
+  > 
+  > 3
+  > 4
+  > EOF
+
+  $ hg diff --git
+  diff --git a/a.c b/a.c
+  --- a/a.c
+  +++ b/a.c
+  @@ -3,6 +3,11 @@
+ return 1;
+   }
+   
+  +int b()
+  +{
+  +  return 2;
+  +}
+  +
+   int c()
+   {
+ return 3;
+  diff --git a/c.rb b/c.rb
+  --- a/c.rb
+  +++ b/c.rb
+  @@ -1,3 +1,7 @@
+  +["foo", "bar", "baz"].map do |i|
+  +  i
+  +end
+  + 
+   ["foo", "bar", "baz"].map do |i|
+ i.upcase
+   end
+  diff --git a/d.rb b/d.rb
+  --- a/d.rb
+  +++ b/d.rb
+  @@ -3,3 +3,9 @@
+   
+ common_ending()
+   end
+  +
+  +def bar
+  +  do_bar_stuff()
+  +
+  +  common_ending()
+  +end
+  diff --git a/functions.c b/functions.c
+  --- a/functions.c
+  +++ b/functions.c
+  @@ -1,5 +1,10 @@
+   1
+   2
+  +/* function */
+  +bar() {
+  +foo
+  +}
+  +
+   /* function */
+   foo() {
+   foo
+  diff --git a/partial.pl b/partial.pl
+  --- a/partial.pl
+  +++ b/partial.pl
+  @@ -1,5 +1,8 @@
+   }
+   
+  +if (!$smtp_server) {
+  +   $smtp_server = $repo->config('sendemail.smtpserver');
+  +}
+   if (!$smtp_server) {
+  foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
+  if (-x $_) {
+  diff --git a/spaces.txt b/spaces.txt
+  --- a/spaces.txt
+  +++ b/spaces.txt
+  @@ -2,6 +2,9 @@
+   2
+   a
+   
+  +b
+  +a
+  +
+   b
+   3
+   4
diff --git a/mercurial/thirdparty/xdiff.c b/mercurial/thirdparty/xdiff.c
--- a/mercurial/thirdparty/xdiff.c
+++ b/mercurial/thirdparty/xdiff.c
@@ -38,9 +38,9 @@
return PyErr_NoMemory();
 
xpparam_t xpp = {
-   0,/* flags */
-   NULL, /* anchors */
-   0,/* anchors_nr */
+   XDF_INDENT_HEURISTIC, /* flags */
+   NULL, /* anchors */
+   0,/* anchors_nr */
};
xdemitconf_t xecfg = {
0,  /* ctxlen */



To: ryanmce, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2577: mdiff: prefer xdiff for diff calculation

[ryanmce (Ryan McElroy)]

ryanmce created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Let's switch to xdiff for its better diff quality and faster performance!
  
  bdiff is still used as a fallback for cases xdiff isn't built, or the pure
  Python version.

TEST PLAN
  Added the "patience" test case mentioned in previous commit. It fails
  with a huge diff output before this change.
  
  I expected some annotate/diff output changes due to diff blocks being
  shifted around. Let's see what sandcastle says. I'll make adjustments
  accordingly.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2577

AFFECTED FILES
  mercurial/mdiff.py
  tests/hghave.py
  tests/test-diff-antipatience.t

CHANGE DETAILS

diff --git a/tests/test-diff-antipatience.t b/tests/test-diff-antipatience.t
new file mode 100644
--- /dev/null
+++ b/tests/test-diff-antipatience.t
@@ -0,0 +1,41 @@
+#require xdiff
+
+Test case that makes use of the weakness of patience diff algorithm
+
+  $ hg init
+  >>> open('a', 'w').write('\n'.join(list('a' + 'x' * 300 + 'u' + 'x' * 700 + 
'a\n')))
+  $ hg commit -m 1 -A a
+  >>> open('a', 'w').write('\n'.join(list('b' + 'x' * 700 + 'u' + 'x' * 300 + 
'b\n')))
+  $ hg diff
+  diff -r 6c45a8fe8cb6 a
+  --- a/a  Thu Jan 01 00:00:00 1970 +
+  +++ b/a  Thu Jan 01 00:00:00 1970 +
+  @@ -1,4 +1,4 @@
+  -a
+  +b
+   x
+   x
+   x
+  @@ -299,7 +299,6 @@
+   x
+   x
+   x
+  -u
+   x
+   x
+   x
+  @@ -700,6 +699,7 @@
+   x
+   x
+   x
+  +u
+   x
+   x
+   x
+  @@ -1000,5 +1000,5 @@
+   x
+   x
+   x
+  -a
+  +b
+   
diff --git a/tests/hghave.py b/tests/hghave.py
--- a/tests/hghave.py
+++ b/tests/hghave.py
@@ -708,3 +708,11 @@
 # libfuzzer is new in clang 6
 return int(mat.group(1)) > 5
 return False
+
+@check("xdiff", "xdiff algorithm")
+def has_xdiff():
+try:
+from mercurial.cext import xdiff
+return xdiff.blocks('', '') == [(0, 0, 0, 0)]
+except ImportError:
+return False
diff --git a/mercurial/mdiff.py b/mercurial/mdiff.py
--- a/mercurial/mdiff.py
+++ b/mercurial/mdiff.py
@@ -25,13 +25,18 @@
 bdiff = policy.importmod(r'bdiff')
 mpatch = policy.importmod(r'mpatch')
 
-blocks = bdiff.blocks
 fixws = bdiff.fixws
 patches = mpatch.patches
 patchedsize = mpatch.patchedsize
 textdiff = bdiff.bdiff
 splitnewlines = bdiff.splitnewlines
 
+try:
+from .cext import xdiff
+blocks = xdiff.blocks
+except ImportError:
+blocks = bdiff.blocks
+
 class diffopts(object):
 '''context is the number of context lines
 text treats all files as text
@@ -200,7 +205,7 @@
 if opts.ignorews or opts.ignorewsamount or opts.ignorewseol:
 text1 = wsclean(opts, text1, False)
 text2 = wsclean(opts, text2, False)
-diff = bdiff.blocks(text1, text2)
+diff = blocks(text1, text2)
 for i, s1 in enumerate(diff):
 # The first match is special.
 # we've either found a match starting at line 0 or a match later
@@ -508,7 +513,7 @@
 
 # similar to difflib.SequenceMatcher.get_matching_blocks
 def get_matching_blocks(a, b):
-return [(d[0], d[2], d[1] - d[0]) for d in bdiff.blocks(a, b)]
+return [(d[0], d[2], d[1] - d[0]) for d in blocks(a, b)]
 
 def trivialdiffheader(length):
 return struct.pack(">lll", 0, 0, length) if length else ''



To: ryanmce, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D2576: xdiff: add a python wrapper

[ryanmce (Ryan McElroy)]

ryanmce created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Implement a `mercurial.cext.xdiff` module that exposes the xdiff algorithm.
  
  `xdiff.blocks` should be a drop-in replacement for `bdiff.blocks`.
  
  In theory we can change the pure C version of `bdiff.c` directly. However
  that means we lose bdiff entirely. It seems more flexible to have both at
  the same time so they can be easily switched via Python code. Hence the
  Python module approach.

TEST PLAN
  `make local`. And test from `hg debugshell`:
  
In [1]: from mercurial.cext import bdiff, xdiff

In [2]: bdiff.blocks('b\nc\nd\n', 'a\nb\nc\ne\nf\n')
Out[2]: [(0, 2, 1, 3), (3, 3, 5, 5)]

In [3]: xdiff.blocks('b\nc\nd\n', 'a\nb\nc\ne\nf\n')
Out[3]: [(0, 2, 1, 3), (3, 3, 5, 5)]

In [4]: bdiff.blocks('a\nb', '')
Out[4]: [(2, 2, 0, 0)]

In [5]: xdiff.blocks('a\nb', '')
Out[5]: [(2, 2, 0, 0)]

In [6]: bdiff.blocks('a\nb', 'a\nb\n')
Out[6]: [(0, 1, 0, 1), (2, 2, 2, 2)]

In [7]: xdiff.blocks('a\nb', 'a\nb\n')
Out[7]: [(0, 1, 0, 1), (2, 2, 2, 2)]

In [8]: xdiff.blocks('', '')
Out[8]: [(0, 0, 0, 0)]

In [9]: bdiff.blocks('', '')
Out[9]: [(0, 0, 0, 0)]

In [10]: xdiff.blocks('', '\n')
Out[10]: [(0, 0, 1, 1)]

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2576

AFFECTED FILES
  contrib/check-code.py
  mercurial/thirdparty/xdiff.c
  setup.py

CHANGE DETAILS

diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -220,7 +220,7 @@
 py2exe.Distribution # silence unused import warning
 py2exeloaded = True
 # import py2exe's patched Distribution class
-from distutils.core import Distribution
+from distutils.core import Distribution # noqa: F811
 except ImportError:
 py2exeloaded = False
 
@@ -874,6 +874,28 @@
   extra_compile_args=osutil_cflags,
   extra_link_args=osutil_ldflags,
   depends=common_depends),
+Extension('mercurial.cext.xdiff',
+  sources=[
+  'mercurial/thirdparty/xdiff/xdiffi.c',
+  'mercurial/thirdparty/xdiff/xemit.c',
+  'mercurial/thirdparty/xdiff/xmerge.c',
+  'mercurial/thirdparty/xdiff/xprepare.c',
+  'mercurial/thirdparty/xdiff/xutils.c',
+  'mercurial/thirdparty/xdiff.c',
+  ],
+  include_dirs=common_include_dirs + [
+  'mercurial/thirdparty/xdiff',
+  ],
+  depends=common_depends + [
+  'mercurial/thirdparty/xdiff/xdiff.h',
+  'mercurial/thirdparty/xdiff/xdiffi.h',
+  'mercurial/thirdparty/xdiff/xemit.h',
+  'mercurial/thirdparty/xdiff/xinclude.h',
+  'mercurial/thirdparty/xdiff/xmacros.h',
+  'mercurial/thirdparty/xdiff/xprepare.h',
+  'mercurial/thirdparty/xdiff/xtypes.h',
+  'mercurial/thirdparty/xdiff/xutils.h',
+  ]),
 Extension('hgext.fsmonitor.pywatchman.bser',
   ['hgext/fsmonitor/pywatchman/bser.c']),
 ]
diff --git a/mercurial/thirdparty/xdiff.c b/mercurial/thirdparty/xdiff.c
new file mode 100644
--- /dev/null
+++ b/mercurial/thirdparty/xdiff.c
@@ -0,0 +1,96 @@
+/*
+ xdiff.c: simple Python wrapper for xdiff library
+
+ Copyright (c) 2018 Facebook, Inc.
+
+ This software may be used and distributed according to the terms of the
+ GNU General Public License version 2 or any later version.
+*/
+
+#include "xdiff.h"
+#include "Python.h"
+
+static int hunk_consumer(long a1, long a2, long b1, long b2, void *priv)
+{
+   PyObject *rl = (PyObject *)priv;
+   PyObject *m = Py_BuildValue("", a1, a2, b1, b2);
+   if (!m)
+   return -1;
+   if (PyList_Append(rl, m) != 0) {
+   Py_DECREF(m);
+   return -1;
+   }
+   return 0;
+}
+
+static PyObject *blocks(PyObject *self, PyObject *args)
+{
+   char *sa, *sb;
+   int na, nb;
+
+   if (!PyArg_ParseTuple(args, "s#s#", , , , ))
+   return NULL;
+
+   mmfile_t a = {sa, na}, b = {sb, nb};
+
+   PyObject *rl = PyList_New(0);
+   if (!rl)
+   return PyErr_NoMemory();
+
+   xpparam_t xpp = {
+   0,/* flags */
+   NULL, /* anchors */
+   0,/* anchors_nr */
+   };
+   xdemitconf_t xecfg = {
+   0,  /* ctxlen */
+   0,  /* interhunkctxlen */
+   XDL_EMIT_BDIFFHUNK, /* flags */
+   NULL,   /* find_func */
+   NULL,   /* find_func_priv */
+   hunk_consumer,  /* hunk_consume_func */
+   };
+   xdemitcb_t ecb = {
+   rl,   /* priv */
+   NULL, /* outf */
+   };
+
+   if (xdl_diff(, , , , ) != 0) 

D1506: dagop: handle IndexError when using wdir() in dag range

[ryanmce (Ryan McElroy)]

ryanmce added a comment.


  I understand that fixing the crash is a good first step, but ideally, 
wouldn't this be supported? Is there a reason it's particularly hard to support?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1506

To: swhitaker, #hg-reviewers, mbthomas, yuja
Cc: ryanmce, yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1332: cmdutil: create dirs for templated cat file output

[ryanmce (Ryan McElroy)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHG407ec7f3ff02: cmdutil: create dirs for templated cat file 
output (authored by ryanmce, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D1332?vs=3326=3373

REVISION DETAIL
  https://phab.mercurial-scm.org/D1332

AFFECTED FILES
  mercurial/cmdutil.py
  tests/test-cat.t

CHANGE DETAILS

diff --git a/tests/test-cat.t b/tests/test-cat.t
--- a/tests/test-cat.t
+++ b/tests/test-cat.t
@@ -126,7 +126,6 @@
   $ echo a > foo/a
   $ hg add foo/a
   $ hg commit -qm "add foo/a"
-  $ mkdir output
   $ hg cat --output "output/%p" foo/a
-  abort: No such file or directory: output/foo/a
-  [255]
+  $ cat output/foo/a
+  a
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -3029,6 +3029,11 @@
 if fntemplate:
 filename = makefilename(repo, fntemplate, ctx.node(),
 pathname=os.path.join(prefix, path))
+# attempt to create the directory if it does not already exist
+try:
+os.makedirs(os.path.dirname(filename))
+except OSError:
+pass
 with formatter.maybereopen(basefm, filename, opts) as fm:
 data = ctx[path].data()
 if opts.get('decode'):



To: ryanmce, #hg-reviewers, mbthomas, yuja
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1331: cat: test output path behvaior when target path does not exist

[ryanmce (Ryan McElroy)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHG8154119ed236: cat: test output path behvaior when target 
path does not exist (authored by ryanmce, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D1331?vs=3325=3372

REVISION DETAIL
  https://phab.mercurial-scm.org/D1331

AFFECTED FILES
  tests/test-cat.t

CHANGE DETAILS

diff --git a/tests/test-cat.t b/tests/test-cat.t
--- a/tests/test-cat.t
+++ b/tests/test-cat.t
@@ -119,3 +119,14 @@
   $ PATTERN='t4' hg log -r '.' -T "{envvars % '{key} -> {value}\n'}" \
   > --config "experimental.exportableenviron=PATTERN"
   PATTERN -> t4
+
+Test behavior of output when directory structure does not already exist
+
+  $ mkdir foo
+  $ echo a > foo/a
+  $ hg add foo/a
+  $ hg commit -qm "add foo/a"
+  $ mkdir output
+  $ hg cat --output "output/%p" foo/a
+  abort: No such file or directory: output/foo/a
+  [255]



To: ryanmce, #hg-reviewers, mbthomas, yuja
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1331: cat: test output path behvaior when target path does not exist

[ryanmce (Ryan McElroy)]

ryanmce created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  In the next patch, we will change the behavior of the cat command to create
  output file parent directories before attempting to write the output file.
  This patch documents the current unfortunate behavior.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1331

AFFECTED FILES
  tests/test-cat.t

CHANGE DETAILS

diff --git a/tests/test-cat.t b/tests/test-cat.t
--- a/tests/test-cat.t
+++ b/tests/test-cat.t
@@ -119,3 +119,14 @@
   $ PATTERN='t4' hg log -r '.' -T "{envvars % '{key} -> {value}\n'}" \
   > --config "experimental.exportableenviron=PATTERN"
   PATTERN -> t4
+
+Test behavior of output when directory structure does not already exist
+
+  $ mkdir foo
+  $ echo a > foo/a
+  $ hg add foo/a
+  $ hg commit -qm "add foo/a"
+  $ mkdir output
+  $ hg cat --output "output/%p" foo/a
+  abort: No such file or directory: output/foo/a
+  [255]



To: ryanmce, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1332: cmdutil: create dirs for templated cat file output

[ryanmce (Ryan McElroy)]

ryanmce created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Previously, the caller was responsible for creating the directory structure of
  files written out using a path template. This is onerous, especially if the
  exact filenames are not known upfront, but are being accessed via a matcher.
  
  This patch changes things so that hg will attempt to create the appropriate
  directories before writing the output file.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1332

AFFECTED FILES
  mercurial/cmdutil.py
  tests/test-cat.t

CHANGE DETAILS

diff --git a/tests/test-cat.t b/tests/test-cat.t
--- a/tests/test-cat.t
+++ b/tests/test-cat.t
@@ -126,7 +126,6 @@
   $ echo a > foo/a
   $ hg add foo/a
   $ hg commit -qm "add foo/a"
-  $ mkdir output
   $ hg cat --output "output/%p" foo/a
-  abort: No such file or directory: output/foo/a
-  [255]
+  $ cat output/foo/a
+  a
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -3029,6 +3029,11 @@
 if fntemplate:
 filename = makefilename(repo, fntemplate, ctx.node(),
 pathname=os.path.join(prefix, path))
+# attempt to create the directory if it does not already exist
+try:
+os.makedirs(os.path.dirname(filename))
+except OSError:
+pass
 with formatter.maybereopen(basefm, filename, opts) as fm:
 data = ctx[path].data()
 if opts.get('decode'):



To: ryanmce, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D932: filemerge: introduce functions to halt merge flow

[ryanmce (Ryan McElroy)]

ryanmce added inline comments.

INLINE COMMENTS

> martinvonz wrote in filemerge.py:755-756
> nit: Is the default used only with non-interactive ui? If it also works as a 
> default that lets the user just press enter to choose Yes, I'd expect the 
> "yn" to be "Yn"

I followed the pattern seen elsewhere in this file.

If you just hit enter, you do get the default, though.

I can follow-up with a patch to fix all of these unclear prompts if there are 
no objections... (after the freeze of course)

> filemerge.py:775-776
>  checked = True
>  if ui.promptchoice(_("was merge of '%s' successful (yn)?"
>   "$$  $$ ") % fd, 1):
>  r = 1

See here

> filemerge.py:783-785
>  if ui.promptchoice(_(" output file %s appears unchanged\n"
>   "was merge successful (yn)?"
>   "$$  $$ ") % fd, 1):

And here

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D932

To: ryanmce, #hg-reviewers, krbullock, mbthomas, durin42
Cc: martinvonz, durin42, krbullock, pulkit, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D952: merge: additional test cases to show merge-halting behavior

[ryanmce (Ryan McElroy)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHGe10bac797852: merge: additional test cases to show 
merge-halting behavior (authored by ryanmce, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D952?vs=2503=2996

REVISION DETAIL
  https://phab.mercurial-scm.org/D952

AFFECTED FILES
  tests/test-merge-halt.t

CHANGE DETAILS

diff --git a/tests/test-merge-halt.t b/tests/test-merge-halt.t
--- a/tests/test-merge-halt.t
+++ b/tests/test-merge-halt.t
@@ -55,8 +55,13 @@
   rebase aborted
 
 Testing on-failure=prompt
-  $ echo on-failure=prompt >> $HGRCPATH
-  $ cat <> $HGRCPATH
+  > [merge]
+  > on-failure=prompt
+  > [ui]
+  > interactive=1
+  > EOS
+  $ cat < y
   > n
   > EOS
@@ -77,3 +82,83 @@
   $ hg rebase --abort
   rebase aborted
 
+Check that successful tool with failed post-check halts the merge
+  $ cat <> $HGRCPATH
+  > [merge-tools]
+  > true.check=changed
+  > EOS
+  $ cat < y
+  > n
+  > n
+  > EOS
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+   output file a appears unchanged
+  was merge successful (yn)? y
+   output file b appears unchanged
+  was merge successful (yn)? n
+  merging b failed!
+  continue merge operation (yn)? n
+  merge halted after failed merge (see hg resolve)
+  [1]
+
+  $ hg resolve --list
+  R a
+  U b
+
+  $ hg rebase --abort
+  rebase aborted
+
+Check that conflicts with conflict check also halts the merge
+  $ cat <> $HGRCPATH
+  > [merge-tools]
+  > true.check=conflicts
+  > true.premerge=keep
+  > [merge]
+  > on-failure=halt
+  > EOS
+  $ hg rebase -s 1 -d 2 --tool true
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+  merging a failed!
+  merge halted after failed merge (see hg resolve)
+  [1]
+
+  $ hg resolve --list
+  U a
+  U b
+
+  $ hg rebase --abort
+  rebase aborted
+
+Check that always-prompt also can halt the merge
+  $ cat < y
+  > n
+  > EOS
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+  was merge of 'a' successful (yn)? y
+  was merge of 'b' successful (yn)? n
+  merging b failed!
+  merge halted after failed merge (see hg resolve)
+  [1]
+
+  $ hg resolve --list
+  R a
+  U b
+
+  $ hg rebase --abort
+  rebase aborted
+
+Check that successful tool otherwise allows the merge to continue
+  $ hg rebase -s 1 -d 2 --tool echo --keep --config 
merge-tools.echo.premerge=keep
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+  $TESTTMP/repo/a *a~base* *a~other* (glob)
+  $TESTTMP/repo/b *b~base* *b~other* (glob)



To: ryanmce, #hg-reviewers, mbthomas, durin42
Cc: durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1155: config: gather allowdivergence under the evolution namespace

[ryanmce (Ryan McElroy)]

ryanmce added a comment.


  I think this series overall cleans up the namespaces. I don't really love 
that evolution is still "hiding" under experimental but currently we don't have 
a way of marking non-"experimental" configs as experimental still so I guess 
this will have to suffice for now. I'll take a closer look tomorrow if nobody 
else gets to this first.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1155

To: lothiraldan, #hg-reviewers
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1165: arbitraryfilecontext: skip the cmp fast path if any side is a symlink

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  queued

INLINE COMMENTS

> context.py:2578
> +# Note that filecmp uses the opposite return values (True if 
> same)
> +# as our ``cmp`` functions (True if different).
>  return not filecmp.cmp(self.path(), 
> self._repo.wjoin(fctx.path()))

I will remove the backticks in-flight; I think this are discouraged in comments 
still

Excellent comment overall, nonetheless

> test-arbitraryfilectx.t:4-5
> +  > from __future__ import absolute_import
> +  > from mercurial import context, commands, registrar
> +  > import filecmp
> +  > cmdtable = {}

--- /data/users/rmcelroy/mercurial/hg/tests/test-check-module-imports.t
  +++ /data/users/rmcelroy/mercurial/hg/tests/test-check-module-imports.t.err
  @@ -42,3 +42,6 @@
 > -X tests/test-lock.py \
 > -X tests/test-verify-repo-operations.py \
 > | sed 's-\\-/-g' | $PYTHON "$import_checker" -
  +  tests/test-arbitraryfilectx.t:4: imports from mercurial not lexically 
sorted: commands < context
  +  tests/test-arbitraryfilectx.t:5: stdlib import "filecmp" follows local 
import: mercurial
  +  [1]

I can fix this in flight. Are you running all the `test-check-*.t` tests 
locally?

> test-arbitraryfilectx.t:8-13
> +  > @command(b'eval', [], 'hg eval CMD')
> +  > def eval_(ui, repo, *cmds, **opts):
> +  > cmd = " ".join(cmds)
> +  > res = str(eval(cmd, globals(), locals()))
> +  > ui.warn("%s" % res)
> +  > EOF

What a terrible, evil extension!

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1165

To: phillco, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1150: config: use 'experimental.evolution.allowunstable'

[ryanmce (Ryan McElroy)]

ryanmce added inline comments.

INLINE COMMENTS

> test-check-commit.t:22
>  
> -

Please avoid unrelated changes

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1150

To: lothiraldan, durin42, #hg-reviewers
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1133: test-clang-format: new test to verify that files stay clang-formatted

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  Yeah, I was running the wrong run-tests.py. Thanks for the help @durin42.
  
  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1133

To: durin42, #hg-reviewers, ryanmce
Cc: quark, ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1133: test-clang-format: new test to verify that files stay clang-formatted

[ryanmce (Ryan McElroy)]

ryanmce added a comment.


  I get `skipped: unknown feature: clang-format` when running 
`test-check-clang-format.t` now. What am I doing wrong?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1133

To: durin42, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1157: context: audit paths before clearing unknown files and dirs

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1157

To: mbthomas, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1074: branch: add a --rev flag to change branch name of given revisions

[ryanmce (Ryan McElroy)]

ryanmce requested changes to this revision.
ryanmce added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> cmdutil.py:722-731
> +revs = scmutil.revrange(repo, revs)
> +roots = repo.revs('roots(%ld)', revs)
> +if len(roots) > 1:
> +raise error.Abort(_("cannot change branch of non-linear revisions"))
> +root = repo[roots.first()]
> +if root.phase() <= phases.public:
> +raise error.Abort(_("cannot change branch of public revisions"))

What about an unclean working copy? It looks like that's supported -- could we 
make sure it's tested?

> cmdutil.py:733-734
> +
> +replacements = {}
> +with repo.wlock(), repo.lock(), repo.transaction('branches'):
> +# avoid import cycle mercurial.cmdutil -> mercurial.context ->

In theory, the phase boundary could move after our check but before we take the 
repolock. Can we guard against that please?

Two ways to accomplish this:

1/ Take the repo lock earlier
2/ Check before repo lock (fast path) and again after the repo lock is held (in 
case something changed)

> cmdutil.py:791-792
> +if newid is not None:
> +# avoid import cycle mercurial.cmdutil -> mercurial.hg ->
> +# mercurial.cmdutil
> +from . import hg

Seems like we need a refactor? (out of scope for this change though)

> test-branch-change.t:8
> +  > [alias]
> +  > glog = log -G -T "{rev}:{node|short} {desc}\n{branch} ({bookmarks})"
> +  > [experimental]

nit: I'd prefer no newline in the glog output so things are more compact

> test-branch-change.t:10
> +  > [experimental]
> +  > evolution = createmarkers
> +  > [extensions]

This test needs to include a case where we strip commits, especially with a 
merge, just so the behavior is clear in this change.

That will probably (in my understanding of the current code) expose an issue 
that needs to be fixed.

> test-branch-change.t:17-34
> +  $ for ch in a b c d e f g h; do echo foo >> $ch; hg ci -Aqm "Added "$ch; 
> done
> +  $ hg glog
> +  @  7:ec2426147f0e Added h
> +  |  default ()
> +  o  6:87d6d6676308 Added g
> +  |  default ()
> +  o  5:825660c69f0c Added f

Not sure we need this many commits for the test to be useful.

> test-branch-change.t:304-305
> +
> +  $ hg branch -r 23::28 wat
> +  changed branch on 0 changesets
> +  $ hg glog

Hm, I think I'd prefer this to be an error about public changesets rather than 
a no-op with a 0 return value. Trying to modify the branch of public changesets 
-- even if the change is a no-op, still seems like an error to me. Thoughts?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1074

To: pulkit, #hg-reviewers, dlax, ryanmce
Cc: ryanmce, dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1138: webcommands: replace str(ctx) etc with pycompat.bytestr(ctx) etc

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1138

To: durin42, #hg-reviewers, pulkit, ryanmce
Cc: ryanmce, pulkit, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1137: templater: don't blow up when trying to build an abort message

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1137

To: durin42, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1135: webutil: use pycompat.bytestr() instead of str()

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1135

To: durin42, #hg-reviewers, pulkit, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1134: hgweb: correct an earlier error of mine - `start` should be bytes

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1134

To: durin42, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1136: templatefilters: defend against evil unicode strs in json filter

[ryanmce (Ryan McElroy)]

ryanmce added a comment.


  I'll queue this series except this patch so we can have more discussion on 
what we should do here.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1136

To: durin42, #hg-reviewers
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1136: templatefilters: defend against evil unicode strs in json filter

[ryanmce (Ryan McElroy)]

ryanmce added a comment.


  I'm not a python 3 expert by any means, but I'm not sold on this change.
  
  I think we need perhaps a programming error so it's clear its a bug and not 
something the user needs to fix in their command inputs.

INLINE COMMENTS

> templatefilters.py:241-242
> +# hurt someone.
> +raise TypeError(
> +r'Mercurial only does output with bytes on Python 3: %r' % obj)
>  elif util.safehasattr(obj, 'keys'):

I fear that this is is an non-actionable error. How might I see it as a user? 
If I see it, what do I do?

Also, why is this a TypeError? Should this be a ProgrammingError instead? Or is 
there a way user input could trigger this?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1136

To: durin42, #hg-reviewers
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1135: webutil: use pycompat.bytestr() instead of str()

[ryanmce (Ryan McElroy)]

ryanmce added inline comments.

INLINE COMMENTS

> webutil.py:593-596
> +yield {'name': key,
> +   'value': pycompat.bytestr(value),
> +   'separator': separator,
> +}

Buh, this looks ugly as sin. Can we drop 'name' to the next line please? (I 
can do that in-flight, actually)

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1135

To: durin42, #hg-reviewers, pulkit
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1117: config: simplify aliasing commands.update.check

[ryanmce (Ryan McElroy)]

ryanmce added a comment.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1117

To: lothiraldan, #hg-reviewers, durin42, ryanmce
Cc: ryanmce, durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1117: config: simplify aliasing commands.update.check

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.


  I agree with @lothiraldan  that there's no pressing need to remove aliases, 
and we should have a mechanism to work before we do this for sure.
  
  This seems like exactly one of the reasons the config registration system was 
built in the first place -- to permanently (or semi-permanently at least) 
forward old configs to new ones, to allow smooth config upgrade paths.
  
  I'll queue this as-is but I think we should not necessarily drop this in the 
future.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1117

To: lothiraldan, #hg-reviewers, durin42, ryanmce
Cc: ryanmce, durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D949: run-tests: set HGPLAIN=1 when bisecting

[ryanmce (Ryan McElroy)]

ryanmce added a comment.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D949

To: quark, #hg-reviewers, ryanmce
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D948: run-tests: extract Popen logic to a single method

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D948

To: quark, #hg-reviewers, ryanmce
Cc: ryanmce, dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1122: arbitraryfilecontext: skip the cmp fast path if any side is a symlink

[ryanmce (Ryan McElroy)]

ryanmce added a comment.


  I'm dropping this from hg-committed for now. Please re-submit with the 
check-code issues fixed.
  
  @hg-reviewers: If I'm doing this wrong by dropping the patch, please let me 
know so I can correct my behavior in the future.

INLINE COMMENTS

> test-arbitraryfilectx.t:53
> +  $ hg eval "context.arbitraryfilectx('real_A', 
> repo).cmp(repo[None]['sym_A'])"
> +  False (no-e ol)
> +

why is there a space after the e here?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1122

To: phillco, #hg-reviewers, durin42
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1132: mpatch: switch alignment of wrapped line from tab to spaces with clang-format

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1132

To: durin42, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1133: test-clang-format: new test to verify that files stay clang-formatted

[ryanmce (Ryan McElroy)]

ryanmce requested changes to this revision.
ryanmce added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> test-check-clang-format.t:1
> +#require clang-format test-repo
> +  $ cd "$TESTDIR"/..

This test fails for me with errors about lz4revlog.

It feels like what we need here is a change similar to 
https://phab.mercurial-scm.org/rHG6c113a7dec52be38927b16962282ea660de0b03c, 
where we use the system hg to examine the local repository. Please reuse the 
`syshg` function here.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1133

To: durin42, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1122: arbitraryfilecontext: skip the cmp fast path if any side is a symlink

[ryanmce (Ryan McElroy)]

ryanmce added a comment.


  Hm, check-code is failing here.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1122

To: phillco, #hg-reviewers, durin42
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D947: run-tests: move bisect logic to a separate method

[ryanmce (Ryan McElroy)]

ryanmce added a comment.


  I think I request revision because the series needed to change, and I wanted 
to take it out of other people's queues. Will look again at the whole series 
now.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D947

To: quark, #hg-reviewers, ryanmce
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D952: merge: additional test cases to show changing behavior

[ryanmce (Ryan McElroy)]

ryanmce added a comment.


  > https://phab.mercurial-scm.org/D953 is abandoned- should this be as well?
  
  No, but it should probably be retitled. I'll do that now.
  
  It turns out that the behavior did change, but only slightly, and not in a 
useful way, with the https://phab.mercurial-scm.org/D953. This still adds 
useful tests that show that the other ways of saying a file merge process 
didn't go well also can halt the merge.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D952

To: ryanmce, #hg-reviewers, mbthomas
Cc: durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D932: filemerge: introduce functions to halt merge flow

[ryanmce (Ryan McElroy)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHGa1625035db16: filemerge: introduce functions to halt merge 
flow (authored by ryanmce, committed by ).

CHANGED PRIOR TO COMMIT
  https://phab.mercurial-scm.org/D932?vs=2501=2876#toc

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D932?vs=2501=2876

REVISION DETAIL
  https://phab.mercurial-scm.org/D932

AFFECTED FILES
  mercurial/configitems.py
  mercurial/filemerge.py
  mercurial/help/config.txt

CHANGE DETAILS

diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -1288,6 +1288,17 @@
different contents. Similar to ``merge.checkignored``, except for files that
are not ignored. (default: ``abort``)
 
+``on-failure``
+   When set to ``continue`` (the default), the merge process attempts to
+   merge all unresolved files using the merge chosen tool, regardless of
+   whether previous file merge attempts during the process succeeded or not.
+   Setting this to ``prompt`` will prompt after any merge failure continue
+   or halt the merge process. Setting this to ``halt`` will automatically
+   halt the merge process on any merge tool failure. The merge process
+   can be restarted by using the ``resolve`` command. When a merge is
+   halted, the repository is left in a normal ``unresolved`` merge state.
+   (default: ``continue``)
+
 ``merge-patterns``
 --
 
diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -745,6 +745,20 @@
 if not r and back is not None:
 back.remove()
 
+def _haltmerge():
+msg = _('merge halted after failed merge (see hg resolve)')
+raise error.InterventionRequired(msg)
+
+def _onfilemergefailure(ui):
+action = ui.config('merge', 'on-failure')
+if action == 'prompt':
+msg = _('continue merge operation (yn)?' '$$  $$ ')
+if ui.promptchoice(msg, 0) == 1:
+_haltmerge()
+if action == 'halt':
+_haltmerge()
+# default action is 'continue', in which case we neither prompt nor halt
+
 def _check(repo, r, ui, tool, fcd, files):
 fd = fcd.path()
 unused, unused, unused, back = files
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -531,6 +531,9 @@
 coreconfigitem('merge', 'followcopies',
 default=True,
 )
+coreconfigitem('merge', 'on-failure',
+default='continue',
+)
 coreconfigitem('merge', 'preferancestor',
 default=lambda: ['*'],
 )



To: ryanmce, #hg-reviewers, krbullock, mbthomas, durin42
Cc: durin42, krbullock, pulkit, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D951: merge: allow user to halt merge on merge-tool failures

[ryanmce (Ryan McElroy)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHGf583cef55995: merge: allow user to halt merge on merge-tool 
failures (authored by ryanmce, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D951?vs=2502=2877

REVISION DETAIL
  https://phab.mercurial-scm.org/D951

AFFECTED FILES
  mercurial/filemerge.py
  tests/test-merge-halt.t

CHANGE DETAILS

diff --git a/tests/test-merge-halt.t b/tests/test-merge-halt.t
--- a/tests/test-merge-halt.t
+++ b/tests/test-merge-halt.t
@@ -44,8 +44,7 @@
   merging a
   merging b
   merging a failed!
-  merging b failed!
-  unresolved conflicts (see hg resolve, then hg rebase --continue)
+  merge halted after failed merge (see hg resolve)
   [1]
 
   $ hg resolve --list
@@ -65,8 +64,10 @@
   merging a
   merging b
   merging a failed!
+  continue merge operation (yn)? y
   merging b failed!
-  unresolved conflicts (see hg resolve, then hg rebase --continue)
+  continue merge operation (yn)? n
+  merge halted after failed merge (see hg resolve)
   [1]
 
   $ hg resolve --list
diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -739,6 +739,7 @@
 if r:
 if onfailure:
 ui.warn(onfailure % fd)
+_onfilemergefailure(ui)
 
 return True, r, deleted
 finally:



To: ryanmce, #hg-reviewers, mbthomas, durin42
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1099: dagutil: use a listcomp instead of a map()

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1099

To: durin42, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1101: wireproto: use a proper exception instead of `assert False`

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1101

To: durin42, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1104: peer: ensure command names are always ascii bytestrs

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1104

To: durin42, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1100: wireproto: use listcomp instead of map()

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1100

To: durin42, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1103: httppeer: always produce native str header keys and values

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1103

To: durin42, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D931: merge: ensure that we always commit the mergestate

[ryanmce (Ryan McElroy)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHGc0a524f77e8a: merge: ensure that we always commit the 
mergestate (authored by ryanmce, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D931?vs=2500=2714

REVISION DETAIL
  https://phab.mercurial-scm.org/D931

AFFECTED FILES
  mercurial/merge.py

CHANGE DETAILS

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -1537,30 +1537,32 @@
 newactions.append((f, args, msg))
 mergeactions = newactions
 
-# premerge
-tocomplete = []
-for f, args, msg in mergeactions:
-repo.ui.debug(" %s: %s -> m (premerge)\n" % (f, msg))
-z += 1
-progress(_updating, z, item=f, total=numupdates, unit=_files)
-if f == '.hgsubstate': # subrepo states need updating
-subrepo.submerge(repo, wctx, mctx, wctx.ancestor(mctx),
- overwrite, labels)
-continue
-wctx[f].audit()
-complete, r = ms.preresolve(f, wctx)
-if not complete:
-numupdates += 1
-tocomplete.append((f, args, msg))
+try:
+# premerge
+tocomplete = []
+for f, args, msg in mergeactions:
+repo.ui.debug(" %s: %s -> m (premerge)\n" % (f, msg))
+z += 1
+progress(_updating, z, item=f, total=numupdates, unit=_files)
+if f == '.hgsubstate': # subrepo states need updating
+subrepo.submerge(repo, wctx, mctx, wctx.ancestor(mctx),
+ overwrite, labels)
+continue
+wctx[f].audit()
+complete, r = ms.preresolve(f, wctx)
+if not complete:
+numupdates += 1
+tocomplete.append((f, args, msg))
 
-# merge
-for f, args, msg in tocomplete:
-repo.ui.debug(" %s: %s -> m (merge)\n" % (f, msg))
-z += 1
-progress(_updating, z, item=f, total=numupdates, unit=_files)
-ms.resolve(f, wctx)
+# merge
+for f, args, msg in tocomplete:
+repo.ui.debug(" %s: %s -> m (merge)\n" % (f, msg))
+z += 1
+progress(_updating, z, item=f, total=numupdates, unit=_files)
+ms.resolve(f, wctx)
 
-ms.commit()
+finally:
+ms.commit()
 
 unresolved = ms.unresolvedcount()
 



To: ryanmce, #hg-reviewers, mbthomas, durin42
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D930: merge: add tests to show current behavior on failed filemerges

[ryanmce (Ryan McElroy)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHG143337bcec9b: merge: add tests to show current behavior on 
failed filemerges (authored by ryanmce, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D930?vs=2499=2712

REVISION DETAIL
  https://phab.mercurial-scm.org/D930

AFFECTED FILES
  tests/test-merge-halt.t

CHANGE DETAILS

diff --git a/tests/test-merge-halt.t b/tests/test-merge-halt.t
new file mode 100644
--- /dev/null
+++ b/tests/test-merge-halt.t
@@ -0,0 +1,78 @@
+  $ cat >> $HGRCPATH < [extensions]
+  > rebase=
+  > [phases]
+  > publish=False
+  > [merge]
+  > EOF
+
+  $ hg init repo
+  $ cd repo
+  $ echo a > a
+  $ echo b > b
+  $ hg commit -qAm ab
+  $ echo c >> a
+  $ echo c >> b
+  $ hg commit -qAm c
+  $ hg up -q ".^"
+  $ echo d >> a
+  $ echo d >> b
+  $ hg commit -qAm d
+
+Testing on-failure=continue
+  $ echo on-failure=continue >> $HGRCPATH
+  $ hg rebase -s 1 -d 2 --tool false
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+  merging a failed!
+  merging b failed!
+  unresolved conflicts (see hg resolve, then hg rebase --continue)
+  [1]
+
+  $ hg resolve --list
+  U a
+  U b
+
+  $ hg rebase --abort
+  rebase aborted
+
+Testing on-failure=halt
+  $ echo on-failure=halt >> $HGRCPATH
+  $ hg rebase -s 1 -d 2 --tool false
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+  merging a failed!
+  merging b failed!
+  unresolved conflicts (see hg resolve, then hg rebase --continue)
+  [1]
+
+  $ hg resolve --list
+  U a
+  U b
+
+  $ hg rebase --abort
+  rebase aborted
+
+Testing on-failure=prompt
+  $ echo on-failure=prompt >> $HGRCPATH
+  $ cat < y
+  > n
+  > EOS
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+  merging a failed!
+  merging b failed!
+  unresolved conflicts (see hg resolve, then hg rebase --continue)
+  [1]
+
+  $ hg resolve --list
+  U a
+  U b
+
+  $ hg rebase --abort
+  rebase aborted
+



To: ryanmce, #hg-reviewers, quark, durin42
Cc: quark, krbullock, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1030: mpatch: switch alignment of wrapped line from tab to spaces with clang-format

[ryanmce (Ryan McElroy)]

ryanmce requested changes to this revision.
ryanmce added a comment.
This revision now requires changes to proceed.


  Seems like we should figure out the spacing stuff on 
https://phab.mercurial-scm.org/D1028 before moving forward here.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1030

To: durin42, #hg-reviewers, pulkit, ryanmce
Cc: ryanmce, pulkit, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1046: tweakdefaults: enable rebase

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision as: ryanmce.
ryanmce added a comment.


  Accepting as myself (not as a reviewer though, since we're both FB). I'm a 
big +1 on this.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1046

To: quark, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1028: mpatch: reformat function prototypes with clang-format

[ryanmce (Ryan McElroy)]

ryanmce added a comment.


  I thought it was string that check-code doesn't fire on 
https://phab.mercurial-scm.org/D1030 but does fire here.
  
  It turns out that the check-code only finds spaces at the start of the line. 
So is you start with a tab and then spaces, that's fine. But if you start with 
spaces you trip up against test-check-code. In 
https://phab.mercurial-scm.org/D1030, the re-indented line start with a tab, 
whereas here the re-indented line is top-level so it starts with spaces.
  
  I, for one, am -1 on tabs but that's not a battle worth fighting probably. 
However, we can probably come up with a smarter test-check-code. If we're 
moving towards clang-format, it seems that the test-code should be checking 
that and not arbitrarily for spaces at the start of a line.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1028

To: durin42, #hg-reviewers, pulkit, yuja
Cc: ryanmce, yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1053: i18n: cache translated messages per encoding

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1053

To: yuja, #hg-reviewers, quark, ryanmce
Cc: ryanmce, quark, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D950: run-tests: update back to original node after bisecting

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision as: ryanmce.
ryanmce added a comment.


  I like this overall; but I don't feel I can queue a new flag to bisect 
without input from the rest of the community.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D950

To: quark, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D950: run-tests: update back to original node after bisecting

[ryanmce (Ryan McElroy)]

ryanmce added inline comments.

INLINE COMMENTS

> quark wrote in run-tests.py:2124-2153
> During the meeting deciding to experiment with Phabricator, I think we agreed 
> that with Phabricator's ability to diff with whitespaces ignored 
>  
> and its yellow margins of "Moved from line X", the patch size standard could 
> be more flexible.

I suppose, I ended up using that ability but it would still help readability 
either way. I won't fuss too much over it.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D950

To: quark, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1044: bisect: add --abort flag

[ryanmce (Ryan McElroy)]

ryanmce requested changes to this revision.
ryanmce added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> commands.py:800
> +if firsttime:
> +raise error.Abort(_('not in a bisect'))
> +cmdutil.checkunfinished(repo)

Histedit does this:

  abort: no histedit in progress

That wording sounds more natural to me, so I'd suggest something similar here, 
like:

  abort: no bisect in progress

> commands.py:804
> +if not state['original']:
> +ui.warn(_('warning: unknown original changeset\n'))
> +else:

I think we can be more clear here. It's not that the original is an unknown 
node (which is how I parse this wording), but that that state file didn't have 
one, so perhaps "warning: not updating since bisect state did not include 
original changeset" would be more clear (though it's a bit wordy).

> dlax wrote in commands.py:809
> This "bisect aborted" message is not very useful (especially at "warning" 
> level): either the command fails with a message or it succeeds silently. Also 
> maybe it'd be more useful to display the new working directory parent.

I agree. This can be a status or even an info. +1 on the idea of mentioning the 
parent.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1044

To: quark, #hg-reviewers, ryanmce
Cc: ryanmce, dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1016: repoview: remove incorrect documentation of the function

[ryanmce (Ryan McElroy)]

ryanmce added a comment.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1016

To: pulkit, #hg-reviewers, ryanmce
Cc: ryanmce, dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1034: codemod: use pycompat.iswindows

[ryanmce (Ryan McElroy)]

ryanmce added a comment.


  Thanks for making this nice improvement @quark!

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1034

To: quark, #hg-reviewers, spectral, lothiraldan
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1021: check-code: forbid platform.system()

[ryanmce (Ryan McElroy)]

ryanmce added a comment.


  +1 to this change, happy to land this once my comments on the series are 
addressed

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1021

To: quark, #hg-reviewers, lothiraldan
Cc: ryanmce, lothiraldan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1020: largefiles: do not use platform.system()

[ryanmce (Ryan McElroy)]

ryanmce requested changes to this revision.
ryanmce added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> lfutil.py:83
>  return os.path.join(appdata, longname)
> -elif platform.system() == 'Darwin':
> +elif pycompat.sysplatform == 'darwin':
>  home = encoding.environ.get('HOME')

why is this not a pycompat.osname check? It doesn't make sense to abort below 
with pycompat.osname if that might still be posix.

To be clear, your patch isn't making this worse, but it's exposing something 
sketchy. Either we need the Abort below to include the sysplatform in its 
output or we need this check to be against pycombat.osname.

I suspect we want to change the abort below, in a separate patch.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1020

To: quark, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1019: logtoprocess: do not use platform.system()

[ryanmce (Ryan McElroy)]

ryanmce requested changes to this revision.
ryanmce added a comment.
This revision now requires changes to proceed.


  I'm +1 on this direction but think we shouldn't `have == 'nt'` as the "is 
this windows?" check here.

INLINE COMMENTS

> logtoprocess.py:55
>  def uisetup(ui):
> -if platform.system() == 'Windows':
> +if pycompat.osname == 'nt':
>  # no fork on Windows, but we can create a detached process

seems like this should be `pycompat.iswindows` (which probably doesn't exist 
yet, but should) because comparisons to things that should be constants 
scattered throughout the code scare me.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1019

To: quark, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D939: remotenames: add functionality to store remotenames under .hg/hgremotenames/

[ryanmce (Ryan McElroy)]

ryanmce requested changes to this revision.
ryanmce added a comment.


  I think we need to take a step back and think through how to implement this 
more robustly than we have currently done in the remotenames extension:
  
  - storage file names
  - storage format
  
  I like that you've split branches and bookmarks, but I suspect that storing 
all paths in the same file isn't the best option.

INLINE COMMENTS

> remotenames.py:35
> +
> +`node remotepath/bookmarkname`
> +

As per the issue you raised about paths containing '/', this probably is 
insufficient.

> dlax wrote in remotenames.py:64
> Functions `saveremotebranches` and `saveremotebookmarks` are very similar, 
> only differing by the vfs file name. Perhaps there could be a single function 
> taking the filename as a parameter?

+1

> remotenames.py:67-69
> +""" save remotenames i.e. remotebookmarks and remotebranches in their
> +respective files under ".hg/remotenames/" directory.
> +"""

nit: start multiline docblocks on next line

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D939

To: pulkit, #hg-reviewers, dlax, ryanmce
Cc: ryanmce, dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D938: remotenames: move utility functions to join and split names to core

[ryanmce (Ryan McElroy)]

ryanmce requested changes to this revision.
ryanmce added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> remotenames.py:10-19
> +def splitremotename(remote):
> +name = ''
> +if '/' in remote:
> +remote, name = remote.split('/', 1)
> +return remote, name
> +
> +def joinremotename(remote, ref):

As you mentioned when we chatted, this doesn't work great because the remote 
path can have a `/` character in it, eg:

  $ hg paths
  com/mon = https://www.mercurial-scm.org/repo/hg-committed/

And then this whole thing breaks. So we need to think about a better storage 
mechanism.

I would suggest JSON as one possibility since it already takes care of all 
encoding problems, and allows nested information that's straightforward to 
manipulate.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D938

To: pulkit, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D937: remotenames: move function to pull remotenames from the remoterepo to core

[ryanmce (Ryan McElroy)]

ryanmce requested changes to this revision.
ryanmce added inline comments.

INLINE COMMENTS

> remotenames.py:10
> +
> +def pullremotenames(localrepo, remoterepo, remotepath):
> +""" pulls bookmarks and branches information of the remote repo during a

It might be better to call this `fetchremotenames` or similar since it might be 
called both on push and on pull

> remotenames.py:11-16
> +""" pulls bookmarks and branches information of the remote repo during a
> +pull or clone operation.
> +localrepo is our local repository
> +remoterepo is the repo from which we are pulling or cloning
> +remotepath is the path of the remote repository
> +"""

nit: for multiline docblocks, I prefer to leave the first line empty:

  """
  pulls bookmarks and branches information of the remote repo during a
  pull or clone operation.
  
  localrepo is our local repository
  remoterepo is the repo from which we are pulling or cloning
  remotepath is the path of the remote repository
  """

> remotenames.py:22-28
> +bmap = {}
> +repo = localrepo.unfiltered()
> +for branch, nodes in remoterepo.branchmap().iteritems():
> +bmap[branch] = []
> +for node in nodes:
> +if node in repo and not repo[node].obsolete():
> +bmap[branch].append(node)

It might be worth refactoring this function into:

(1) `fetchremotenames()`, a top-level function that loops through each 
supported namespace and then calls the appropriate fetching function for each 
name.
(2) `fetchremotebookmarks`, which does the appropriate stuff for bookmarks
(3) `fetchremotebranchheads`, which does the appropriate stuff for branch heads

That should make what's going on more clear.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D937

To: pulkit, #hg-reviewers, dlax, ryanmce
Cc: ryanmce, dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D941: remotenames: add a function to return the vfs object

[ryanmce (Ryan McElroy)]

ryanmce requested changes to this revision.
ryanmce added inline comments.

INLINE COMMENTS

> remotenames.py:31
>  
> +def getremotevfs(repo):
> +""" returns a vfs object for .hg/remotenames/

This is not a great name. It implies it's a vfs on a remote peer, which is 
definitely not what we want to imply.
We might just call it `getvfs` in this case -- it's already in the remotenames 
module so it's obvious what it is if it's used outside of the module.

After reading the description, I see that this is for shared usage in the 
future. You should add a comment here in the code explaining that.
Also, a better way to deal with that is to build a shared vfs in core -- but 
that's outside of the scope of this change.

> remotenames.py:32-33
> +def getremotevfs(repo):
> +""" returns a vfs object for .hg/remotenames/
> +"""
> +return vfsmod.vfs(repo.vfs.join(remotenamedir))

nit: For single-line docblocks, please close the docblock on the same line:

  """returns a vfs object for .hg/remotenames/"""

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D941

To: pulkit, #hg-reviewers, dlax, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D738: directaccess: add support to export and tests to demonstrate things

[ryanmce (Ryan McElroy)]

ryanmce requested changes to this revision.
ryanmce added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> durham wrote in test-directaccess.t:64
> I'm still not sure we want to support rev numbers.  I think we definitely 
> don't want to support them for write commands, even recoverable ones.

This is showing the rev numbers are not supported, right?

> test-directaccess.t:23-35
> +  o  6:87d6d66763085b629e6d7ed56778c79827273022 Added g
> +  |
> +  o  5:825660c69f0cd8d9542c1fea6e5d444c666f063b Added f
> +  |
> +  o  4:aa98ab95a9284d289c73a270d70c37914795a842 Added e
> +  |
> +  o  3:62615734edd52f06b6fb9c2beb429e4fe30d57b8 Added d

Why so many commits here? It seems to needlessly bloat the test.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D738

To: pulkit, #hg-reviewers, durham, ryanmce
Cc: ryanmce, durham, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D948: run-tests: extract Popen logic to a single method

[ryanmce (Ryan McElroy)]

ryanmce added inline comments.

INLINE COMMENTS

> ryanmce wrote in run-tests.py:2116
> I can do this in-flight

Actually, since I've asked for changed on the topmost revision, can you fix 
this up with the next submission please?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D948

To: quark, #hg-reviewers, ryanmce
Cc: ryanmce, dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D950: run-tests: update back to original node after bisecting

[ryanmce (Ryan McElroy)]

ryanmce requested changes to this revision.
ryanmce added a comment.
This revision now requires changes to proceed.


  I'm +1 on this series and the rest looks fine but I think this one needs some 
improvements before landing.

INLINE COMMENTS

> run-tests.py:2115-2116
>  return data
> +orignode = pread(hgcmd + ['id', '--debug'])[:21]
> +if orignode.endswith('+'):
> +self.stream.writeln('skipping bisect: uncommitted changes')

It feels fragile to rely on the output of a debug flag command to determine 
state. Can you use `hg status` instead?

> run-tests.py:2120-2121
> +orignode = orignode[:20]
> +if len(orignode) != 20:
> +self.stream.writeln('skipping bisect: unknown node')
> +return

When does this happen? Add a comment, perhaps? I'd also prefer a more 
explanatory output.

> run-tests.py:2124-2153
> +try:
> +pread(bisectcmd + ['--reset']),
> +pread(bisectcmd + ['--bad', '.'])
> +pread(bisectcmd + ['--good',
> +  self._runner.options.known_good_rev])
> +# TODO: we probably need to forward more options
> +# that alter hg's behavior inside the tests.

For readability, please add the try/finally (with the finally block being just 
`pass` for now) in a separate patch.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D950

To: quark, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D948: run-tests: extract Popen logic to a single method

[ryanmce (Ryan McElroy)]

ryanmce added inline comments.

INLINE COMMENTS

> dlax wrote in run-tests.py:2116
> Perhaps also update the indentation of continuation line?

I can do this in-flight

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D948

To: quark, #hg-reviewers
Cc: ryanmce, dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1016: repoview: remove incorrect documentation of the function

[ryanmce (Ryan McElroy)]

ryanmce requested changes to this revision.
ryanmce added a comment.
This revision now requires changes to proceed.


  I agree that removing is better than leaving bad docs, so I'm +1 on this, but 
as @dlax suggests, improving it would be even better.
  
  If you don't have ideas on how to improve it, feel free to re-request review 
and I can queue this.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1016

To: pulkit, #hg-reviewers, ryanmce
Cc: ryanmce, dlax, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1009: bdiff: remove trailing newlines

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1009

To: durin42, #hg-reviewers, pulkit, ryanmce
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1004: bdiff: format header file with clang-format

[ryanmce (Ryan McElroy)]

ryanmce added inline comments.

INLINE COMMENTS

> bdiff.h:18
>  int bdiff_diff(struct bdiff_line *a, int an, struct bdiff_line *b, int bn,
> - struct bdiff_hunk *base);
> +   struct bdiff_hunk *base);
>  void bdiff_freehunks(struct bdiff_hunk *l);

This doesn't show up in Differential unless you choose the "show all 
whitespace" option.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1004

To: durin42, #hg-reviewers, quark
Cc: ryanmce, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D930: merge: add tests to show current behavior on failed filemerges

[ryanmce (Ryan McElroy)]

ryanmce added a comment.


  > I guess you missed phabricator.callsign = HG in .hg/hgrc.
  
  Yes, that's the case. I've fixed that in my repo.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D930

To: ryanmce, #hg-reviewers, quark
Cc: quark, krbullock, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D821: unamend: move fb extension unamend to core

[ryanmce (Ryan McElroy)]

ryanmce added inline comments.

INLINE COMMENTS

> ryanmce wrote in uncommit.py:260
> Test test

Tested nested comments, sorry for the spam.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D821

To: pulkit, #hg-reviewers, durham
Cc: ryanmce, singhsrb, durham, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D945: fsmonitor: update to match new dirstate refactor

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D945

To: durham, #hg-reviewers, quark, yuja, ryanmce
Cc: ryanmce, yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1000: contrib: add check-code rule banning use of readlink

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1000

To: durin42, #hg-reviewers, quark, ryanmce
Cc: ryanmce, quark, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1001: ui: recommend tweakdefaults in the default hgrc template

[ryanmce (Ryan McElroy)]

ryanmce added a comment.


  queued

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1001

To: durin42, #hg-reviewers, quark, pulkit, ryanmce
Cc: ryanmce, quark, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1001: ui: recommend tweakdefaults in the default hgrc template

[ryanmce (Ryan McElroy)]

ryanmce accepted this revision.
ryanmce added a comment.
This revision is now accepted and ready to land.


  yay!

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1001

To: durin42, #hg-reviewers, quark, pulkit, ryanmce
Cc: ryanmce, quark, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D930: merge: add tests to show current behavior on failed filemerges

[ryanmce (Ryan McElroy)]

ryanmce updated this revision to Diff 2499.

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D930?vs=2411=2499

REVISION DETAIL
  https://phab.mercurial-scm.org/D930

AFFECTED FILES
  tests/test-merge-halt.t

CHANGE DETAILS

diff --git a/tests/test-merge-halt.t b/tests/test-merge-halt.t
new file mode 100644
--- /dev/null
+++ b/tests/test-merge-halt.t
@@ -0,0 +1,78 @@
+  $ cat >> $HGRCPATH < [extensions]
+  > rebase=
+  > [phases]
+  > publish=False
+  > [merge]
+  > EOF
+
+  $ hg init repo
+  $ cd repo
+  $ echo a > a
+  $ echo b > b
+  $ hg commit -qAm ab
+  $ echo c >> a
+  $ echo c >> b
+  $ hg commit -qAm c
+  $ hg up -q ".^"
+  $ echo d >> a
+  $ echo d >> b
+  $ hg commit -qAm d
+
+Testing on-failure=continue
+  $ echo on-failure=continue >> $HGRCPATH
+  $ hg rebase -s 1 -d 2 --tool false
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+  merging a failed!
+  merging b failed!
+  unresolved conflicts (see hg resolve, then hg rebase --continue)
+  [1]
+
+  $ hg resolve --list
+  U a
+  U b
+
+  $ hg rebase --abort
+  rebase aborted
+
+Testing on-failure=halt
+  $ echo on-failure=halt >> $HGRCPATH
+  $ hg rebase -s 1 -d 2 --tool false
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+  merging a failed!
+  merging b failed!
+  unresolved conflicts (see hg resolve, then hg rebase --continue)
+  [1]
+
+  $ hg resolve --list
+  U a
+  U b
+
+  $ hg rebase --abort
+  rebase aborted
+
+Testing on-failure=prompt
+  $ echo on-failure=prompt >> $HGRCPATH
+  $ cat < y
+  > n
+  > EOS
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+  merging a failed!
+  merging b failed!
+  unresolved conflicts (see hg resolve, then hg rebase --continue)
+  [1]
+
+  $ hg resolve --list
+  U a
+  U b
+
+  $ hg rebase --abort
+  rebase aborted
+



To: ryanmce, #hg-reviewers
Cc: quark, krbullock, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D932: filemerge: introduce functions to halt merge flow

[ryanmce (Ryan McElroy)]

ryanmce updated this revision to Diff 2501.

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D932?vs=2457=2501

REVISION DETAIL
  https://phab.mercurial-scm.org/D932

AFFECTED FILES
  mercurial/configitems.py
  mercurial/filemerge.py
  mercurial/help/config.txt

CHANGE DETAILS

diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -1239,6 +1239,17 @@
different contents. Similar to ``merge.checkignored``, except for files that
are not ignored. (default: ``abort``)
 
+``on-failure``
+   When set to ``continue`` (the default), the merge process attempts to
+   merge all unresolved files using the merge chosen tool, regardless of
+   whether previous file merge attempts during the process succeeded or not.
+   Setting this to ``prompt`` will prompt after any merge failure continue
+   or halt the merge process. Setting this to ``halt`` will automatically
+   halt the merge process on any merge tool failure. The merge process
+   can be restarted by using the ``resolve`` command. When a merge is
+   halted, the repository is left in a normal ``unresolved`` merge state.
+   (default: ``continue``)
+
 ``merge-patterns``
 --
 
diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -721,6 +721,20 @@
 if not r and back is not None:
 util.unlink(back)
 
+def _haltmerge():
+msg = _('merge halted after failed merge (see hg resolve)')
+raise error.InterventionRequired(msg)
+
+def _onfilemergefailure(ui):
+action = ui.config('merge', 'on-failure')
+if action == 'prompt':
+msg = _('continue merge operation (yn)?' '$$  $$ ')
+if ui.promptchoice(msg, 0) == 1:
+_haltmerge()
+if action == 'halt':
+_haltmerge()
+# default action is 'continue', in which case we neither prompt nor halt
+
 def _check(repo, r, ui, tool, fcd, files):
 fd = fcd.path()
 unused, unused, unused, back = files
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -305,6 +305,9 @@
 coreconfigitem('merge', 'followcopies',
 default=True,
 )
+coreconfigitem('merge', 'on-failure',
+default='continue',
+)
 coreconfigitem('merge', 'preferancestor',
 default=lambda: ['*'],
 )



To: ryanmce, #hg-reviewers, krbullock, mbthomas
Cc: krbullock, pulkit, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D931: merge: ensure that we always commit the mergestate

[ryanmce (Ryan McElroy)]

ryanmce updated this revision to Diff 2500.

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D931?vs=2412=2500

REVISION DETAIL
  https://phab.mercurial-scm.org/D931

AFFECTED FILES
  mercurial/merge.py

CHANGE DETAILS

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -1325,30 +1325,32 @@
 newactions.append((f, args, msg))
 mergeactions = newactions
 
-# premerge
-tocomplete = []
-for f, args, msg in mergeactions:
-repo.ui.debug(" %s: %s -> m (premerge)\n" % (f, msg))
-z += 1
-progress(_updating, z, item=f, total=numupdates, unit=_files)
-if f == '.hgsubstate': # subrepo states need updating
-subrepo.submerge(repo, wctx, mctx, wctx.ancestor(mctx),
- overwrite, labels)
-continue
-wctx[f].audit()
-complete, r = ms.preresolve(f, wctx)
-if not complete:
-numupdates += 1
-tocomplete.append((f, args, msg))
+try:
+# premerge
+tocomplete = []
+for f, args, msg in mergeactions:
+repo.ui.debug(" %s: %s -> m (premerge)\n" % (f, msg))
+z += 1
+progress(_updating, z, item=f, total=numupdates, unit=_files)
+if f == '.hgsubstate': # subrepo states need updating
+subrepo.submerge(repo, wctx, mctx, wctx.ancestor(mctx),
+ overwrite, labels)
+continue
+wctx[f].audit()
+complete, r = ms.preresolve(f, wctx)
+if not complete:
+numupdates += 1
+tocomplete.append((f, args, msg))
 
-# merge
-for f, args, msg in tocomplete:
-repo.ui.debug(" %s: %s -> m (merge)\n" % (f, msg))
-z += 1
-progress(_updating, z, item=f, total=numupdates, unit=_files)
-ms.resolve(f, wctx)
+# merge
+for f, args, msg in tocomplete:
+repo.ui.debug(" %s: %s -> m (merge)\n" % (f, msg))
+z += 1
+progress(_updating, z, item=f, total=numupdates, unit=_files)
+ms.resolve(f, wctx)
 
-ms.commit()
+finally:
+ms.commit()
 
 unresolved = ms.unresolvedcount()
 



To: ryanmce, #hg-reviewers, mbthomas
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D952: merge: additional test cases to show changing behavior

[ryanmce (Ryan McElroy)]

ryanmce updated this revision to Diff 2503.

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D952?vs=2455=2503

REVISION DETAIL
  https://phab.mercurial-scm.org/D952

AFFECTED FILES
  tests/test-merge-halt.t

CHANGE DETAILS

diff --git a/tests/test-merge-halt.t b/tests/test-merge-halt.t
--- a/tests/test-merge-halt.t
+++ b/tests/test-merge-halt.t
@@ -55,8 +55,13 @@
   rebase aborted
 
 Testing on-failure=prompt
-  $ echo on-failure=prompt >> $HGRCPATH
-  $ cat <> $HGRCPATH
+  > [merge]
+  > on-failure=prompt
+  > [ui]
+  > interactive=1
+  > EOS
+  $ cat < y
   > n
   > EOS
@@ -77,3 +82,83 @@
   $ hg rebase --abort
   rebase aborted
 
+Check that successful tool with failed post-check halts the merge
+  $ cat <> $HGRCPATH
+  > [merge-tools]
+  > true.check=changed
+  > EOS
+  $ cat < y
+  > n
+  > n
+  > EOS
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+   output file a appears unchanged
+  was merge successful (yn)? y
+   output file b appears unchanged
+  was merge successful (yn)? n
+  merging b failed!
+  continue merge operation (yn)? n
+  merge halted after failed merge (see hg resolve)
+  [1]
+
+  $ hg resolve --list
+  R a
+  U b
+
+  $ hg rebase --abort
+  rebase aborted
+
+Check that conflicts with conflict check also halts the merge
+  $ cat <> $HGRCPATH
+  > [merge-tools]
+  > true.check=conflicts
+  > true.premerge=keep
+  > [merge]
+  > on-failure=halt
+  > EOS
+  $ hg rebase -s 1 -d 2 --tool true
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+  merging a failed!
+  merge halted after failed merge (see hg resolve)
+  [1]
+
+  $ hg resolve --list
+  U a
+  U b
+
+  $ hg rebase --abort
+  rebase aborted
+
+Check that always-prompt also can halt the merge
+  $ cat < y
+  > n
+  > EOS
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+  was merge of 'a' successful (yn)? y
+  was merge of 'b' successful (yn)? n
+  merging b failed!
+  merge halted after failed merge (see hg resolve)
+  [1]
+
+  $ hg resolve --list
+  R a
+  U b
+
+  $ hg rebase --abort
+  rebase aborted
+
+Check that successful tool otherwise allows the merge to continue
+  $ hg rebase -s 1 -d 2 --tool echo --keep --config 
merge-tools.echo.premerge=keep
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+  $TESTTMP/repo/a *a~base* *a~other* (glob)
+  $TESTTMP/repo/b *b~base* *b~other* (glob)



To: ryanmce, #hg-reviewers, mbthomas
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D951: merge: allow user to halt merge on merge-tool failures

[ryanmce (Ryan McElroy)]

ryanmce updated this revision to Diff 2502.

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D951?vs=2454=2502

REVISION DETAIL
  https://phab.mercurial-scm.org/D951

AFFECTED FILES
  mercurial/filemerge.py
  tests/test-merge-halt.t

CHANGE DETAILS

diff --git a/tests/test-merge-halt.t b/tests/test-merge-halt.t
--- a/tests/test-merge-halt.t
+++ b/tests/test-merge-halt.t
@@ -44,8 +44,7 @@
   merging a
   merging b
   merging a failed!
-  merging b failed!
-  unresolved conflicts (see hg resolve, then hg rebase --continue)
+  merge halted after failed merge (see hg resolve)
   [1]
 
   $ hg resolve --list
@@ -65,8 +64,10 @@
   merging a
   merging b
   merging a failed!
+  continue merge operation (yn)? y
   merging b failed!
-  unresolved conflicts (see hg resolve, then hg rebase --continue)
+  continue merge operation (yn)? n
+  merge halted after failed merge (see hg resolve)
   [1]
 
   $ hg resolve --list
diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -715,6 +715,7 @@
 if r:
 if onfailure:
 ui.warn(onfailure % fd)
+_onfilemergefailure(ui)
 
 return True, r, deleted
 finally:



To: ryanmce, #hg-reviewers, mbthomas
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D953: merge: use filemerge failure handler in post-filemerge checks checks

[ryanmce (Ryan McElroy)]

ryanmce abandoned this revision.
ryanmce added a comment.


  It turns out this isn't needed -- setting the return to 1 means we catch the 
failure in the generic handler I added earlier. I fixed up the tests in the 
previous patch to include the important test changes from here (ie, more 
testing of the prompt stuff).

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D953

To: ryanmce, #hg-reviewers, mbthomas
Cc: mbthomas, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D930: merge: add tests to show current behavior on failed filemerges

[ryanmce (Ryan McElroy)]

ryanmce added inline comments.

INLINE COMMENTS

> quark wrote in test-merge-halt.t:32
> Should we have an `hg resolve -l` here?

sounds good

> quark wrote in test-merge-halt.t:36
> It feels a bit strange that the text says "works". Maybe use some neutral 
> words like "Testing onfailure=continue", "Testing onfailure=halt", and 
> "Testing onfailure=halt".

I like the suggestion. Will follow.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D930

To: ryanmce, #hg-reviewers
Cc: quark, krbullock, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D932: filemerge: introduce functions to halt merge flow

[ryanmce (Ryan McElroy)]

ryanmce added inline comments.

INLINE COMMENTS

> configitems.py:302
>  )
> +coreconfigitem('merge', 'onfailure',
> +default='continue',

this could be `on-failure`, per the comment on 
https://phab.mercurial-scm.org/D789

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D932

To: ryanmce, #hg-reviewers, krbullock, mbthomas
Cc: krbullock, pulkit, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D789: merge: add option to abort merge process on failure

[ryanmce (Ryan McElroy)]

ryanmce marked an inline comment as done.
ryanmce added inline comments.

INLINE COMMENTS

> markand wrote in configitems.py:302
> Please follow the naming style as defined in the UI Guidelines 
> . 
> Therefore it should be **abort-on-failure**.

Thanks! I didn't know about this new naming guideline. I definitely like it 
better than the old allthewordssmashedtogether guideline. Will update the 
current series.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D789

To: ryanmce, #hg-reviewers, quark, yuja, mbthomas
Cc: markand, mbthomas, yuja, quark, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel