| Submitter | Martin von Zweigbergk |
|---|---|
| Date | Nov. 9, 2015, 11:06 p.m. |
| Message ID | <6c4a3f70880b72538c47.1447110405@waste.org> |
| Download | mbox | patch |
| Permalink | /patch/11342/ |
| State | Rejected |
| Delegated to: | Matt Mackall |
| Headers | show |
Comments
On Mon, 2015-11-09 at 17:06 -0600, Martin von Zweigbergk wrote: > # HG changeset patch > # User Martin von Zweigbergk <martinvonz@google.com> > # Date 1447109202 28800 > # Mon Nov 09 14:46:42 2015 -0800 > # Node ID 6c4a3f70880b72538c475336cb0349f573a9aacf > # Parent 8b2fbe3f59b1b969878691cb472369ad0067f165 > status: add dirty() method to status type > > We do "s.modified or s.added or s.removed{ or s.deleted}" in quite a > few places, ..but we probably shouldn't, because it usually means we're forgetting small details like "is the branch name changed" or "are there two parents of the working directory" or "are the subrepos clean" or "do we have a topic"[1], etc.? And those questions actually can't be asked of the status object. So we should instead have a nice method on wctx that knows about all the things (and how to ignore some of them) so that we don't have people open-coding checks. And that's basically what we have already in wctx.dirty(), but we've apparently got lots of code that needs to be caught up. (First on that list is cmdutil.bailifchanged.) -- Mathematics is the supreme nostalgia of our time.
On Mon, Nov 9, 2015 at 3:31 PM Matt Mackall <mpm@selenic.com> wrote: > On Mon, 2015-11-09 at 17:06 -0600, Martin von Zweigbergk wrote: > > # HG changeset patch > > # User Martin von Zweigbergk <martinvonz@google.com> > > # Date 1447109202 28800 > > # Mon Nov 09 14:46:42 2015 -0800 > > # Node ID 6c4a3f70880b72538c475336cb0349f573a9aacf > > # Parent 8b2fbe3f59b1b969878691cb472369ad0067f165 > > status: add dirty() method to status type > > > > We do "s.modified or s.added or s.removed{ or s.deleted}" in quite a > > few places, > > ..but we probably shouldn't, because it usually means we're forgetting > small details like "is the branch name changed" or "are there two > parents of the working directory" or "are the subrepos clean" or "do we > have a topic"[1], etc.? And those questions actually can't be asked of > the status object. > > So we should instead have a nice method on wctx that knows about all > the things (and how to ignore some of them) so that we don't have > people open-coding checks. And that's basically what we have already in > wctx.dirty(), but we've apparently got lots of code that needs to be > caught up. > That makes sense, thanks for the explanation. > > (First on that list is cmdutil.bailifchanged.) > > -- > Mathematics is the supreme nostalgia of our time. > >
Patch
diff --git a/hgext/histedit.py b/hgext/histedit.py --- a/hgext/histedit.py +++ b/hgext/histedit.py @@ -971,11 +971,9 @@ actobj = actiontable[action].fromrule(state, currentnode) - s = repo.status() - if s.modified or s.added or s.removed or s.deleted: + if repo.status().dirty(missing=True): actobj.continuedirty() - s = repo.status() - if s.modified or s.added or s.removed or s.deleted: + if repo.status().dirty(missing=True): raise error.Abort(_("working copy still dirty")) parentctx, replacements = actobj.continueclean() diff --git a/hgext/keyword.py b/hgext/keyword.py --- a/hgext/keyword.py +++ b/hgext/keyword.py @@ -360,7 +360,7 @@ wlock = repo.wlock() try: status = _status(ui, repo, wctx, kwt, *pats, **opts) - if status.modified or status.added or status.removed or status.deleted: + if status.dirty(missing=True): raise error.Abort(_('outstanding uncommitted changes')) kwt.overwrite(wctx, status.clean, True, expand) finally: diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py --- a/hgext/largefiles/overrides.py +++ b/hgext/largefiles/overrides.py @@ -1044,7 +1044,7 @@ repo.lfstatus = True s = repo.status() repo.lfstatus = False - if s.modified or s.added or s.removed or s.deleted: + if s.dirty(missing=True): raise error.Abort(_('uncommitted changes')) def cmdutilforget(orig, ui, repo, match, prefix, explicitonly): diff --git a/hgext/shelve.py b/hgext/shelve.py --- a/hgext/shelve.py +++ b/hgext/shelve.py @@ -664,8 +664,7 @@ # to the original pctx. # Store pending changes in a commit - s = repo.status() - if s.modified or s.added or s.removed or s.deleted: + if repo.status().dirty(missing=True): ui.status(_("temporarily committing pending changes " "(restore with 'hg unshelve --abort')\n")) def commitfunc(ui, repo, message, match, opts): diff --git a/hgext/strip.py b/hgext/strip.py --- a/hgext/strip.py +++ b/hgext/strip.py @@ -36,7 +36,7 @@ cmdutil.checkunfinished(repo) s = repo.status() if not force: - if s.modified or s.added or s.removed or s.deleted: + if s.dirty(missing=True): _("local changes found") # i18n tool detection raise error.Abort(_("local changes found" + excsuffix)) if checksubstate(repo): diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py --- a/mercurial/cmdutil.py +++ b/mercurial/cmdutil.py @@ -306,8 +306,7 @@ def bailifchanged(repo, merge=True): if merge and repo.dirstate.p2() != nullid: raise error.Abort(_('outstanding uncommitted merge')) - modified, added, removed, deleted = repo.status()[:4] - if modified or added or removed or deleted: + if repo.status().dirty(missing=True): raise error.Abort(_('uncommitted changes')) ctx = repo[None] for s in sorted(ctx.substate): diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -6232,8 +6232,7 @@ elif (parents[0].closesbranch() and pnode in repo.branchheads(branch, closed=True)): t += _(' (head closed)') - elif not (status.modified or status.added or status.removed or renamed or - copied or subs): + elif not (status.dirty() or renamed or copied or subs): t += _(' (clean)') cleanworkdir = True elif pnode not in bheads: diff --git a/mercurial/context.py b/mercurial/context.py --- a/mercurial/context.py +++ b/mercurial/context.py @@ -1372,8 +1372,7 @@ # check current working dir return ((merge and self.p2()) or (branch and self.branch() != self.p1().branch()) or - self.modified() or self.added() or self.removed() or - (missing and self.deleted())) + self._status.dirty(missing=missing)) def add(self, list, prefix=""): join = lambda f: os.path.join(prefix, f) diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py --- a/mercurial/scmutil.py +++ b/mercurial/scmutil.py @@ -69,6 +69,10 @@ '''files that have not been modified''' return self[6] + def dirty(self, missing=False): + return bool(self.modified or self.added or self.removed or + (missing and self.deleted)) + def __repr__(self, *args, **kwargs): return (('<status modified=%r, added=%r, removed=%r, deleted=%r, ' 'unknown=%r, ignored=%r, clean=%r>') % self)
# HG changeset patch # User Martin von Zweigbergk <martinvonz@google.com> # Date 1447109202 28800 # Mon Nov 09 14:46:42 2015 -0800 # Node ID 6c4a3f70880b72538c475336cb0349f573a9aacf # Parent 8b2fbe3f59b1b969878691cb472369ad0067f165 status: add dirty() method to status type We do "s.modified or s.added or s.removed{ or s.deleted}" in quite a few places, so let's introduce a method on the status type for this purpose.