Showing obsolescence markers stopped working / old behavior was surprising

Issue #330 resolved
Manuel Jacob created an issue

Mercurial renamed "precursors" to "predecessors" in version 4.4, so they’re used synonymously. When the alias was dropped in Mercurial 4.6, this line stopped working: https://bitbucket.org/conservancy/kallithea/src/0b942d34e4deb9b43558d880bc5018c2aba2f2be/kallithea/lib/vcs/backends/hg/changeset.py#lines-103

Mercurial moved the successorssets function from the obsolete module to the obsutil module. Alias was also dropped in Mercurial 4.6, so the following line stopped working: https://bitbucket.org/conservancy/kallithea/src/0b942d34e4deb9b43558d880bc5018c2aba2f2be/kallithea/lib/vcs/backends/hg/changeset.py#lines-91

The code is called from here: https://bitbucket.org/conservancy/kallithea/src/0b942d34e4deb9b43558d880bc5018c2aba2f2be/kallithea/templates/changeset/changeset.html#lines-106. However, the error is silently ignored (at least I didn't find a way to show it).

Even when this naming error is fixed, I find the current behavior a bit surprising:

  • The successors property returns only the newest changesets in the chain of successors.
  • The precursors property returns the changesets referenced by the obsolescence markers, regardless of whether they're in the repository or not.

Instead, I would propose that both properties return the closest changesets in the obsolescence chain that are in the repository. This would be useful for stepping through the obsolescence chain and also matches the behavior of Mercurial’s templates and TortoiseHG’s repository view. An API for getting both of these sets were added in Mercurial 4.3.

Is there a reason why Kallithea introduced its own terminology ("Replaced by" instead of "Succeeded by" or "Successors")?

Comments (8)

  1. Manuel Jacob reporter

    See pull request #403 for a change that implements the proposed new semantics for Mercurial versions 4.3 or later (on older versions of Mercurial it falls back to the curent implementation).

  2. Mads Kiilerich

    If there is a reason for special terminology, then it is probably an attempt at abstracting the VCS without making it too Mercurial specific. But this feature is for Mercurial anyway.

    The proposed cleanup seems reasonable.

  3. Manuel Jacob reporter

    @Mads Kiilerich Did you see pull request #403 and pull request #406? The latter adds a test and fixes the compatibility issue. The first changes the behavior to the one proposed above (with the disadvantage of having different behavior depending on which Mercurial version is used). Currently the first isn’t based on the latter, but I could rebase it (or mix and match from both pull requests freely).

  4. Log in to comment