pullrequests: also limit to stop displaying merged changes

If the number of available changes are ok but merged changesets are not, then
just show the available changesets.

pullrequests: introduce limit to stop displaying additional changes

We've noticed some scalability issues when many descendants exist for the
changesets in a pull request.

To resolve this issue, we instead do not display any additional changes at all
if the amount of additional changes is larger than a configured limit.

(This occurred because we were merging a lot of heads in the repository.)

Throwing away some of the changesets would also keep the total amount more
manageable, but this would result in an (for the user) unpredictable subset of
changesets being shown.

This could be extended further with "too long to be shown - click here to
show", but that was quite a bit of additional work and did not cover our use
case of not allowing the display at all in case of too many additional changes.

controllers: move controllers base class from lib/base to controllers

TG quickstart put it in lib/base.py , but it fits better on the controllers
layer as a base there.

The contributing docs were a bit ahead of time ... but with a typo.

lib: drop is_hg and is_git

It is just as simple to be explicit.

lib: move webhelpers2 and friends to webutils

Gives less of the unfortunate use of helpers - especially in low level libs.

diffs: remove unused argument enable_comments and class no-comment

enable_comments was only used to set/not-set the 'no-comment' CSS class.
While this class was emitted, no CSS rule nor any JavaScript logic was
actually using it. Last real usage of that class was removed with commit
e87baa8f1c5bd2488aefc23b95c0db3a04bc8431.

Cleanup the code by not emitting 'no-comment' and remove the
'enable_comments' flag.

routing: separate url handling from routing - move it to webutils

This is a helper method relying on the thread local tg.request. We didn't have
a good place to put it. Now we do.

This (re)moves unfortunate dependencies to the routing module (which almost is
a controller).

tests: add coverage of Git pullrequests

Git is so different than Mercurial that it is easier to use separate tests.
Some of the tests that are relevant for Mercurial doesn't apply to the Git
support.

Also fix crash an odd corner case of creating PRs from a repo that has no
branches at all.

diff: drop per file ignore-whitespace and context - it didn't work and had conceptual issue (Issue #344)

Diffs are currently generated at the low level as one big diff between two vcs
resisions, provided global values for diff context size and flag for ignoring
whitespace. All files use the same flags. There is no way to actually compute
the full diff using these use per file flags, and no simple and efficient way
to add it.

The best option is thus to drop the failed attempt at making it per file, and
just rely on the simple global flags in the URL.

The links for changing whitespace and context is sometimes shown for the whole
"page", and sometimes next to the diff for one file. For now, keep showing the
link in these places, but make sure it navigates back to the FID of the section
where the link was clicked.

The implementation is completely rewritten and moved to a more appropriate
location in helpers.

With a more clean implementation, we also consistently use the simple getters
to extract values from the URL.

py3: trivial renaming of .iteritems() to .items()

A bit like "2to3 -f dict", but we don't want list().

cleanup: minor correctness fixes

"Trivial" potential problems spotted with pytype.

vcs: drop the superfluous and leaky hgcompat "layer"

Explicit is better. And gives less pyflakes noise.

py3: add b'' annotations in some places where they will be needed later

Mostly entirely trivial adding of b prefix that is a ignored for py2 ... and
also a bit of related trivial reformatting/refactorings.

lib: handle both HTML, unsafe strings, and exceptions passed to helpers.flash()

Before, h.flash would trust any input to contain html ... and callers would
convert exceptions to string, often with a simple str() or unicode() ... which
really didn't deserve to be trusted.

Instead, only trust messages that have a __html__ and escape anything else ...
but also apply str/unicode on the parameter so the caller doesn't have to but
*can* pass an exception directly.

vcs: always return bytes from node.content

We will rather have the unicode conversions explicit.

Note: Py3 bytes doesn't have .startswith - replace that with a regexp.

hg: bump minimum version to 5.1

We will soon move to Python 3 which only will support 5.1 or later.

Remove old hacks and tech debt.

Also avoids future warning:
DeprecationWarning: inspect.getargspec() is deprecated since Python 3.0, use inspect.signature() or inspect.getfullargspec()

cleanup: minor refactorings and simplification of dict usage

Makes it more py3 compatible.

page: pass url query params to Page instead of passing request.GET.mixed() to .pager

Standardize on one transparent way to use Pager:

Avoid passing random parameters to .pager() ... or any parameters at all.

hg: fix pull requests between repositories by using the makeunionrepository factory with Mercurial 4.8

Follow-up to 9ca238e56396 that missed one case ...

cleanup: remove unnecessary (and potentially problematic) use of 'literal'

webhelpers.html.literal (kallithea.lib.helpers.literal) is only needed when
the passed string may contain HTML that needs to be interpreted literally.
It is unnecessary for plain strings.

Incorrect usage of literal can lead to XSS issues, via a malicious user
controlling data which will be rendered in other users' browsers. The data
could either be stored previously in the system or be part of a forged URL
the victim clicks on.

For example, when a user browses to a forged URL where a repository
changeset or branch name contains a javascript snippet, the snippet
was executed when printed on the page using 'literal'.

Remaining uses of 'literal' have been reviewed with no apparent problems
found.

Reported by Bob Hogg <wombat@rwhogg.site> (thanks!).

cleanup: remove unnecessary (and potentially problematic) use of 'literal'

webhelpers.html.literal (kallithea.lib.helpers.literal) is only needed when
the passed string may contain HTML that needs to be interpreted literally.
It is unnecessary for plain strings.

Incorrect usage of literal can lead to XSS issues, via a malicious user
controlling data which will be rendered in other users' browsers. The data
could either be stored previously in the system or be part of a forged URL
the victim clicks on.

For example, when a user browses to a forged URL where a repository
changeset or branch name contains a javascript snippet, the snippet
was executed when printed on the page using 'literal'.

Remaining uses of 'literal' have been reviewed with no apparent problems
found.

Reported by Bob Hogg <wombat@rwhogg.site> (thanks!).

controllers: remove pr_comment flag in delete_cs_pr_comment

A separate comment is not really needed as we can check co.pull_request.

Suggested by Mads Killerich.

controllers: forward pullrequests.comment to changeset

Similar to the way delete_comment is handled.

controllers: align pullrequests.comment with changeset.comment

This commit purely serves to highlight the differences.
The subsequent commit will remove the duplication.

controllers: pullrequests: comments are always using AJAX

This is preparation to align commenting on changeset and pullrequests.

controllers: pullrequests: rename _get_is_allowed_change_status

Rename the oddly named method '_get_is_allowed_change_status' to
'_is_allowed_to_change_status'. Not only does this read more easily, but
also it is more in line with the variable to which the result is assigned to
'allowed_to_change_status'.

controllers: forward pullrequests.delete_comment to changeset

Remove duplication between pullrequests and changeset.
We move the code outside ChangesetController to make it callable from
PullrequestsController.

Note:
- instead of keeping the method pullrequests.delete_comment itself and
letting it forward to changeset.delete_comment, an alternative solution
would have been to change the routing table directly. However, the chosen
solution makes it more explicit which operations are supported on each
controller.

controllers: align pullrequests.delete_comment with changeset.delete_comment

This commit purely serves to highlight the differences.
The subsequent commit will remove the duplication.

pullrequests: don't show empty "additional changesets" (issue #280)

When opening a pullrequest on a revision range including the tipmost
revision, and then pushing a new revision on top of that, the PR page shows:
This pullrequest can be updated with changes on ...:
and then nothing.

This is because the available revisions looped over are in 'avail_cs' but
the guard around the loop is checking on 'avail_revs'. The former, while
derived from avail_revs, can become empty under circumstances like this one.

Fix the problem by changing the guard checking avail_cs rather than
avail_revs, and making sure the printed message is aligned to 'No additional
changes found'.

vcs: introduce 'branches' attribute on changesets, making it possible for Git to show multiple branches for a changeset

Mercurial changesets will always have have exactly one branch (which might be
"default"). The VCS data model was the same.

Git allows for a changeset to have 0 or more branches ... and possibly one of
them as active. The right data model is thus to have an enumerable of branches.

We thus add a 'branches' attribute and use it where applicable.

The existing 'branch' attribute used some heuristics to decide which branch use
as "the" branch ... and in some places code (and tests) rely on that. We thus
keep that old method, knowing that some of its uses probably should move to
'branches'.

The code for retrieving Git branches is based on work by Dominik Ruf.

auth: restore anonymous repository access

Dominik Ruf found that aa25ef34ebab introduced a regression in anonymous access
to repositories ... if that is enabled.

The refactoring was too strict when it missed that not all repo permission
checks require a logged in user. Read access can be granted to the default user
... but not write or admin.

Instead of the commands used in aa25ef34ebab, the following commands are used
to consistently also allow the default user in all decorators where we only need
repo read access:

# Introduce explicit allow_default_user=True - that was the default before aa25ef34ebab
sed -i 's/@LoginRequired()/@LoginRequired(allow_default_user=True)/g' `hg mani`
sed -i 's/@LoginRequired(\(..*\))/@LoginRequired(\1, allow_default_user=True)/g' `hg mani`
# The primary case: Replace @NotAnonymous with removal of allow_default_user=True
perl -0pi -e 's/\@LoginRequired\((?:(.*), )?allow_default_user=True\)\n\s*\@NotAnonymous\(\)/\@LoginRequired(\1)/g' `hg mani`
# If there is a global permission check, no anonymous is ever allowed
perl -0pi -e 's/\@LoginRequired\(allow_default_user=True\)(\n\s*\@HasPermission)/\@LoginRequired()\1/g' `hg mani`
# Repo access for write or admin also assume no default user
perl -0pi -e 's/\@LoginRequired\(allow_default_user=True\)(\n\s*\@HasRepoPermissionLevelDecorator\('"'(write|admin)'"'\))/\@LoginRequired()\1/g' `hg mani`

auth: refactor to introduce @LoginRequired(allow_default_user=True) and deprecate @NotAnonymous()

It was error prone that @LoginRequired defaulted to allow anonymous users (if
'default' user is enabled). See also 245b4e3abf39.

Refactor code to make it more explicit and safe by default: Deprecate
@NotAnonymous by making it the default of @LoginRequired. That will make it
safe by default.

To preserve same functionality, set allow_default_user=True in all the cases
where @LoginRequired was *not* followed by @NotAnonymous or other permission
checks - that was done with some script hacks:
sed -i 's/@LoginRequired(\(..*\))/@LoginRequired(\1, allow_default_user=True)/g' `hg mani`
sed -i 's/@LoginRequired()/@LoginRequired(allow_default_user=True)/g' `hg mani`
perl -0pi -e 's/\@LoginRequired\(allow_default_user=True\)\n\s*\@NotAnonymous\(\)/\@LoginRequired()/g' `hg mani`
perl -0pi -e 's/\@LoginRequired\(allow_default_user=True\)(\n\s*\@Has(Repo)?Permission)/\@LoginRequired()\1/g' `hg mani`

It has been reviewed that all uses of allow_default_user=True are in places
where the there indeed wasn't any checking for default user before. These may
or may not be correct, but now they are explicit and can be spotted and fixed.

The few remaining uses of @NotAnonymous should probably be removed somehow.

users: remove code that is unused after most autocomplete has been switched to ajax

@mention support still require _USERS_AC_DATA as a global variable.

diffs: wrap vcs repo get_diff

Refactor to get a single place for diff error handling outside the vcs lib.

diffs: move as_html and _safe_id from method to a pure function - avoid calling the method as function

as_html was sometimes used in a way where we actually don't want to use the
whole DiffProcessor - we just created a dummy instance and passed custom input
as parameter to the instance method.

Instead, make it the function we apparently want.

Make it clear that as_html not just returns a "diff" but that it is a html diff.

diffs: drop the DiffLimitExceeded container - just make it a flag available as property

Keep it simple.

diffs: inline prepare() into __init__ and make the result available as .parsed

Make it more clear what the DiffProcessor is: Something that works on a raw
diff as input, mainly compute when initialized, and returns an object where the
result is available in different ways.

diffs: cleanup of variable naming around cut_off_limit

A brief summary of this area:

The base controller sets self.cut_off_limit from config and is used for diffs,
unless controllers are given a fulldiff query parameter. In a few cases, these
are passed to templates as c.cut_off_limit or c.fulldiff . Also, if the diff
function returns a LimitedDiffContainer, c.limited_diff is set so the UI can
report the data set is partial.

codingstyle: trivial whitespace fixes

Reported by flake8.

py3: replace list comprehension with for-loop due to scope

The scope of the list comprehension variable 'x' will be limited to the list
comprehension in python3. Thus switching to a full loop (without this
scope restriction) in preparation for python3.

controllers: consistently use c.cs_comments and cs_statuses

c.comments and c.statuses were also used for lists of comments and statuses. To
get more consistency and avoid confusion and conflicts, use different for names
for mappings from changeset hashes.

pullrequests: fix broken "new PR iteration" handling of ancestor changes

An earlier refactor (5d60c9a391cd) flubbed this code and accidentally
assumed the most recent common ancestor would not change when iterating,
which is of course only the case when there are no merges from 'other' to
'org' among the new revisions.

This was not only not caught during manual testing (nor review), but
neither did the test suite test this. That has now also been rectified.

tg: minimize future diff by some mocking and replacing some pylons imports with tg

No actual tg dependency yet, just a temporary hack faking tg as an alias for
pylons.

Based on work by Alessandro Molina.

pullrequests: rename "as_form" to something more descriptive

This parameter is (only) used for showing the PR contents preview on the
"New Pull Request" page.

pullrequests: introduce "action objects" for PR creation

Inspired by the command design pattern, this attempts the following:

* move policy and business logic from controllers into the model,
* move validation, authorization and execution logic closer together in
the code,
* establish a reusable pattern for modelling higher-level concepts
(like "create a new PR iteration"),
* make error handling more well-defined, and independent of the
controller layer, and
* provide clear separation between, one one hand, the validation and
authorization of a request, and on the other, the actual execution.

templates: properly escape inline JavaScript values

TLDR: Kallithea has issues with escaping values for use in inline JS.
Despite judicious poking of the code, no actual security vulnerabilities
have been found, just lots of corner-case bugs. This patch fixes those,
and hardens the code against actual security issues.

The long version:

To embed a Python value (typically a 'unicode' plain-text value) in a
larger file, it must be escaped in a context specific manner. Example:

>>> s = u'<script>alert("It\'s a trap!");</script>'

1) Escaped for insertion into HTML element context

>>> print cgi.escape(s)
&lt;script&gt;alert("It's a trap!");&lt;/script&gt;

2) Escaped for insertion into HTML element or attribute context

>>> print h.escape(s)
&lt;script&gt;alert(&#34;It&#39;s a trap!&#34;);&lt;/script&gt;

This is the default Mako escaping, as usually used by Kallithea.

3) Encoded as JSON

>>> print json.dumps(s)
"<script>alert(\"It's a trap!\");</script>"

4) Escaped for insertion into a JavaScript file

>>> print '(' + json.dumps(s) + ')'
("<script>alert(\"It's a trap!\");</script>")

The parentheses are not actually required for strings, but may be needed
to avoid syntax errors if the value is a number or dict (object).

5) Escaped for insertion into a HTML inline <script> element

>>> print h.js(s)
("\x3cscript\x3ealert(\"It's a trap!\");\x3c/script\x3e")

Here, we need to combine JS and HTML escaping, further complicated by
the fact that "<script>" tag contents can either be parsed in XHTML mode
(in which case '<', '>' and '&' must additionally be XML escaped) or
HTML mode (in which case '</script>' must be escaped, but not using HTML
escaping, which is not available in HTML "<script>" tags). Therefore,
the XML special characters (which can only occur in string literals) are
escaped using JavaScript string literal escape sequences.

(This, incidentally, is why modern web security best practices ban all
use of inline JavaScript...)

Unsurprisingly, Kallithea does not do (5) correctly. In most cases,
Kallithea might slap a pair of single quotes around the HTML escaped
Python value. A typical benign example:

$('#child_link').html('${_('No revisions')}');

This works in English, but if a localized version of the string contains
an apostrophe, the result will be broken JavaScript. In the more severe
cases, where the text is user controllable, it leaves the door open to
injections. In this example, the script inserts the string as HTML, so
Mako's implicit HTML escaping makes sense; but in many other cases, HTML
escaping is actually an error, because the value is not used by the
script in an HTML context.

The good news is that the HTML escaping thwarts attempts at XSS, since
it's impossible to inject syntactically valid JavaScript of any useful
complexity. It does allow JavaScript errors and gibberish to appear on
the page, though.

In these cases, the escaping has been fixed to use either the new 'h.js'
helper, which does JavaScript escaping (but not HTML escaping), OR the
new 'h.jshtml' helper (which does both), in those cases where it was
unclear if the value might be used (by the script) in an HTML context.
Some of these can probably be "relaxed" from h.jshtml to h.js later, but
for now, using h.jshtml fixes escaping and doesn't introduce new errors.

In a few places, Kallithea JSON encodes values in the controller, then
inserts the JSON (without any further escaping) into <script> tags. This
is also wrong, and carries actual risk of XSS vulnerabilities. However,
in all cases, security vulnerabilities were narrowly avoided due to other
filtering in Kallithea. (E.g. many special characters are banned from
appearing in usernames.) In these cases, the escaping has been fixed
and moved to the template, making it immediately visible that proper
escaping has been performed.

Mini-FAQ (frequently anticipated questions):

Q: Why do everything in one big, hard to review patch?
Q: Why add escaping in specific case FOO, it doesn't seem needed?

Because the goal here is to have "escape everywhere" as the default
policy, rather than identifying individual bugs and fixing them one
by one by adding escaping where needed. As such, this patch surely
introduces a lot of needless escaping. This is no different from
how Mako/Pylons HTML escape everything by default, even when not
needed: it's errs on the side of needless work, to prevent erring
on the side of skipping required (and security critical) work.

As for reviewability, the most important thing to notice is not where
escaping has been introduced, but any places where it might have been
missed (or where h.jshtml is needed, but h.js is used).

Q: The added escaping is kinda verbose/ugly.

That is not a question, but yes, I agree. Hopefully it'll encourage us
to move away from inline JavaScript altogether. That's a significantly
larger job, though; with luck this patch will keep us safe and secure
until such a time as we can implement the real fix.

Q: Why not use Mako filter syntax ("${val|h.js}")?

Because of long-standing Mako bug #140, preventing use of 'h' in
filters.

Q: Why not work around bug #140, or even use straight "${val|js}"?

Because Mako still applies the default h.escape filter before the
explicitly specified filters.

Q: Where do we go from here?

Longer term, we should stop doing variable expansions in script blocks,
and instead pass data to JS via e.g. data attributes, or asynchronously
using AJAX calls. Once we've done that, we can remove inline JavaScript
altogether in favor of separate script files, and set a strict Content
Security Policy explicitly blocking inline scripting, and thus also the
most common kind of cross-site scripting attack.

pullrequests: pass around reviewer User objects, not IDs

This moves reviewer user validation into the controller and eliminates
the UserInvalidException.

cleanup: remove SQLAlchemy session argument to action_logger

There's always a global SQLAlchemy session associated with the current
thread; using another session for a single function call does not make
any sense (as sessions cannot be mixed), unless the code works carefully
to ensure the two sessions (and all objects loaded from them) are kept
completely separate. Suffice to say that Kallithea does no such thing,
thus there's no need to pretend to support multiple concurrent sessions.

auth: simplify repository permission checks

In practice, Kallithea has the 'repository.admin' permission imply the
'repository.write' permission, which again implies 'repository.read'.

This codifies/enforces this practice by replacing HasRepoPermissionAny
"perm function" with the new HasRepositoryLevel function, reducing the
risk of errors and saving quite a lot of typing.

pullrequests: refactor default title code

This refactoring simply emphasizes that the origin reference used to
generate the default title is not always the same as the one actually
used to create the pull request; for PRs not from a branch head, the
branch name is stored so it can be used for creating new iterations.

controllers: avoid setting request state in controller instances - set it in the thread global request variable

In TurboGears, controllers are singletons and we should avoid using instance
variables for any volatile data. Instead, use the "global thread local" request
context.

With everything in request, some use of c is dropped.

Note: kallithea/controllers/api/__init__.py still use instance variables that
will cause problems with TurboGears.

lib: move jsonify from utils to base

Suggested by Mads Kiilerich.

The jsonify method is the only thing in utils that directly uses pylons.
Move it to base where it fits better and we can use existing global imports.

pullrequests: fix 'upgrade' from revision to branch when creating PR

This will do that the default title for the PR created from a non-head revision
will contain a short hash instead a long.

org_ref_type was set to 'branch' while org_ref_name remained the full hash.
h.short_ref would thus not truncate. Instead, we keep the org_ref_type as 'rev'
while storing the branch name in the database.

pullrequests: refactor PullRequestModel().create

The database lookups of the user and repositories are moved outside,
into the callers, which have already looked up the relevant objects,
thus saving two database lookups when creating a PR.

db: it should be "PullRequestReviewer" (singular)

sed -i 's/\bPullRequestReviewers\b/PullRequestReviewer/g' $(hg files)

Just as it's "User", "PullRequest", etc. It's not a collection.

helpers: move Page/RepoPage to a separate file page.py

The Page and RepoPage classes are not used from templates and thus do not
belong in helpers.
They could have been put in utils.py, but because they are completely
standalone we can easily split them to a separate file.

cleanup: use obj.foo_id instead of obj.foo.foo_id

Don't use constructs like obj.user.user_id when obj.user_id works
equally well (and potentially saves a database load).

model: change ChangesetComment 'user' to 'author'

Rename the 'user_id' field to 'author_id' and replace other references
to the comment 'user' throughout the model. The database column name
'user_id' remain unchanged for now; a later Alembic script can fix the
name of these and other columns to match their Python name.

db: PullRequest/Repository/RepoGroup/UserGroup: change 'user' to 'owner'

Rename the 'user' and 'user_id' fields on the four classes to something
more informative. The database column names remain unchanged for now;
a later Alembic script can fix the name of these and other columns to
match their Python name.

This might break rcextensions, though, and external scripts that use
the HTML form interface.

db: add some PullRequest.query() shortcuts

This makes database query code more explicit and increases readability.

E.g. the function name get_pullrequest_cnt_for_user was bad, because the
concept of "pullrequest for user" is incredibly vague, and could refer
to any kind of association between PRs and users. (Quiz time! Does it
mean that the user is the PR owner, that the user is reviewing, or that
the user has commented on the PR and thus is receiving notifications?)

A descriptive name could be "get_open_pull_request_count_for_reviewer",
because the function is indeed only concerned with reviewers and only
with open pull requests. But at this point, we might as well say
PullRequest.query(reviewer_id=user, include_closed=False).count()
which is only slightly longer, and doesn't require us to write dozens
of little wrapper functions (including, any moment now, a separate
function for listing the PRs instead of counting them).

Note that we're not actually going down an abstraction level by doing
this. We're still operating on the concepts of "pull request", "open"
and "reviewer", and are not leaking database implementation details.

The query() shortcuts are designed so they default to not altering
the query. Any processing requires explicit opt-in by the caller.

controllers: remove redundant Repository database lookups

BaseRepoController already provides the repository object in c.db_repo
(may be None) and BaseController already provides the repository name
in c.repo_name (as given in the URL).

(Arguably, that's a bad design, and we should revisit that decision in
the future. For now, the code just performs slightly better.)

Turbogears2 migration: replace pylons.url by kallithea.config.routing.url

In preparation for the migration to Turbogears2, introduce a
kallithea.config.routing.url to replace pylons.url. The implementation is
basically the same: wrap around routes.url().

This change involves:
- a number of import statement changes
- fixing some tests in test_libs.py; to avoid duplication, the different
implementations of fake_url were grouped in one place.

This change was first proposed by Alessandro Molina in his initial
port. Following changes were made afterwards:
- move UrlGenerator from kallithea.lib.utils to kallithea.config.routing
- add documentation to UrlGenerator
- kallithea/lib/auth.py used url_for instead of url, for no apparent reason
so this was changed.
- fix libs tests
- rebase onto Pylons-based Kallithea first

diff: correct handling of links to old filename in renames

There were links to the file at the parent revision ... but if the file had
been renamed, it used the wrong name.

diffs: fold diff_block_simple (used by PR and compare) into diff_block

Change to using the same datamodel and enjoy the reduced amount of code
duplication.

diff: minor cleanups

More consistency and preparing for later changes.

pullrequests: when creating, don't convert selected revision to tag name

It kind of makes sense that if a branch head or bookmarked revision is chosen,
then pretend the symbolic name was chosen. The name can move and will give the
PR a future. That is currently done both when populating the select boxes and
when actually creating the new PR. There, a selected revision is also
"upgraded" to its branch.

Tags are however mostly static. Fair enough if a user wants to create a PR from
a tag, but if a revision is selected, it shouldn't be side tracked to the tag name.

pullrequests: when creating PRs from/to revisions, shorten the visible name

Having the long name in the UI was annoying ... and we still have the full hash
elsewhere in the 'ref'.

pullrequests: drop 'Closing.' comment when closing a PR - the semantic change is shown clearly everywhere

Things have changed since 9cfc66a665ae.

pullrequests: better handling of Mercurial pullrequests with missing revisions - don't crash

Trying to display a Mercurial PR with missing changesets could give a crash
when trying to compute available updates after 3f646f7bac39 did that
c.cs_ranges could be empty. That would normally not happen on Mercurial, but
could happen when restoring an old filesystem backup ... or when using strip on
the server.

pullrequests: better handling of revision range pullrequests with missing revisions - don't crash

Trying to display a revision range PR with missing changesets would give a
crash.

3f646f7bac39 solved a similar problem but caused another one when it did that
c.cs_ranges could be empty.