[PATCH 1 of 2] patch: exclude diff opt from difffeatureopts by setting to false

[Piotr Listkiewicz]

Maybe other devs have opinion about it apart from our talk in irc,
especially because i still cant resolve the problem. The problem is
functions from scmutil.rev*() module needs revset module, this dependency
between scmutil -> revset causes problems with import cycle.

The question is what to do with it, one option would be to move  those
funtion to revset module, but from my talk with Yuja on irc it seems like
this would be an odd solution as revset is an implementation detail of
scmutil.rev*(). Another option would be to move those to other module, but
the question is where?

2016-04-26 18:01 GMT+02:00 Yuya Nishihara <yuya at tcha.org>:

> On Tue, 26 Apr 2016 12:32:34 +0200, Piotr Listkiewicz wrote:
> > > I'd rather make ctx.diff() accept diffopts object in place of **opts,
> which
> > > will allow us to utilize dedicated diffopts factory functions.
> > >   diffopts = patch.difffeatureopts(...)
> > >   ctx.diff(opts=diffopts)
> >
> > I like this idea, but i have one problem.
> >
> > In https://selenic.com/hg/file/78074575df2e/mercurial/revset.py#l1792 it
> > wants to pass git=True to ctx.diff parameters. In new solution it would
> > pass diffopts created with patch.diffopts(repo.ui, {"git": True}).
> >
> > The problem is revsets.py would need to import patch.py in which there
> are
> > diffopts factory methods which creates import cycle:
> >
> > mercurial.patch -> mercurial.scmutil -> mercurial.revset ->
> mercurial.patch
> >
> > In this moment the only solution i see is to move diffopts, diffallopts,
> > difffeatureopts from patch.py to another file but i have no idea if this
> is
> > acceptable. Maybe there is another simpler solution?
>
> Uh, no idea. Moving them to scmutil would be okay, but it never solve the
> cycle issue.
>
> It seems the dependency "scmutil -> revset" is likely to cause import
> cycle.
> Last time, IIRC, it was "scmutil -> revset -> merge.mergestate -> scmutil".
> That's probably because revset is the module getting bigger, whereas
> scmutil
> should sit in low layer.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20160504/df5dd46c/attachment.html>