| Submitter | Yuya Nishihara |
|---|---|
| Date | Feb. 7, 2018, 1:25 p.m. |
| Message ID | <5f9dcb5d72da427abbfa.1518009907@mimosa> |
| Download | mbox | patch |
| Permalink | /patch/27427/ |
| State | Accepted |
| Headers | show |
Comments
Yuya Nishihara a écrit : > # HG changeset patch > # User Yuya Nishihara <yuya@tcha.org> > # Date 1516517658 -32400 > # Sun Jan 21 15:54:18 2018 +0900 > # Node ID 5f9dcb5d72da427abbfa2c304bdbe4dd555e0c7d > # Parent f95d0d1e012a512550de945350e08f3dc7db090f > log: pack filematcher and hunksfilter into changesetdiffer object > > This is just a way of getting rid of clumsy makefilematcher/makehunksfilter > arguments. There might be a better abstraction, but I don't think this is bad. Alternatively we could have a makediffer(makefilematcher=None, makehunksfilter=None) factory function returning the showdiff function with capture context; that would avoid setting _make<attribute> in client code. Not sure this is better and I agree that your proposal is "not bad". Nice cleanup overall. > This makes filematcher and hunksfilter available by default, but that should > be fine. > > diff --git a/mercurial/commands.py b/mercurial/commands.py > --- a/mercurial/commands.py > +++ b/mercurial/commands.py > @@ -3419,17 +3419,15 @@ def log(ui, repo, *pats, **opts): > ) > > repo = scmutil.unhidehashlikerevs(repo, opts.get('rev'), 'nowarn') > - revs, filematcher = logcmdutil.getrevs(repo, pats, opts) > - hunksfilter = None > + revs, differ = logcmdutil.getrevs(repo, pats, opts) > > if opts.get('graph'): > if linerange: > raise error.Abort(_('graph not supported with line range patterns')) > - return logcmdutil.graphlog(ui, repo, revs, filematcher, opts) > + return logcmdutil.graphlog(ui, repo, revs, differ, opts) > > if linerange: > - revs, filematcher, hunksfilter = logcmdutil.getlinerangerevs( > - repo, revs, opts) > + revs, differ = logcmdutil.getlinerangerevs(repo, revs, opts) > > getrenamed = None > if opts.get('copies'): > @@ -3439,9 +3437,7 @@ def log(ui, repo, *pats, **opts): > getrenamed = templatekw.getrenamedfn(repo, endrev=endrev) > > ui.pager('log') > - displayer = logcmdutil.changesetdisplayer(ui, repo, opts, > - makefilematcher=filematcher, > - makehunksfilter=hunksfilter, > + displayer = logcmdutil.changesetdisplayer(ui, repo, opts, differ, > buffered=True) > for rev in revs: > ctx = repo[rev] > diff --git a/mercurial/logcmdutil.py b/mercurial/logcmdutil.py > --- a/mercurial/logcmdutil.py > +++ b/mercurial/logcmdutil.py > @@ -109,6 +109,23 @@ def diffordiffstat(ui, repo, diffopts, n > sub.diff(ui, diffopts, tempnode2, submatch, changes=changes, > stat=stat, fp=fp, prefix=prefix) > > +class changesetdiffer(object): > + """Generate diff of changeset with pre-configured filtering functions""" > + > + def _makefilematcher(self, ctx): > + return scmutil.matchall(ctx.repo()) > + > + def _makehunksfilter(self, ctx): > + return None > + > + def showdiff(self, ui, ctx, diffopts, stat=False): > + repo = ctx.repo() > + node = ctx.node() > + prev = ctx.p1().node() > + diffordiffstat(ui, repo, diffopts, prev, node, > + match=self._makefilematcher(ctx), stat=stat, > + hunksfilterfn=self._makehunksfilter(ctx)) > + > def changesetlabels(ctx): > labels = ['log.changeset', 'changeset.%s' % ctx.phasestr()] > if ctx.obsolete(): > @@ -122,13 +139,11 @@ def changesetlabels(ctx): > class changesetprinter(object): > '''show changeset information when templating not requested.''' > > - def __init__(self, ui, repo, makefilematcher=None, makehunksfilter=None, > - diffopts=None, buffered=False): > + def __init__(self, ui, repo, differ=None, diffopts=None, buffered=False): > self.ui = ui > self.repo = repo > self.buffered = buffered > - self._makefilematcher = makefilematcher or (lambda ctx: None) > - self._makehunksfilter = makehunksfilter or (lambda ctx: None) > + self._differ = differ or changesetdiffer() > self.diffopts = diffopts or {} > self.header = {} > self.hunk = {} > @@ -267,35 +282,23 @@ class changesetprinter(object): > ''' > > def _showpatch(self, ctx): > - matchfn = self._makefilematcher(ctx) > - hunksfilterfn = self._makehunksfilter(ctx) > - if not matchfn: > - return > stat = self.diffopts.get('stat') > diff = self.diffopts.get('patch') > diffopts = patch.diffallopts(self.ui, self.diffopts) > - node = ctx.node() > - prev = ctx.p1().node() > if stat: > - diffordiffstat(self.ui, self.repo, diffopts, prev, node, > - match=matchfn, stat=True, > - hunksfilterfn=hunksfilterfn) > + self._differ.showdiff(self.ui, ctx, diffopts, stat=True) > if stat and diff: > self.ui.write("\n") > if diff: > - diffordiffstat(self.ui, self.repo, diffopts, prev, node, > - match=matchfn, stat=False, > - hunksfilterfn=hunksfilterfn) > + self._differ.showdiff(self.ui, ctx, diffopts, stat=False) > if stat or diff: > self.ui.write("\n") > > class jsonchangeset(changesetprinter): > '''format changeset information.''' > > - def __init__(self, ui, repo, makefilematcher=None, makehunksfilter=None, > - diffopts=None, buffered=False): > - changesetprinter.__init__(self, ui, repo, makefilematcher, > - makehunksfilter, diffopts, buffered) > + def __init__(self, ui, repo, differ=None, diffopts=None, buffered=False): > + changesetprinter.__init__(self, ui, repo, differ, diffopts, buffered) > self.cache = {} > self._first = True > > @@ -370,21 +373,17 @@ class jsonchangeset(changesetprinter): > ", ".join('"%s": "%s"' % (j(k), j(v)) > for k, v in copies)) > > - matchfn = self._makefilematcher(ctx) > stat = self.diffopts.get('stat') > diff = self.diffopts.get('patch') > diffopts = patch.difffeatureopts(self.ui, self.diffopts, git=True) > - node, prev = ctx.node(), ctx.p1().node() > - if matchfn and stat: > + if stat: > self.ui.pushbuffer() > - diffordiffstat(self.ui, self.repo, diffopts, prev, node, > - match=matchfn, stat=True) > + self._differ.showdiff(self.ui, ctx, diffopts, stat=True) > self.ui.write((',\n "diffstat": "%s"') > % j(self.ui.popbuffer())) > - if matchfn and diff: > + if diff: > self.ui.pushbuffer() > - diffordiffstat(self.ui, self.repo, diffopts, prev, node, > - match=matchfn, stat=False) > + self._differ.showdiff(self.ui, ctx, diffopts, stat=False) > self.ui.write((',\n "diff": "%s"') % j(self.ui.popbuffer())) > > self.ui.write("\n }") > @@ -400,10 +399,9 @@ class changesettemplater(changesetprinte > > # Arguments before "buffered" used to be positional. Consider not > # adding/removing arguments before "buffered" to not break callers. > - def __init__(self, ui, repo, tmplspec, makefilematcher=None, > - makehunksfilter=None, diffopts=None, buffered=False): > - changesetprinter.__init__(self, ui, repo, makefilematcher, > - makehunksfilter, diffopts, buffered) > + def __init__(self, ui, repo, tmplspec, differ=None, diffopts=None, > + buffered=False): > + changesetprinter.__init__(self, ui, repo, differ, diffopts, buffered) > tres = formatter.templateresources(ui, repo) > self.t = formatter.loadtemplater(ui, tmplspec, > defaults=templatekw.keywords, > @@ -520,8 +518,7 @@ def maketemplater(ui, repo, tmpl, buffer > spec = templatespec(tmpl, None) > return changesettemplater(ui, repo, spec, buffered=buffered) > > -def changesetdisplayer(ui, repo, opts, makefilematcher=None, > - makehunksfilter=None, buffered=False): > +def changesetdisplayer(ui, repo, opts, differ=None, buffered=False): > """show one changeset using template or regular display. > > Display format will be the first non-empty hit of: > @@ -532,12 +529,7 @@ def changesetdisplayer(ui, repo, opts, m > If all of these values are either the unset or the empty string, > regular display via changesetprinter() is done. > """ > - # options > - if not makefilematcher and (opts.get('patch') or opts.get('stat')): > - def makefilematcher(ctx): > - return scmutil.matchall(repo) > - > - postargs = (makefilematcher, makehunksfilter, opts, buffered) > + postargs = (differ, opts, buffered) > if opts.get('template') == 'json': > return jsonchangeset(ui, repo, *postargs) > > @@ -713,10 +705,9 @@ def _initialrevs(repo, opts): > return revs > > def getrevs(repo, pats, opts): > - """Return (revs, filematcher) where revs is a smartset > + """Return (revs, differ) where revs is a smartset > > - filematcher is a callable taking a changectx and returning a match > - objects filtering the files to be detailed when displaying the revision. > + differ is a changesetdiffer with pre-configured file matcher. > """ > follow = opts.get('follow') or opts.get('follow_first') > followfirst = opts.get('follow_first') > @@ -749,7 +740,10 @@ def getrevs(repo, pats, opts): > revs = matcher(repo, revs) > if limit is not None: > revs = revs.slice(0, limit) > - return revs, filematcher > + > + differ = changesetdiffer() > + differ._makefilematcher = filematcher > + return revs, differ > > def _parselinerangeopt(repo, opts): > """Parse --line-range log option and return a list of tuples (filename, > @@ -772,16 +766,13 @@ def _parselinerangeopt(repo, opts): > return linerangebyfname > > def getlinerangerevs(repo, userrevs, opts): > - """Return (revs, filematcher, hunksfilter). > + """Return (revs, differ). > > "revs" are revisions obtained by processing "line-range" log options and > walking block ancestors of each specified file/line-range. > > - "filematcher(ctx) -> match" is a factory function returning a match object > - for a given revision for file patterns specified in --line-range option. > - > - "hunksfilter(ctx) -> filterfn(fctx, hunks)" is a factory function > - returning a hunks filtering function. > + "differ" is a changesetdiffer with pre-configured file matcher and hunks > + filter. > """ > wctx = repo[None] > > @@ -830,7 +821,10 @@ def getlinerangerevs(repo, userrevs, opt > > revs = sorted(linerangesbyrev, reverse=True) > > - return revs, filematcher, hunksfilter > + differ = changesetdiffer() > + differ._makefilematcher = filematcher > + differ._makehunksfilter = hunksfilter > + return revs, differ > > def _graphnodeformatter(ui, displayer): > spec = ui.config('ui', 'graphnodetemplate') > @@ -897,7 +891,7 @@ def displaygraph(ui, repo, dag, displaye > lines = [] > displayer.close() > > -def graphlog(ui, repo, revs, filematcher, opts): > +def graphlog(ui, repo, revs, differ, opts): > # Parameters are identical to log command ones > revdag = graphmod.dagwalker(repo, revs) > > @@ -909,8 +903,7 @@ def graphlog(ui, repo, revs, filematcher > getrenamed = templatekw.getrenamedfn(repo, endrev=endrev) > > ui.pager('log') > - displayer = changesetdisplayer(ui, repo, opts, makefilematcher=filematcher, > - buffered=True) > + displayer = changesetdisplayer(ui, repo, opts, differ, buffered=True) > displaygraph(ui, repo, revdag, displayer, graphmod.asciiedges, getrenamed) > > def checkunsupportedgraphflags(pats, opts): > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel >
On Wed, 7 Feb 2018 15:58:14 +0100, Denis Laxalde wrote: > Yuya Nishihara a écrit : > > # HG changeset patch > > # User Yuya Nishihara <yuya@tcha.org> > > # Date 1516517658 -32400 > > # Sun Jan 21 15:54:18 2018 +0900 > > # Node ID 5f9dcb5d72da427abbfa2c304bdbe4dd555e0c7d > > # Parent f95d0d1e012a512550de945350e08f3dc7db090f > > log: pack filematcher and hunksfilter into changesetdiffer object > > > > This is just a way of getting rid of clumsy makefilematcher/makehunksfilter > > arguments. There might be a better abstraction, but I don't think this is bad. > > Alternatively we could have a makediffer(makefilematcher=None, > makehunksfilter=None) factory function returning the showdiff function > with capture context; that would avoid setting _make<attribute> in > client code. Yeah, that's doable. But I slightly prefer a class over a set of closures. I mean it might be possible to factor out makefilematcher/makehunksfilter to a class holding a repo, instead of overwriting object attributes. Anyway, thanks for reviewing.
On Wed, Feb 07, 2018 at 10:25:07PM +0900, Yuya Nishihara wrote: > # HG changeset patch > # User Yuya Nishihara <yuya@tcha.org> > # Date 1516517658 -32400 > # Sun Jan 21 15:54:18 2018 +0900 > # Node ID 5f9dcb5d72da427abbfa2c304bdbe4dd555e0c7d > # Parent f95d0d1e012a512550de945350e08f3dc7db090f > log: pack filematcher and hunksfilter into changesetdiffer object queued, thanks
Patch
diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -3419,17 +3419,15 @@ def log(ui, repo, *pats, **opts): ) repo = scmutil.unhidehashlikerevs(repo, opts.get('rev'), 'nowarn') - revs, filematcher = logcmdutil.getrevs(repo, pats, opts) - hunksfilter = None + revs, differ = logcmdutil.getrevs(repo, pats, opts) if opts.get('graph'): if linerange: raise error.Abort(_('graph not supported with line range patterns')) - return logcmdutil.graphlog(ui, repo, revs, filematcher, opts) + return logcmdutil.graphlog(ui, repo, revs, differ, opts) if linerange: - revs, filematcher, hunksfilter = logcmdutil.getlinerangerevs( - repo, revs, opts) + revs, differ = logcmdutil.getlinerangerevs(repo, revs, opts) getrenamed = None if opts.get('copies'): @@ -3439,9 +3437,7 @@ def log(ui, repo, *pats, **opts): getrenamed = templatekw.getrenamedfn(repo, endrev=endrev) ui.pager('log') - displayer = logcmdutil.changesetdisplayer(ui, repo, opts, - makefilematcher=filematcher, - makehunksfilter=hunksfilter, + displayer = logcmdutil.changesetdisplayer(ui, repo, opts, differ, buffered=True) for rev in revs: ctx = repo[rev] diff --git a/mercurial/logcmdutil.py b/mercurial/logcmdutil.py --- a/mercurial/logcmdutil.py +++ b/mercurial/logcmdutil.py @@ -109,6 +109,23 @@ def diffordiffstat(ui, repo, diffopts, n sub.diff(ui, diffopts, tempnode2, submatch, changes=changes, stat=stat, fp=fp, prefix=prefix) +class changesetdiffer(object): + """Generate diff of changeset with pre-configured filtering functions""" + + def _makefilematcher(self, ctx): + return scmutil.matchall(ctx.repo()) + + def _makehunksfilter(self, ctx): + return None + + def showdiff(self, ui, ctx, diffopts, stat=False): + repo = ctx.repo() + node = ctx.node() + prev = ctx.p1().node() + diffordiffstat(ui, repo, diffopts, prev, node, + match=self._makefilematcher(ctx), stat=stat, + hunksfilterfn=self._makehunksfilter(ctx)) + def changesetlabels(ctx): labels = ['log.changeset', 'changeset.%s' % ctx.phasestr()] if ctx.obsolete(): @@ -122,13 +139,11 @@ def changesetlabels(ctx): class changesetprinter(object): '''show changeset information when templating not requested.''' - def __init__(self, ui, repo, makefilematcher=None, makehunksfilter=None, - diffopts=None, buffered=False): + def __init__(self, ui, repo, differ=None, diffopts=None, buffered=False): self.ui = ui self.repo = repo self.buffered = buffered - self._makefilematcher = makefilematcher or (lambda ctx: None) - self._makehunksfilter = makehunksfilter or (lambda ctx: None) + self._differ = differ or changesetdiffer() self.diffopts = diffopts or {} self.header = {} self.hunk = {} @@ -267,35 +282,23 @@ class changesetprinter(object): ''' def _showpatch(self, ctx): - matchfn = self._makefilematcher(ctx) - hunksfilterfn = self._makehunksfilter(ctx) - if not matchfn: - return stat = self.diffopts.get('stat') diff = self.diffopts.get('patch') diffopts = patch.diffallopts(self.ui, self.diffopts) - node = ctx.node() - prev = ctx.p1().node() if stat: - diffordiffstat(self.ui, self.repo, diffopts, prev, node, - match=matchfn, stat=True, - hunksfilterfn=hunksfilterfn) + self._differ.showdiff(self.ui, ctx, diffopts, stat=True) if stat and diff: self.ui.write("\n") if diff: - diffordiffstat(self.ui, self.repo, diffopts, prev, node, - match=matchfn, stat=False, - hunksfilterfn=hunksfilterfn) + self._differ.showdiff(self.ui, ctx, diffopts, stat=False) if stat or diff: self.ui.write("\n") class jsonchangeset(changesetprinter): '''format changeset information.''' - def __init__(self, ui, repo, makefilematcher=None, makehunksfilter=None, - diffopts=None, buffered=False): - changesetprinter.__init__(self, ui, repo, makefilematcher, - makehunksfilter, diffopts, buffered) + def __init__(self, ui, repo, differ=None, diffopts=None, buffered=False): + changesetprinter.__init__(self, ui, repo, differ, diffopts, buffered) self.cache = {} self._first = True @@ -370,21 +373,17 @@ class jsonchangeset(changesetprinter): ", ".join('"%s": "%s"' % (j(k), j(v)) for k, v in copies)) - matchfn = self._makefilematcher(ctx) stat = self.diffopts.get('stat') diff = self.diffopts.get('patch') diffopts = patch.difffeatureopts(self.ui, self.diffopts, git=True) - node, prev = ctx.node(), ctx.p1().node() - if matchfn and stat: + if stat: self.ui.pushbuffer() - diffordiffstat(self.ui, self.repo, diffopts, prev, node, - match=matchfn, stat=True) + self._differ.showdiff(self.ui, ctx, diffopts, stat=True) self.ui.write((',\n "diffstat": "%s"') % j(self.ui.popbuffer())) - if matchfn and diff: + if diff: self.ui.pushbuffer() - diffordiffstat(self.ui, self.repo, diffopts, prev, node, - match=matchfn, stat=False) + self._differ.showdiff(self.ui, ctx, diffopts, stat=False) self.ui.write((',\n "diff": "%s"') % j(self.ui.popbuffer())) self.ui.write("\n }") @@ -400,10 +399,9 @@ class changesettemplater(changesetprinte # Arguments before "buffered" used to be positional. Consider not # adding/removing arguments before "buffered" to not break callers. - def __init__(self, ui, repo, tmplspec, makefilematcher=None, - makehunksfilter=None, diffopts=None, buffered=False): - changesetprinter.__init__(self, ui, repo, makefilematcher, - makehunksfilter, diffopts, buffered) + def __init__(self, ui, repo, tmplspec, differ=None, diffopts=None, + buffered=False): + changesetprinter.__init__(self, ui, repo, differ, diffopts, buffered) tres = formatter.templateresources(ui, repo) self.t = formatter.loadtemplater(ui, tmplspec, defaults=templatekw.keywords, @@ -520,8 +518,7 @@ def maketemplater(ui, repo, tmpl, buffer spec = templatespec(tmpl, None) return changesettemplater(ui, repo, spec, buffered=buffered) -def changesetdisplayer(ui, repo, opts, makefilematcher=None, - makehunksfilter=None, buffered=False): +def changesetdisplayer(ui, repo, opts, differ=None, buffered=False): """show one changeset using template or regular display. Display format will be the first non-empty hit of: @@ -532,12 +529,7 @@ def changesetdisplayer(ui, repo, opts, m If all of these values are either the unset or the empty string, regular display via changesetprinter() is done. """ - # options - if not makefilematcher and (opts.get('patch') or opts.get('stat')): - def makefilematcher(ctx): - return scmutil.matchall(repo) - - postargs = (makefilematcher, makehunksfilter, opts, buffered) + postargs = (differ, opts, buffered) if opts.get('template') == 'json': return jsonchangeset(ui, repo, *postargs) @@ -713,10 +705,9 @@ def _initialrevs(repo, opts): return revs def getrevs(repo, pats, opts): - """Return (revs, filematcher) where revs is a smartset + """Return (revs, differ) where revs is a smartset - filematcher is a callable taking a changectx and returning a match - objects filtering the files to be detailed when displaying the revision. + differ is a changesetdiffer with pre-configured file matcher. """ follow = opts.get('follow') or opts.get('follow_first') followfirst = opts.get('follow_first') @@ -749,7 +740,10 @@ def getrevs(repo, pats, opts): revs = matcher(repo, revs) if limit is not None: revs = revs.slice(0, limit) - return revs, filematcher + + differ = changesetdiffer() + differ._makefilematcher = filematcher + return revs, differ def _parselinerangeopt(repo, opts): """Parse --line-range log option and return a list of tuples (filename, @@ -772,16 +766,13 @@ def _parselinerangeopt(repo, opts): return linerangebyfname def getlinerangerevs(repo, userrevs, opts): - """Return (revs, filematcher, hunksfilter). + """Return (revs, differ). "revs" are revisions obtained by processing "line-range" log options and walking block ancestors of each specified file/line-range. - "filematcher(ctx) -> match" is a factory function returning a match object - for a given revision for file patterns specified in --line-range option. - - "hunksfilter(ctx) -> filterfn(fctx, hunks)" is a factory function - returning a hunks filtering function. + "differ" is a changesetdiffer with pre-configured file matcher and hunks + filter. """ wctx = repo[None] @@ -830,7 +821,10 @@ def getlinerangerevs(repo, userrevs, opt revs = sorted(linerangesbyrev, reverse=True) - return revs, filematcher, hunksfilter + differ = changesetdiffer() + differ._makefilematcher = filematcher + differ._makehunksfilter = hunksfilter + return revs, differ def _graphnodeformatter(ui, displayer): spec = ui.config('ui', 'graphnodetemplate') @@ -897,7 +891,7 @@ def displaygraph(ui, repo, dag, displaye lines = [] displayer.close() -def graphlog(ui, repo, revs, filematcher, opts): +def graphlog(ui, repo, revs, differ, opts): # Parameters are identical to log command ones revdag = graphmod.dagwalker(repo, revs) @@ -909,8 +903,7 @@ def graphlog(ui, repo, revs, filematcher getrenamed = templatekw.getrenamedfn(repo, endrev=endrev) ui.pager('log') - displayer = changesetdisplayer(ui, repo, opts, makefilematcher=filematcher, - buffered=True) + displayer = changesetdisplayer(ui, repo, opts, differ, buffered=True) displaygraph(ui, repo, revdag, displayer, graphmod.asciiedges, getrenamed) def checkunsupportedgraphflags(pats, opts):