D774: merge: add pathconflict merge state

[ryanmce (Ryan McElroy)]

ryanmce requested changes to this revision.
ryanmce added a comment.
This revision now requires changes to proceed.


  Logic looks good to me. Just a couple of inline nits about comments to improve understanding that I think would be useful.

INLINE COMMENTS

> merge.py:84-87
> +    u: unresolved conflict
> +    r: resolved conflict
> +    pu: unresolved path conflict (file conflicts with directory)
> +    pr: resolved path conflict

I think it's worth mentioning why these need to be separate states (eg, so you can round-trip from resolved path conflict to unresolved path conflict)

> merge.py:88
> +    pr: resolved path conflict
> +    d: driver-resolved conflict
>      '''

Thanks for adding this!

> merge.py:364-365
>                  records.append(('D', '\0'.join([d] + v)))
> +            elif v[0] in ('pu', 'pr'):
> +                records.append(('P', '\0'.join([d] + v)))
>              # v[1] == local ('cd'), v[6] == other ('dc') -- not supported by

Even though there's not a lot of good comments here, now, could you add some when you modify this?

Something like "if the state includes a path conflict (pu or pr), include a Path (P) record which ...." (I don't know what to write here -- why don't we need to distinguish between pu and pr in the record? A reader should not have to figure this out on their own, ideally.

> merge.py:434
>  
> +    def addpath(self, path, frename, forigin):
> +        """add a new conflicting path to the merge state

Maybe this should be called `addpathconflict`?

REPOSITORY
  rHG Mercurial

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

To: mbthomas, #hg-reviewers, ryanmce
Cc: ryanmce, mercurial-devel