[PATCH python-hglib] Add the abilty to trace the protocol between the client and server

[Barry Scott]

> On 16 Oct 2016, at 15:50, Yuya Nishihara <yuya at tcha.org> wrote:
> 
> On Thu, 06 Oct 2016 18:04:47 +0100, Barry A. Scott wrote:
>> # HG changeset patch
>> # User Barry A. Scott <barry at barrys-emacs.org>
>> # Date 1475770955 -3600
>> #      Thu Oct 06 17:22:35 2016 +0100
>> # Branch hglib-protocol-trace
>> # Node ID efc527cc43d7394a5bd0deb1d29c4307592f7528
>> # Parent  6f15cb7cc9cb4427f35c60080f85dbf4ca5abd10
>> Add the abilty to trace the protocol between the client and server.
>> This is useful when debugging issues with driving hg via hglib
>> where output and error messages can be lost.
>> 
>> call setprotocoltrace with the name of a trace function or None.
>> If the trace function is None no tracing is done.
>> The trace function is called with the channel and its data.
> 
> This generally looks good to me. Can you update the commit message and
> coding style?
> 
> https://www.mercurial-scm.org/wiki/ContributingChanges <https://www.mercurial-scm.org/wiki/ContributingChanges>
> 

No problem. I have read that link and the coding style and thought that I coded in the
required style. Can you point to the drop off please?

>> diff -r 6f15cb7cc9cb -r efc527cc43d7 hglib/client.py
>> --- a/hglib/client.py	Mon Jul 18 23:40:45 2016 -0500
>> +++ b/hglib/client.py	Thu Oct 06 17:22:35 2016 +0100
>> @@ -62,6 +62,15 @@
>>         if connect:
>>             self.open()
>> 
>> +        self._protocoltracefn = None
>> +
>> +    def setprotocoltrace( self, tracefn=None ):
>> +        """
>> +        if tracefn is None no trace calls will be made.
>> +        Otherwise tracefn is call as tracefn( channel, data )
>> +        """
>> +        self._protocoltracefn = tracefn
>> +
>>     def __enter__(self):
>>         if self.server is None:
>>             self.open()
>> @@ -119,6 +128,7 @@
>> 
>>     def runcommand(self, args, inchannels, outchannels):
>>         def writeblock(data):
>> +            if self._protocoltracefn is not None: self._protocoltracefn( b'i', data )
> 
> Using a fake channel name seems a bit unfortunate. I slightly prefer b'' or
> None.s

I reasoned that given:

    stdout is ‘o’
    stderr is ‘e’

Clearly if there was an actual channel id for stdin the code would use ‘i’.

I could use b’’ or None, but that would just mean I need more code in the callers code mapping
None or b’’ into ‘i’ for printing.

It seemed better to have an obvious identifier.

Barry



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-devel/attachments/20161018/b74cf7cd/attachment-0002.html>