On Fri, Jul 27, 2018 at 7:02 AM Yuya Nishihara <y...@tcha.org> wrote:
> On Fri, 27 Jul 2018 09:30:26 +0200, Boris FELD wrote: > > On 26/07/2018 17:24, Martin von Zweigbergk wrote: > > > >> + if checkout is not None and not > > > node.isnode(checkout): > > > > IIUC, hggit/hgsubversion may pass in a 20-length symbolic name? > > > Yes, this is unfortunate. Having everything working but 20-lenght > > > names > > > is a first step, we can follow-up with more logic to check if the > > > symbol > > > is a valid name. > > > > > > > > > I think Yuya is suggesting that you simply remove the "and not > > > node.isnode(checkout)", > > Yes. > > > We need to detect valid node because they cannot go through revsymbol, > > it breaks the tests without that check. The previous API was able to > > handle any cases, it is now hard to get the same result when needed. > > > > If a 20 chars string is a valid node (ie: exists in the repo) it should > > probably take precedence over the name. It seems extremely unlikely to > > not care about that case. > > Yes. So we need to check if 'checkout in destrepo' instead of > isnode(checkout), > right? > > if checkout in destrepo: > # binary nodeid > elif isrevsymbol(destrepo, checkout): > # work around for hggit/hgsubversion > Oh, the destrepo is available here? Then yes, it's definitely worth checking. I suggested not worrying about the risk of conflicts because I assumed it was awkward to check if the node was in the destination repo at this point in the code.
_______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel