D6140: revset: add new contiguous(x) function for "x::x"
martinvonz (Martin von Zweigbergk)
phabricator at mercurial-scm.org
Tue Mar 19 13:08:09 EDT 2019
martinvonz added a comment.
In https://phab.mercurial-scm.org/D6140#89733, @marmoute wrote:
> I spend some more time thinking about it, especially about the semantic
> I would expect from a `contiguous` function. My conclusion is that I'm
> seeing something call `contiguous` more as a filter than something
> adding element to the set.
I can see the argument that most functions are nouns (which is not exactly what you're saying, but your comment reminded me of it). I thought of `interior` before, but didn't mention it because it sounds too much like it's exclusive of the input set. Other than that, I kind of like that name.
> The first idea that comes to mind is to filter elements in a set that
> are "contiguous to" another. `contiguous(X, Y)` would return element
> from Y which are connected to an element in X (through Y). Another
> possible behavior would be for `contiguous(X)` to return `X` if the set
> is contigous and nothing otherwise. (Of course I'm not suggesting we
> implement theses now, I am just trying to get a sense of what the name
> could mean and how user could get confused.
> What Martin is trying to achieve here is a simple function to express
> X::X. So maybe it could be a special case of a function expression X::Y.
> However, we don't have this function right now. Maybe we jut need to
> introduce it, with a clear name eg `revbetween` or `connected` or
> `dagrange`, with an optionnal second arguments:
> - X::Y → dagrange(X, Y)
> - X::X → dagrange(X)
And `::X` and `X::`? Maybe `dagrange(heads=X)` and `dagrange(roots=X)`, respectively? That implies that `roots` and `heads` default to `null` and `heads()`, but it seems weird that `dagrange(X)` is equivalent to `dagrange(roots=X, heads=X)`.
> Finding the name for this function will be "simpler" because the two
> arguments value would be clearer. In the case we are discussing right
> now, ``contiguous(X, Y)` seems less clear to me than `connect(X, Y)` or
> `dagrange(X, Y)`.
> Conclusion, I am not very enthousiastic for `contiguous`. If we stay on
> a same API (single argument) I think `connect` or `fillgap` are better
> fits. However I feel like introducing an explicit function for `::` is
> probably the best way to have a clear API.
> In all cases, "contiguous" is not awful, so even if I don't like it
> much, I'll survive if there is a consensus behind it.
I feel like `connect` suffers more from the problem you initially raised: it sounds like it will add commits down to a common ancestor (or up to a common descendant).
I'd be fine with "fillgap" (which is also not a noun, though). But we already had a few votes for "contiguous" and I don't want to bikeshed this name much more.
To: martinvonz, #hg-reviewers
Cc: yuja, av6, spectral, gracinet, marmoute, mercurial-devel
More information about the Mercurial-devel