This instance will be upgraded to Heptapod 0.21.0rc2 on 2021-04-16 at 11:00 UTC+2 (a few minutes of down time)

Commit 26e8cdf5 authored by Manuel Jacob's avatar Manuel Jacob

repomodel, revpanel: improve upon "Not a head revision!" warning

The original motivation was to fix the calculation of what leads to "Not a head
revision!" warnings in several places, which were all different and flawed in
different ways.

When thinking about how to fix them, deeper problems surfaced. The previous code
showed "Not a head revision!" on the working directory line in the log, which
is actually incorrect, as a working directory, when being committed, always
becomes a head, as a freshly created revision trivally can’t have any children.
Obviously, the label "Not a head revision!" was meant to refer to the working
directory parent instead of the working directory. In addition to being at the
wrong place, it also broke down for merges, where there are two working
directory parents. E.g. when merging the heads of two differently named
branches, it incorrectly showed the warning.

Because the working directory pseudo-revision is about what happens when
committing, a "Creates new head!" warning makes more sense. The logic for
determining whether to show this warning is based on that from the `hg commit`
command to determine whether to show "created new head".

In the commit widget, it could be argued that the "Not a head revision!" warning
was not at the wrong place, as it could refer to the parent(s) above the
warning instead of to the working directory. Still, it didn’t quite make sense
when merging two parents, where it’s not clear which parent it refers to. Also,
the previous calculation used a definition of "head" which doesn’t match
Mercurial’s.

With this patch, it’s clarified that the commit widget is about the working
directory. Therefore the warning and calculation is changed to be the same as
in the log widget.

These changes also make sense from a usability perspective. The user doesn’t
need to be warned that they checked out a revision that is not a head. Instead,
the user should be warned against creating a new head, as that is often
undesirable.

Similarly, instead of showing that the working directory parent is closed, we
show if a closed branch head is reopened. Again, we use the same logic as the
`hg commit` command.

When amending a revision that is not a topological head, there is now a note
that it will create orphans. In practice, I found this to be a much more
valueable information than whether it creates an additional head. See the added
comment for detailed reasoning.

A few other places had to be changed to be consistent. One notable exception is
the backout dialog, where the warning stayed "Not a head, backout will create a
new head!". It already had the advantages of the new warning and it was clear
which revision "not a head" refers to. The calculation still had to be fixed.

For my taste, the patch is a bit too big. However splitting in into smaller
parts would have risked that intermediate states would have even more
inconsistencies.
parent 0fae0d10114e
......@@ -249,8 +249,7 @@ class SummaryInfo(object):
else:
return None
elif item == 'ishead':
childbranches = [cctx.branch() for cctx in ctx.children()]
return ctx.branch() not in childbranches
return ctx.node() in ctx.repo().branchheads(ctx.branch())
elif item == 'mqoriginalparent':
target = ctx.thgmqoriginalparent()
if not target:
......
......@@ -183,11 +183,11 @@ class SummaryPage(BasePage):
self.setLayout(QVBoxLayout())
repo = self.repo
contents = ('ishead',) + csinfo.PANEL_DEFAULT
contents = ('createsnewhead',) + csinfo.PANEL_DEFAULT
style = csinfo.panelstyle(contents=contents)
def markup_func(widget, item, value):
if item == 'ishead' and value is False:
text = _('Not a head revision!')
if item == 'createsnewhead' and value is True:
text = _('Creates new head!')
return qtlib.markup(text, fg='red', weight='bold')
raise csinfo.UnknownItem(item)
custom = csinfo.custom(markup=markup_func)
......
......@@ -794,9 +794,8 @@ class HgRepoListModel(QAbstractTableModel):
self._branchheads[branch] = branchheads
if ctx.rev() is None:
for pctx in ctx.parents():
if branchheads and pctx.node() not in branchheads:
labels.append((_('Not a head revision!'), 'log.warning'))
if hglib.createsnewhead(ctx, branchheads):
labels.append((_('Creates new head!'), 'log.warning'))
return labels
if self._show_branch_head_label:
......
......@@ -91,15 +91,20 @@ def data_func(widget, item, ctx):
return revline_data(tctx)
except (error.LookupError, error.RepoLookupError, error.RepoError):
return ts
elif item == 'ishead':
if ctx.rev() is None:
ctx = ctx.p1()
childbranches = [cctx.branch() for cctx in ctx.children()]
return ctx.branch() not in childbranches
elif item == 'isclose':
if ctx.rev() is None:
ctx = ctx.p1()
return ctx.extra().get(b'close') is not None
elif item == 'createsnewhead':
# Strictly speaking, amend can create a new head in the case when
# amending a revision which is not a topological head, as then the
# original amended revision is kept alive by its orphan ancestors.
# However, as the original amended revision along with its orphan
# ancestors are eventually going to be evolved, we should not warn the
# user. Instead, we should show that the amend will create orphans.
return not widget.custom['isAmend'] and hglib.createsnewhead(ctx)
elif item == 'createsorphans':
return widget.custom['isAmend'] and any(p.children()
for p in ctx.parents())
elif item == 'reopensbranchhead':
return any(p.closesbranch() and p.branch() == ctx.branch()
for p in ctx.parents())
elif item == 'predecessors':
ctxlist = obsoleteutil.first_known_predecessors(ctx)
return format_ctxlist(ctxlist)
......@@ -196,18 +201,19 @@ def nomarkup(widget, item, value):
revnum = qtlib.markup(revnum)
return '%s - %s' % (revnum, summary)
csets = []
if item == 'ishead':
if value is False:
if widget.custom['isAmend']:
text = _('Not a head revision.')
return qtlib.markup(text, weight='bold')
else:
text = _('Not a head revision!')
return qtlib.markup(text, fg='red', weight='bold')
if item == 'createsnewhead':
if value is True:
text = _('Creates new head!')
return qtlib.markup(text, fg='red', weight='bold')
raise csinfo.UnknownItem(item)
elif item == 'createsorphans':
if value is True:
text = _('Creates orphans.')
return qtlib.markup(text, weight='bold')
raise csinfo.UnknownItem(item)
elif item == 'isclose':
elif item == 'reopensbranchhead':
if value is True:
text = _('Head is closed!')
text = _('Reopens closed branch head!')
return qtlib.markup(text, fg='red', weight='bold')
raise csinfo.UnknownItem(item)
for cset in value:
......@@ -220,6 +226,7 @@ def nomarkup(widget, item, value):
def WDirInfoWidget(repo):
'creates a wdir info widget and returns it'
custom = csinfo.custom(data=data_func, label=label_func, markup=nomarkup)
style = csinfo.panelstyle(contents=('parents', 'ishead', 'isclose'),
selectable=True)
style = csinfo.panelstyle(contents=('parents', 'createsnewhead',
'createsorphans', 'reopensbranchhead'),
selectable=True)
return csinfo.create(repo, style=style, custom=custom)
......@@ -1336,3 +1336,12 @@ def parsecmdline(cmdline, cwd):
args.extend(p[len(cwd) + 1:] for p in expanded)
else:
args.append(e)
def createsnewhead(ctx, branchheads=None):
branch = ctx.branch()
if branchheads is None:
branchheads = set(ctx.repo().branchheads(branch))
return branchheads and not any(
p.node() in branchheads and p.branch() == branch for p in ctx.parents()
)
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment