D2577: mdiff: prefer xdiff for diff calculation

indygreg (Gregory Szorc) phabricator at mercurial-scm.org
Sat Mar 3 14:19:58 EST 2018


indygreg 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(...)

global state is evil. Especially on functionality that calls out to C and may release the GIL. If we have multi-threaded applications, things could get in a very bad state.

I'd encourage you to plumb in the diff algorithm to use into the diffopts. Even if the diff algorithm is only visible for user-visible diffs (as opposed to say storage diffs), that's a win.

REPOSITORY
  rHG Mercurial

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

To: ryanmce, #hg-reviewers
Cc: indygreg, quark, durin42, mercurial-devel


More information about the Mercurial-devel mailing list