Again, commands.bookmark is getting too large. checkconflict already has
a lot of state and putting it in the bmstore makes more sense than
having it as a closure. This also allows extensions a place to override
this behavior.
While we're here, add a documentation string because, well, we should be
documenting more of our methods.
commands.bookmark has grown quite large with two closures already. Let's
split this up (and in the process allow extensions to override the
default behavior).
Unlike a mapfile whose template is looked up by spec -> mapfile -> topic,
[templates] section is global. We use :sub-section syntax to define parts
per template.
As commented, this should be used with docheader and docfooter, not with
header nor footer. That's one reason why no props are passed to templater
when rendering a separator. (See map-cmdline.changelog to understand what
the "header" is.)
Since this postfix hack exists only for backward compatibility, we don't need
it for new [templates] section. This isn't a BC as templates defined in
[templates] section weren't loaded until recently.
changegroup.apply() currently creates a transation if there isn't
already one. Having the callers of that method pass in an existing
transaction seems a little cleaner. To do that, we need to make sure
all callers have a transaction. Since the transaction name is used as
a hook argument (HG_TXNNAME), we need to match the name from
changegroup.apply().
Several benefits:
* Gets close the comment describing it
* Splits off unrelated comment about "backup" argument
* Error checking is customarily done early
* If we added an early return to the method, it would still
consistently fail if there was an existing transaction (so
we would find and fix that case quickly)
One test needs updating with for this change, because we no longer
create the backup bundle before we fail. I don't see much reason to
create that backup bundle. If some command was adding content and then
trying to strip it as well within the transaction, we would have a
backup for the user, but the risk of that not being discovered in
development seems very small.
I have checked that all callers have already taken the lock (and if
they hadn't, we should have seen tests fail thanks to the 'transaction
requires locking' devel warning in localrepo.transaction()).
I'm not sure if this is better. If we're planning to add a template keyword
that returns obsoleted nodes unavailable in the repo (i.e. they have no valid
revision numbers), we might want to use the current "node"-only format
everywhere.
context.py seems not a good place to host these functions.
% wc -l mercurial/context.py mercurial/dagop.py
2306 mercurial/context.py
424 mercurial/dagop.py
2730 total
This module hosts the following functions. They are somewhat similar (e.g.
scanning revisions using heap queue or stack) and seem non-trivial in
algorithmic point of view.
- _revancestors()
- _revdescendants()
- reachableroots()
- _toposort()
I was thinking of adding revset._fileancestors() generator for better follow()
implementation, but it would be called from context.py as well. So I decided
to create new module.
Naming is hard. I couldn't come up with any better module name, so it's called
"dag operation" now. I rejected the following candidates:
- ancestor.py - existing, revlog-level DAG algorithm
- ancestorset.py - doesn't always return a set
- dagalgorithm.py - hard to type
- dagutil.py - existing
- revancestor.py - I want to add fileancestors()
% wc -l mercurial/dagop.py mercurial/revset.py
339 mercurial/dagop.py
2020 mercurial/revset.py
2359 total
We move the feature to a proper configuration and document it. The config goes
in the 'server' section because it feels like something the server owner would
want to decide. We pick and open field because it seems likely that other
checking levels will emerge in the future. (eg: server like the mozilla-try
server will likely wants a "none" value)
The option name contains 'push' since this affects 'push' only. The option value
'check-related' is preferred over one explicitly containing 'allow' or 'deny'
because the client still have a strong decision power here. Here, the server is
just advising the client on the check mode to use.
Add a 'predecessors' template that returns the list of all closest known
predecessors for a changectx. The elements of the list are row changectx node id
formatted by default as short nodes.
The "closest predecessors" are the first locally known revisions encountered
while, walking predecessors markers. For example:
1) If a (A, (B)) markers exists and both A and B are locally known A is a
closest predecessors of B.
2) If a (A, (B)) and (B, (C)) markers exists and only A and C are known
locally, A will be the closest precursors of C.
This logic respect repository filtering. So hidden revision will be skipped by
this logic unless --hidden is specified. Since we only display the visible
predecessors, this template will not display anything in most case. It makes a
good candidate for inclusion in the default log output.
I added a new test-file for testing the precursors in various scenarios. This
test file will also be used for the successors template.
A new "obsutil" module has been added to start gathering utility function
outside of the large obsolete.py module.
Now spec.ref should be '' if spec.tmpl is specified. Since spec.ref is the
option to select the initial template to be rendered, it doesn't make sense
to store the given literal template as spec.ref.
Since a map file has another level to select a template (spec -> mapfile
-> topic), this isn't exactly the same as how a map file works. But I believe
most users would expect the new behavior.
A literal template is stored as an unnamed template so that it will never
conflict with the templates defined in [templates] section.
This provides a simpler API for callers which don't need full templating
stack. Instead of storing the given template as the name specified by topic,
use '' as the default template to be rendered.
We've been talking for years about a one-stop config knob to opt in to
better behavior. There have been a lot of ideas thrown around, but
they all seem to be too complicated to get anyone to actually do the
work.. As such, this patch is the stupidest thing that can possibly
work in the name of getting a good feature to users.
Right now it's just three config settings that I think are generally
uncontroversial, but I expect to add more soon. That will likely
include adding new config knobs for the express purpose of adding them
to tweakdefaults.