yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> logcmdutil.py:97
> +            oldchunks = chunks
> +            chunks = patch.difflabel(lambda **kwargs: oldchunks, 
> opts=diffopts)
> +        if ui.canbatchlabelwrites():

I slightly prefer passing `chunks` as an argument in place of `oldchunks` 
trick. But this is really minor nit.

If we make all diffui() batchable (e.g. _exportsingle()) as a follow up, 
difflabel() can
just take `chunks` as an argument.

> ui.py:881
>  
> +    def writenolabels(self, **opts):
> +        '''check if write actually uses the label'''

Can you remove unused `opts` parameter?

I don't think it will be any useful since we want to feed chunks
at once where opts may vary.

> ui.py:886
> +                return True
> +        return self._colormode is None
> +

Perhaps "label -> true" would be preferable than double negative "no label -> 
false".

> ui.py:890
> +        '''check if write calls with labels are batchable'''
> +        assert not self.writenolabels()
> +        # Windows color printing is special, see ``write``.

I think this assert is irrelevant since "no label" writes can be batched.

REPOSITORY
  rHG Mercurial

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

To: joerg.sonnenberger, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to