D6140: revset: add new contiguous(x) function for "x::x"

[martinvonz (Martin von Zweigbergk)]

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.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6140

To: martinvonz, #hg-reviewers
Cc: yuja, av6, spectral, gracinet, marmoute, mercurial-devel