D623: copytrace: move fast heuristic copytracing algorithm to core

2017-09-07 Thread pulkit (Pulkit Goyal)
pulkit planned changes to this revision. pulkit added inline comments. INLINE COMMENTS > yuja wrote in copies.py:608 > This cdst/csrc naming is confusing because c1 is actually the > source revision (= the original wctx) in "update" scenario. And > IIUC, we are searching for copies from c1 to

D624: copytrace: move the full copytracing algorithm under 'full' option

2017-09-07 Thread pulkit (Pulkit Goyal)
pulkit abandoned this revision. pulkit added inline comments. INLINE COMMENTS > yuja wrote in copies.py:376 > This should either abort or fall back to the default copy tracing. > Returning None doesn't make sense. Falling back to default copytracing make sense. I am dropping this revision as

D613: dispatch: store the command name which is going to run in ui object

2017-09-07 Thread yuja (Yuya Nishihara)
yuja added a comment. In https://phab.mercurial-scm.org/D613#10467, @quark wrote: > This is for directaccess as directaccess originally designed (https://www.mercurial-scm.org/repo/evolve/rev/b8f880d417) > > It may be cleaner to use a flag of `scmutil.rev*` instead. So commands

D623: copytrace: move fast heuristic copytracing algorithm to core

2017-09-07 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > pulkit wrote in copies.py:638 > I am sorry but I didn't understand the `base were in other side` thing. Did > you mean base is a child of ctx? //In theory//, base could be anywhere between c1 and c2. If it belonged to the c1 branch,

D627: filemerge: flush if using deferred writes when running a merge tool

2017-09-07 Thread martinvonz (Martin von Zweigbergk)
martinvonz added inline comments. INLINE COMMENTS > filemerge.py:500-503 > +# Must flush any deferred contents if running a merge tool. > +from . import context > +if isinstance(wctx, context.overlayworkingctx): > +wctx.flushall() As Phil and I talked about out-of-band, it

[PATCH 1 of 2] debuginstall: use codecs.lookup() to detect invalid encoding

2017-09-07 Thread Yuya Nishihara
# HG changeset patch # User Yuya Nishihara # Date 1504790843 -32400 # Thu Sep 07 22:27:23 2017 +0900 # Node ID 5fda73a5468d3789bccdcb2da6d6b56e5ca2410b # Parent 1104718fb0907a4a841e6a24006c0c7fcb9caa9e debuginstall: use codecs.lookup() to detect invalid encoding

D637: check-code: fix incorrect capitalization in camelcase regex

2017-09-07 Thread martinvonz (Martin von Zweigbergk)
This revision was automatically updated to reflect the committed changes. Closed by commit rHGba6e14f9a2d8: check-code: fix incorrect capitalization in camelcase regex (authored by martinvonz). REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE

D660: patchbomb: add test that shows --to and --cc override matching config item

2017-09-07 Thread durin42 (Augie Fackler)
durin42 created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REVISION SUMMARY As far as I know this has always been true and is intentional (it's in line with many other behaviors), but it wasn't tested. Since I'm about to tweak To and Cc

D616: context: add overlayworkingcontext and overlayworkingfilectx

2017-09-07 Thread martinvonz (Martin von Zweigbergk)
martinvonz added inline comments. INLINE COMMENTS > context.py:1987-1989 > +If `exists` is True, either `data` or `flags` must be non-None (either > both, > +or just `flags`), and 'date' is non-None. If it is `False`, the file was > +deleted. It sounds like a shorter version of

D660: patchbomb: add test that shows --to and --cc override matching config item

2017-09-07 Thread durin42 (Augie Fackler)
durin42 added a comment. This is an email reply. Will it work? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D660 To: durin42, #hg-reviewers Cc: mercurial-devel ___ Mercurial-devel mailing list

D659: patchbomb: add test that shows --to and --cc override matching config item

2017-09-07 Thread durin42 (Augie Fackler)
durin42 created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REVISION SUMMARY As far as I know this has always been true and is intentional (it's in line with many other behaviors), but it wasn't tested. Since I'm about to tweak To and Cc

D636: cmdutil: remove the redundant commit during amend

2017-09-07 Thread singhsrb (Saurabh Singh)
This revision was automatically updated to reflect the committed changes. Closed by commit rHGe8a7c1a0565a: cmdutil: remove the redundant commit during amend (authored by singhsrb). CHANGED PRIOR TO COMMIT https://phab.mercurial-scm.org/D636?vs=1630=1676#toc REPOSITORY rHG Mercurial

D635: cmdutil: remove redundant commitfunc parameter in amend (API)

2017-09-07 Thread singhsrb (Saurabh Singh)
This revision was automatically updated to reflect the committed changes. Closed by commit rHGa39dce4a76b8: cmdutil: remove redundant commitfunc parameter in amend (API) (authored by singhsrb). REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE

D636: cmdutil: remove the redundant commit during amend

2017-09-07 Thread martinvonz (Martin von Zweigbergk)
martinvonz added a comment. I fixed a few nits in flight and queued this. Thanks for cleaning this up! INLINE COMMENTS > cmdutil.py:3074-3078 > +matcher = scmutil.match(wctx, pats, opts) > +if (opts.get('addremove') > +and scmutil.addremove(repo, matcher, "",

FYI: new emails from phabricator can be replied to

2017-09-07 Thread Augie Fackler
Kevin (TheMystic) and I finally figured out the requisite incantation to get phabricator emails to be reply-to-able. Unfortunately, the email handling in phabricator means that this will only work for *new* emails sent by phabricator, so you can only reply to new messages (more recent than

[survey] How do you feel about Phabricator? Please take a survey!

2017-09-07 Thread Augie Fackler
If you don't care about how code contribution and review happens for Mercurial, you can stop reading now. As you've probably noticed, we're experimenting with using Phabricator for contributing and reviewing patches. We're at a point now where we'd love to hear more broadly how people feel

D628: merge: flush any deferred writes before, and after, running any workers

2017-09-07 Thread phillco (Phil Cohen)
phillco updated this revision to Diff 1669. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D628?vs=1609=1669 REVISION DETAIL https://phab.mercurial-scm.org/D628 AFFECTED FILES mercurial/merge.py CHANGE DETAILS diff --git a/mercurial/merge.py

D627: filemerge: flush if using deferred writes when running a merge tool

2017-09-07 Thread phillco (Phil Cohen)
phillco updated this revision to Diff 1668. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D627?vs=1608=1668 REVISION DETAIL https://phab.mercurial-scm.org/D627 AFFECTED FILES mercurial/filemerge.py CHANGE DETAILS diff --git a/mercurial/filemerge.py

[PATCH 2 of 2] debuginstall: do not pass exception object to formatter (issue5676)

2017-09-07 Thread Yuya Nishihara
# HG changeset patch # User Yuya Nishihara # Date 1504791414 -32400 # Thu Sep 07 22:36:54 2017 +0900 # Node ID 3573b95ec1f32805bde3a08b1f4f7dca8e3d699f # Parent 5fda73a5468d3789bccdcb2da6d6b56e5ca2410b debuginstall: do not pass exception object to formatter (issue5676) diff

D612: directaccess: add a hiddenlevel argument to registrar.command

2017-09-07 Thread yuja (Yuya Nishihara)
yuja added inline comments. INLINE COMMENTS > durham wrote in registrar.py:142 > I think an enum would be better here (UNRECOVERABLE_WRITE, RECOVERABLE_WRITE, > READ_ONLY). Especially because I think people generally copy and paste the > registrar decorators from other functions, and if we're

D639: amend: use context manager for config override

2017-09-07 Thread martinvonz (Martin von Zweigbergk)
This revision was automatically updated to reflect the committed changes. Closed by commit rHG5dc6ac6555e6: amend: use context manager for config override (authored by martinvonz). REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D639?vs=1636=1661 REVISION

D642: checknlink: rename file object from 'fd' to 'fp'

2017-09-07 Thread quark (Jun Wu)
This revision was automatically updated to reflect the committed changes. Closed by commit rHG6c5cdb02f2f9: checknlink: rename file object from 'fd' to 'fp' (authored by quark). REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D642?vs=1639=1664 REVISION

D638: amend: delete dead assignment to "newid"

2017-09-07 Thread martinvonz (Martin von Zweigbergk)
This revision was automatically updated to reflect the committed changes. Closed by commit rHG4059b10d7490: amend: delete dead assignment to "newid" (authored by martinvonz). REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D638?vs=1635=1660 REVISION DETAIL

D641: cleanup: rename "matchfn" to "match" where obviously a matcher

2017-09-07 Thread martinvonz (Martin von Zweigbergk)
This revision was automatically updated to reflect the committed changes. Closed by commit rHG08346a8fa65f: cleanup: rename "matchfn" to "match" where obviously a matcher (authored by martinvonz). REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE

D656: revset: remove "small" argument from "_optimize"

2017-09-07 Thread quark (Jun Wu)
quark created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REVISION SUMMARY `_optimize` calculates weights of subtrees. "small" affects some weight calculation (either 1 or 0). The weights are now only useful in `and` optimization where

D657: revset: move weight information to predicate

2017-09-07 Thread quark (Jun Wu)
quark created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REVISION SUMMARY Previously revset weight is hardcoded and cannot be modified. This patch moves it to predicate so newly registered revsets could define their weight to properly

D450: filemerge: add wctx to all internal tools

2017-09-07 Thread phillco (Phil Cohen)
phillco abandoned this revision. phillco added a subscriber: martinvonz. phillco added a comment. No longer needed thanks to @martinvonz's careful eye. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D450 To: phillco, #hg-reviewers Cc: martinvonz, mercurial-devel

D627: filemerge: flush if using deferred writes when running a merge tool

2017-09-07 Thread phillco (Phil Cohen)
phillco updated this revision to Diff 1671. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D627?vs=1668=1671 REVISION DETAIL https://phab.mercurial-scm.org/D627 AFFECTED FILES mercurial/filemerge.py CHANGE DETAILS diff --git a/mercurial/filemerge.py

D613: dispatch: store the command name which is going to run in ui object

2017-09-07 Thread quark (Jun Wu)
quark added a comment. In https://phab.mercurial-scm.org/D613#10670, @yuja wrote: > Perhaps it could be a read-only flag (or class) set to repo object? Yes. It seems `repo.filtername` may be a reasonable choice. REPOSITORY rHG Mercurial REVISION DETAIL

D628: merge: flush any deferred writes before, and after, running any workers

2017-09-07 Thread phillco (Phil Cohen)
phillco updated this revision to Diff 1672. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D628?vs=1669=1672 REVISION DETAIL https://phab.mercurial-scm.org/D628 AFFECTED FILES mercurial/merge.py CHANGE DETAILS diff --git a/mercurial/merge.py

D449: merge: pass wctx to premerge, filemerge

2017-09-07 Thread phillco (Phil Cohen)
phillco updated this revision to Diff 1670. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D449?vs=1298=1670 REVISION DETAIL https://phab.mercurial-scm.org/D449 AFFECTED FILES hgext/largefiles/overrides.py mercurial/filemerge.py mercurial/merge.py

D449: merge: pass wctx to premerge, filemerge

2017-09-07 Thread phillco (Phil Cohen)
phillco added a comment. I thought this wasn't needed, but it is. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D449 To: phillco, #hg-reviewers Cc: mercurial-devel ___ Mercurial-devel mailing list

D657: revset: move weight information to predicate

2017-09-07 Thread phillco (Phil Cohen)
phillco accepted this revision. phillco added a comment. lgtm INLINE COMMENTS > revset.py:256 > # x - argument in tree form > -symbols = {} > +symbols = revsetlang.symbols > I take it this dictionary was moved there because of the import direction? REPOSITORY rHG Mercurial REVISION

Re: FYI: new emails from phabricator can be replied to

2017-09-07 Thread Jun Wu
Excerpts from Augie Fackler's message of 2017-09-07 14:54:56 -0400: > Hopefully this means if you don't have a phabricator account, you can > still get comments attached to phabricator-sent patches. By default Phabricator requires "From" address to match an existing user. I think we can patch

D632: wrapfunction: use functools.partial if possible

2017-09-07 Thread martinvonz (Martin von Zweigbergk)
martinvonz added inline comments. INLINE COMMENTS > extensions.py:464 > assert callable(origfn) > -wrap = bind(wrapper, origfn) > +if util.safehasattr(origfn, 'im_self'): > +# traditional bind, work with bound and unbound instancemethod, but This patch makes

D632: wrapfunction: use functools.partial if possible

2017-09-07 Thread quark (Jun Wu)
quark marked an inline comment as done. quark added a comment. Fixed. It's tricker than I though - Python 3 does not have "unbound function" concept and there is no easy way to detect "unbound function". So I changed approach to check `ismodule` instead, which seems to be more conservative

D633: wrapcommand: use functools.partial

2017-09-07 Thread quark (Jun Wu)
quark updated this revision to Diff 1679. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D633?vs=1620=1679 REVISION DETAIL https://phab.mercurial-scm.org/D633 AFFECTED FILES mercurial/extensions.py CHANGE DETAILS diff --git a/mercurial/extensions.py

D632: wrapfunction: use functools.partial if possible

2017-09-07 Thread quark (Jun Wu)
quark updated this revision to Diff 1678. quark edited the summary of this revision. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D632?vs=1619=1678 REVISION DETAIL https://phab.mercurial-scm.org/D632 AFFECTED FILES mercurial/dispatch.py

D657: revset: move weight information to predicate

2017-09-07 Thread quark (Jun Wu)
quark added inline comments. INLINE COMMENTS > phillco wrote in revset.py:256 > I take it this dictionary was moved there because of the import direction? Yes. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D657 To: quark, #hg-reviewers, phillco Cc: phillco,

Re: FYI: new emails from phabricator can be replied to

2017-09-07 Thread Jun Wu
Excerpts from Jun Wu's message of 2017-09-07 13:24:27 -0700: > Excerpts from Augie Fackler's message of 2017-09-07 14:54:56 -0400: > > Hopefully this means if you don't have a phabricator account, you can > > still get comments attached to phabricator-sent patches. > > By default Phabricator

D650: blackbox: do not cache file objects

2017-09-07 Thread durham (Durham Goode)
durham accepted this revision. durham added inline comments. INLINE COMMENTS > blackbox.py:188 > +# do not restore _bbinlog intentionally to avoid failed > +# logging again > +else: Might be worth mentioning this behavior change in the

D614: directaccess: make the hiddenlevel an attribute of the function

2017-09-07 Thread durham (Durham Goode)
durham added inline comments. INLINE COMMENTS > registrar.py:152 > +if hiddenlevel not in set([0, 1, 2]): > +hiddenlevel = 0 > +func.hiddenlevel = hiddenlevel Should we throw an exception instead? Seems like a programmer error REPOSITORY rHG Mercurial REVISION

D615: directaccess: store the access level in the ui object in dispatch

2017-09-07 Thread durham (Durham Goode)
durham added inline comments. INLINE COMMENTS > test-basic.t:4 >$ hg config > + extensions.fsmonitor= (fsmonitor !) > + _internal.hiddenlevel=0 Any idea why the order changed? Same for below REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D615 To: pulkit,

D648: blackbox: fix rotation with chg

2017-09-07 Thread quark (Jun Wu)
quark added a comment. I think the original patch was trying to make different `ui` objects use a same file object if their `blackbox.log` path is the same. In theory it could also be problematic in the rotation case. Anyway, that becomes necessary after https://phab.mercurial-scm.org/D650.

D644: rebase: remove complex unhiding code

2017-09-07 Thread durham (Durham Goode)
durham added a comment. I'd love to get rid of hidden filtering everywhere. Just limit knowledge of "hidden" to getting the list of visible heads, and algorithms that need to traverse children. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D644 To: quark,

D644: rebase: remove complex unhiding code

2017-09-07 Thread durham (Durham Goode)
durham added a comment. This sound reasonable, assuming all descendant/children computations are done before prepared is set. Could you run the fb-hgext inhibit/fbamend tests with this change? Since they might do some more interesting acrobatics around visibility changes. REPOSITORY

D643: rebase: use unfiltered repo when loading state

2017-09-07 Thread quark (Jun Wu)
quark updated this revision to Diff 1680. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D643?vs=1641=1680 REVISION DETAIL https://phab.mercurial-scm.org/D643 AFFECTED FILES hgext/rebase.py tests/test-rebase-obsolete.t CHANGE DETAILS diff --git

D648: blackbox: fix rotation with chg

2017-09-07 Thread durham (Durham Goode)
durham added a comment. This seems like mostly a revert of the original commit. timeless might have to chime in here, since it's not clear exactly what issue the original patch was fixing. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D648 To: quark,

D655: blackbox: set lastui even if ui.log is not called (issue5518)

2017-09-07 Thread durham (Durham Goode)
durham accepted this revision. durham added a comment. It might be worth mentioning what lastui is use for in the commit message. I had to look at the code a little to remember what lastui did and why this was safe. REPOSITORY rHG Mercurial REVISION DETAIL

D633: wrapcommand: use functools.partial

2017-09-07 Thread martinvonz (Martin von Zweigbergk)
martinvonz added a comment. Seems unlikely enough that someone passes a bound method as command wrapper. Queued. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D633 To: quark, #hg-reviewers, phillco Cc: martinvonz, mercurial-devel

D632: wrapfunction: use functools.partial if possible

2017-09-07 Thread quark (Jun Wu)
This revision was automatically updated to reflect the committed changes. Closed by commit rHG5361771f9714: wrapfunction: use functools.partial if possible (authored by quark). REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D632?vs=1678=1681 REVISION

D633: wrapcommand: use functools.partial

2017-09-07 Thread quark (Jun Wu)
This revision was automatically updated to reflect the committed changes. Closed by commit rHGa763c891f36e: wrapcommand: use functools.partial (authored by quark). REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D633?vs=1679=1682 REVISION DETAIL

D615: directaccess: store the access level in the ui object in dispatch

2017-09-07 Thread quark (Jun Wu)
quark requested changes to this revision. quark added a comment. This revision now requires changes to proceed. Setting `ui` config seems like an abuse of the config system. I think it'll be cleaner if feed the right "repo" object directly to the command function. i.e. By default commands