Submitter | Barry A. Scott |
---|---|
Date | Oct. 22, 2016, 5:37 p.m. |
Message ID | <12c96b1d8dfff261f91e.1477157827@shell-4.local> |
Download | mbox | patch |
Permalink | /patch/17181/ |
State | Changes Requested |
Headers | show |
Comments
On Sat, 22 Oct 2016 18:37:07 +0100, Barry A. Scott wrote: > # HG changeset patch > # User Barry A. Scott <barry@barrys-emacs.org> > # Date 1477157573 -3600 > # Sat Oct 22 18:32:53 2016 +0100 > # Node ID 12c96b1d8dfff261f91ed010cc59ce57bfa63b4f > # Parent 6f15cb7cc9cb4427f35c60080f85dbf4ca5abd10 > Add feature to allow hglib user to get call backs for prompts, output > and errors. "topic: short summary line with no trailing period" https://www.mercurial-scm.org/wiki/ContributingChanges#Patch_descriptions > setcbout(cbout), setcberr(cberr) and setcbprompt(cbprompt) are used to > set the call back function used by the hgclient class. cb stands for > call back. > > cbout is a function that will be called with the stdout data of the > command as it runs. cbout is called with output as it is made available, > which can be as partial lines or multiple lines. > > cberr is a function that will be called with the stderr data of the > command as it runs. cberr is called with output as it is made available, > which can be as partial lines or multiple lines. > > Command that make remote connects can prompt for username and password > for HTTP/HTTPS connections. > > cbprompt is called when hgclient need a response to a prompt from the > server. It receives the max number of bytes to return and the contents > of stdout received so far. The last text sent to either cbout or cberr > will contain the prompt text itself. > > init() has been added to hgclient to allow use of setcbXXX functions > with init(). The init() code is based on the version in __init__.py. So this patch contains 3 different things. Can you send them as separate patches? - add callbacks - change clone() behavior (which seems wrong) - add init() > + def setcbout(self, cbout): > + """ > + cbout is a function that will be called with the stdout data of > + the command as it runs. Call with None to stop getting call backs. > + """ > + self._cbout = cbout > + > + def setcberr(self, cberr): > + """ > + cberr is a function that will be called with the stderr data of > + the command as it runs.Call with None to stop getting call backs. > + """ > + self._cberr = cberr > + > + def setcbprompt(self, cbprompt): > + """ > + cbprompt is used to reply to prompts by the server > + It receives the max number of bytes to return and the > + contents of stdout received so far. > + > + Call with None to stop getting call backs. > + > + cbprompt is never called from merge() or import_() > + which already handle the prompt. > + """ > + self._cbprompt = cbprompt Nit: The "cb" prefix doesn't make sense here because there's little possibility of name conflicts. And I think your setprotocoltrace() covers setcbout/err. Do we still need them? > def clone(self, source=b('.'), dest=None, branch=None, updaterev=None, > - revrange=None): > + revrange=None, pull=False, uncompressed=False, ssh=None, > + remotecmd=None, insecure=False, encoding=None, configs=None): > """ > Create a copy of an existing repository specified by source in a new > directory dest. > @@ -536,9 +584,30 @@ > revrange - include the specified changeset > """ > args = cmdbuilder(b('clone'), source, dest, b=branch, > - u=updaterev, r=revrange) > + u=updaterev, r=revrange, pull=pull, > + uncompresses=uncompressed, e=ssh, > + remotecmd=remotecmd, insecure=insecure) > self.rawcommand(args) > > + if self._path is None: > + self._path = dest > + # become the client for the cloned hg repo > + self.close() > + self._args += ['-R', dest] > + self.open() This changes the behavior of clone(), but hglib should provide a stable API. Also, dest may be a remote (ssh) path. > + def init(self, dest, ssh=None, remotecmd=None, insecure=False, > + encoding=None, configs=None): > + args = util.cmdbuilder(b'init', dest, e=ssh, remotecmd=remotecmd, > + insecure=insecure) > + self.rawcommand(args) > + > + # become the client for this new hg repo > + self._path = dest > + self.close() > + self._args += ['-R', dest] > + self.open() Same here, dest may be a remote path. IMHO, methods of hgclient shouldn't have such magic. Instead, we can provide a utility function on top of plain init(), e.g. client = client.hgclient() client.init(dest) return open(dest) Maybe we can provide it as a replacement for hglib.init(). def init(): try: _initbyhgclient() except 'exception raised if command server failed to start': falls back to the original init()
> On 23 Oct 2016, at 10:53, Yuya Nishihara <yuya@tcha.org> wrote: > > On Sat, 22 Oct 2016 18:37:07 +0100, Barry A. Scott wrote: >> # HG changeset patch >> # User Barry A. Scott <barry@barrys-emacs.org <mailto:barry@barrys-emacs.org>> >> # Date 1477157573 -3600 >> # Sat Oct 22 18:32:53 2016 +0100 >> # Node ID 12c96b1d8dfff261f91ed010cc59ce57bfa63b4f >> # Parent 6f15cb7cc9cb4427f35c60080f85dbf4ca5abd10 >> Add feature to allow hglib user to get call backs for prompts, output >> and errors. > > "topic: short summary line with no trailing period" > > https://www.mercurial-scm.org/wiki/ContributingChanges#Patch_descriptions <https://www.mercurial-scm.org/wiki/ContributingChanges#Patch_descriptions> Will fix. (I did not notice until it was sent). > >> setcbout(cbout), setcberr(cberr) and setcbprompt(cbprompt) are used to >> set the call back function used by the hgclient class. cb stands for >> call back. >> >> cbout is a function that will be called with the stdout data of the >> command as it runs. cbout is called with output as it is made available, >> which can be as partial lines or multiple lines. >> >> cberr is a function that will be called with the stderr data of the >> command as it runs. cberr is called with output as it is made available, >> which can be as partial lines or multiple lines. >> >> Command that make remote connects can prompt for username and password >> for HTTP/HTTPS connections. >> >> cbprompt is called when hgclient need a response to a prompt from the >> server. It receives the max number of bytes to return and the contents >> of stdout received so far. The last text sent to either cbout or cberr >> will contain the prompt text itself. >> >> init() has been added to hgclient to allow use of setcbXXX functions >> with init(). The init() code is based on the version in __init__.py. > > So this patch contains 3 different things. Can you send them as separate > patches? > > - add callbacks > - change clone() behavior (which seems wrong) > - add init() Yes and no. Both clone and init cannot be used with the call backs with these changes. I can package as 3 patches that build on each other. But in my mind these are linked. I will do as you advise, please confirm 3 patches required. > >> + def setcbout(self, cbout): >> + """ >> + cbout is a function that will be called with the stdout data of >> + the command as it runs. Call with None to stop getting call backs. >> + """ >> + self._cbout = cbout >> + >> + def setcberr(self, cberr): >> + """ >> + cberr is a function that will be called with the stderr data of >> + the command as it runs.Call with None to stop getting call backs. >> + """ >> + self._cberr = cberr >> + >> + def setcbprompt(self, cbprompt): >> + """ >> + cbprompt is used to reply to prompts by the server >> + It receives the max number of bytes to return and the >> + contents of stdout received so far. >> + >> + Call with None to stop getting call backs. >> + >> + cbprompt is never called from merge() or import_() >> + which already handle the prompt. >> + """ >> + self._cbprompt = cbprompt > > Nit: The "cb" prefix doesn't make sense here because there's little possibility > of name conflicts. True but it is descriptive. > > And I think your setprotocoltrace() covers setcbout/err. Do we still need them? They solve two distinct problems. protocol tracing provides a way to see the low level, almost unprocessed messages between client and server. Its purpose is to allow inspection and debugging of the protocol. cbout and cderr provide a user of hglib with the post processed output and error information. In production my GUI will use cdout and cderr often, but never protocol trace. But when I cannot figure out what is happening I turn on protocol trace to get an insight into how hglib works. That is, for example, how I found that password: is sent on the ‘e’ channel. Conclusion yes we need both. >> def clone(self, source=b('.'), dest=None, branch=None, updaterev=None, >> - revrange=None): >> + revrange=None, pull=False, uncompressed=False, ssh=None, >> + remotecmd=None, insecure=False, encoding=None, configs=None): >> """ >> Create a copy of an existing repository specified by source in a new >> directory dest. >> @@ -536,9 +584,30 @@ >> revrange - include the specified changeset >> """ >> args = cmdbuilder(b('clone'), source, dest, b=branch, >> - u=updaterev, r=revrange) >> + u=updaterev, r=revrange, pull=pull, >> + uncompresses=uncompressed, e=ssh, >> + remotecmd=remotecmd, insecure=insecure) >> self.rawcommand(args) >> >> + if self._path is None: >> + self._path = dest >> + # become the client for the cloned hg repo >> + self.close() >> + self._args += ['-R', dest] >> + self.open() > > This changes the behavior of clone(), but hglib should provide a stable API. It is a backward compatible extension of the API by adding what hglib provided in the __init__.py version of clone. > Also, dest may be a remote (ssh) path. I do not see how my patch breaks things in that case. I do not assume the dest is a local path. > >> + def init(self, dest, ssh=None, remotecmd=None, insecure=False, >> + encoding=None, configs=None): >> + args = util.cmdbuilder(b'init', dest, e=ssh, remotecmd=remotecmd, >> + insecure=insecure) >> + self.rawcommand(args) >> + >> + # become the client for this new hg repo >> + self._path = dest >> + self.close() >> + self._args += ['-R', dest] >> + self.open() > > Same here, dest may be a remote path. > > IMHO, methods of hgclient shouldn't have such magic. Instead, we can provide > a utility function on top of plain init(), e.g. > > client = client.hgclient() > client.init(dest) > return open(dest) That is semantically identical to my patch. See __init__.py:11 and client.py:49 of the unpatched source. However that does not work for the call back case. Here is how I use init. c = hglib.open( None ) c.setcbout(cbout) c.setcberr(cbout) c.init(b'path/for/new/repo') c.status() # works on new repo This works for the local path case. And this for clone: c = hglib.open( None ) c.setcbout(cbout) c.setcberr(cbout) c.setcbprompt(cbprompt) c.clone(b'http://example.com/hgrepo', b'path/for/new/repo <http://example.com/hgrepo',%20b'path/for/new/repo>') c.status() # works on new repo > > Maybe we can provide it as a replacement for hglib.init(). > > def init(): > try: > _initbyhgclient() > except 'exception raised if command server failed to start': > falls back to the original init() I choose to leave the hglib.init and hglib.clone code untouched for as its uses of it cannot care about call backs. Barry
On Mon, 24 Oct 2016 11:04:39 +0100, Barry Scott wrote: > > So this patch contains 3 different things. Can you send them as separate > > patches? > > > > - add callbacks > > - change clone() behavior (which seems wrong) > > - add init() > > Yes and no. Both clone and init cannot be used with the call backs with these changes. > > I can package as 3 patches that build on each other. But in my mind these are linked. > > I will do as you advise, please confirm 3 patches required. https://www.mercurial-scm.org/wiki/ContributingChanges#Submission_checklist "6. patch does just one thing (if you need a bullet list, split your patch)" I would split clone/init changes since they solve separate issues on top of the new callback API. > >> + def setcbprompt(self, cbprompt): > >> + """ > >> + cbprompt is used to reply to prompts by the server > >> + It receives the max number of bytes to return and the > >> + contents of stdout received so far. > >> + > >> + Call with None to stop getting call backs. > >> + > >> + cbprompt is never called from merge() or import_() > >> + which already handle the prompt. > >> + """ > >> + self._cbprompt = cbprompt > > > > Nit: The "cb" prefix doesn't make sense here because there's little possibility > > of name conflicts. > > True but it is descriptive. Hmm, but it introduces new naming style, right? I have no strong opinion about this, but "prompt" seems just fine because we already have one. > > And I think your setprotocoltrace() covers setcbout/err. Do we still need them? > > They solve two distinct problems. > > protocol tracing provides a way to see the low level, almost unprocessed messages between client and server. > Its purpose is to allow inspection and debugging of the protocol. > > cbout and cderr provide a user of hglib with the post processed output and error information. > > In production my GUI will use cdout and cderr often, but never protocol trace. But when I cannot > figure out what is happening I turn on protocol trace to get an insight into how hglib works. > That is, for example, how I found that password: is sent on the ‘e’ channel. > > Conclusion yes we need both. Okay, I don't think they are required, but I agree they are semantically different from setprotocoltrace(). Another option is to provide an interface to set a single callback object that handles o/e/I/L channels altogether, so we could provide a helper class to translate o+L and e+L sequences to .prompt() and .password() respectively. TortoiseHg handles user interaction in that way: https://bitbucket.org/tortoisehg/thg/src/3.9.2/tortoisehg/hgqt/cmdcore.py#cmdcore.py-44 BTW, is hglib usable in GUI? I think its synchronous I/O will behave pretty bad in GUI environment. > >> def clone(self, source=b('.'), dest=None, branch=None, updaterev=None, > >> - revrange=None): > >> + revrange=None, pull=False, uncompressed=False, ssh=None, > >> + remotecmd=None, insecure=False, encoding=None, configs=None): > >> """ > >> Create a copy of an existing repository specified by source in a new > >> directory dest. > >> @@ -536,9 +584,30 @@ > >> revrange - include the specified changeset > >> """ > >> args = cmdbuilder(b('clone'), source, dest, b=branch, > >> - u=updaterev, r=revrange) > >> + u=updaterev, r=revrange, pull=pull, > >> + uncompresses=uncompressed, e=ssh, > >> + remotecmd=remotecmd, insecure=insecure) > >> self.rawcommand(args) > >> > >> + if self._path is None: > >> + self._path = dest > >> + # become the client for the cloned hg repo > >> + self.close() > >> + self._args += ['-R', dest] > >> + self.open() > > > > This changes the behavior of clone(), but hglib should provide a stable API. > > It is a backward compatible extension of the API by adding what hglib provided in the __init__.py > version of clone. No. client = hglib.open() client.root() client.clone('.', 'somewhere') client.root() Here clone() shouldn't reopen the client. Be aware that hglib.open() opens the repository found in cwd by default. Maybe you can get some errors by running the test. $ python test.py --with-doctest --with-hg path/to/hg > > Also, dest may be a remote (ssh) path. > > I do not see how my patch breaks things in that case. I do not assume the > dest is a local path. If dest isn't a local path, open() with ['-R', dest] would fail. > >> + def init(self, dest, ssh=None, remotecmd=None, insecure=False, > >> + encoding=None, configs=None): > >> + args = util.cmdbuilder(b'init', dest, e=ssh, remotecmd=remotecmd, > >> + insecure=insecure) > >> + self.rawcommand(args) > >> + > >> + # become the client for this new hg repo > >> + self._path = dest > >> + self.close() > >> + self._args += ['-R', dest] > >> + self.open() > > > > Same here, dest may be a remote path. > > > > IMHO, methods of hgclient shouldn't have such magic. Instead, we can provide > > a utility function on top of plain init(), e.g. > > > > client = client.hgclient() > > client.init(dest) > > return open(dest) > > That is semantically identical to my patch. > > See __init__.py:11 and client.py:49 of the unpatched source. > > However that does not work for the call back case. Here is how I use init. > > c = hglib.open( None ) > c.setcbout(cbout) > c.setcberr(cbout) > c.init(b'path/for/new/repo') > c.status() # works on new repo > > This works for the local path case. And this for clone: > > c = hglib.open( None ) > c.setcbout(cbout) > c.setcberr(cbout) > c.setcbprompt(cbprompt) > c.clone(b'http://example.com/hgrepo', b'path/for/new/repo <http://example.com/hgrepo',%20b'path/for/new/repo>') > c.status() # works on new repo > > > > > Maybe we can provide it as a replacement for hglib.init(). > > > > def init(): > > try: > > _initbyhgclient() > > except 'exception raised if command server failed to start': > > falls back to the original init() > > > I choose to leave the hglib.init and hglib.clone code untouched for as its uses of it > cannot care about call backs. Maybe they could take a callback as an argument?
On Monday, 24 October 2016 22:30:08 BST Yuya Nishihara wrote: > On Mon, 24 Oct 2016 11:04:39 +0100, Barry Scott wrote: > > > So this patch contains 3 different things. Can you send them as separate > > > patches? > > > > > > - add callbacks > > > - change clone() behavior (which seems wrong) > > > - add init() > > > > Yes and no. Both clone and init cannot be used with the call backs with > > these changes. > > > > I can package as 3 patches that build on each other. But in my mind these > > are linked. > > > > I will do as you advise, please confirm 3 patches required. > > https://www.mercurial-scm.org/wiki/ContributingChanges#Submission_checklist > > "6. patch does just one thing (if you need a bullet list, split your patch)" > > I would split clone/init changes since they solve separate issues on top of > the new callback API. > > > >> + def setcbprompt(self, cbprompt): > > >> + """ > > >> + cbprompt is used to reply to prompts by the server > > >> + It receives the max number of bytes to return and the > > >> + contents of stdout received so far. > > >> + > > >> + Call with None to stop getting call backs. > > >> + > > >> + cbprompt is never called from merge() or import_() > > >> + which already handle the prompt. > > >> + """ > > >> + self._cbprompt = cbprompt > > > > > > Nit: The "cb" prefix doesn't make sense here because there's little > > > possibility of name conflicts. > > > > True but it is descriptive. > > Hmm, but it introduces new naming style, right? I have no strong opinion > about this, but "prompt" seems just fine because we already have one. prompt is not part of the public API. > > > > And I think your setprotocoltrace() covers setcbout/err. Do we still > > > need them?> > > They solve two distinct problems. > > > > protocol tracing provides a way to see the low level, almost unprocessed > > messages between client and server. Its purpose is to allow inspection > > and debugging of the protocol. > > > > cbout and cderr provide a user of hglib with the post processed output and > > error information. > > > > In production my GUI will use cdout and cderr often, but never protocol > > trace. But when I cannot figure out what is happening I turn on protocol > > trace to get an insight into how hglib works. That is, for example, how I > > found that password: is sent on the ‘e’ channel. > > > > Conclusion yes we need both. > > Okay, I don't think they are required, but I agree they are semantically > different from setprotocoltrace(). > > Another option is to provide an interface to set a single callback object > that handles o/e/I/L channels altogether, so we could provide a helper class > to translate o+L and e+L sequences to .prompt() and .password() > respectively. I've assumed that its better to leave the hglib patch simple. As we talked about before a getcredentials call back would be great. But That needs hg itself to change. I sugest that we stay with this API until that is possible. > > TortoiseHg handles user interaction in that way: > https://bitbucket.org/tortoisehg/thg/src/3.9.2/tortoisehg/hgqt/cmdcore.py#cm > dcore.py-44 > > BTW, is hglib usable in GUI? I think its synchronous I/O will behave pretty > bad in GUI environment. With these patches its usable :-) I run any long running hg operation in a background thread. > > >> def clone(self, source=b('.'), dest=None, branch=None, > > >> updaterev=None, > > >> > > >> - revrange=None): > > >> + revrange=None, pull=False, uncompressed=False, ssh=None, > > >> > > >> + remotecmd=None, insecure=False, encoding=None, configs=None): > > >> """ > > >> Create a copy of an existing repository specified by source in > > >> a new > > >> directory dest. > > >> > > >> @@ -536,9 +584,30 @@ > > >> > > >> revrange - include the specified changeset > > >> """ > > >> args = cmdbuilder(b('clone'), source, dest, b=branch, > > >> > > >> - u=updaterev, r=revrange) > > >> + u=updaterev, r=revrange, pull=pull, > > >> + uncompresses=uncompressed, e=ssh, > > >> + remotecmd=remotecmd, insecure=insecure) > > >> > > >> self.rawcommand(args) > > >> > > >> + if self._path is None: > > >> + self._path = dest > > >> + # become the client for the cloned hg repo > > >> + self.close() > > >> + self._args += ['-R', dest] > > >> + self.open() > > > > > > This changes the behavior of clone(), but hglib should provide a stable > > > API.> > > It is a backward compatible extension of the API by adding what hglib > > provided in the __init__.py version of clone. > > No. > > client = hglib.open() > client.root() > client.clone('.', 'somewhere') > client.root() > > Here clone() shouldn't reopen the client. Be aware that hglib.open() opens > the repository found in cwd by default. What I wanted was to be able to create the hgclient() object, setup call backs then do the init() or the clone() with the call backs being called as required. If I drop the reopenning that avoids your objects to semantic changhed. Indeed in my GUI code I do not use the hgclient() object that I do the clone() of init() on. I use a new object. > Maybe you can get some errors by running the test. > > $ python test.py --with-doctest --with-hg path/to/hg Will do. > > > > Also, dest may be a remote (ssh) path. > > > > I do not see how my patch breaks things in that case. I do not assume the > > dest is a local path. > > If dest isn't a local path, open() with ['-R', dest] would fail. I'll drop the reopen code. > > > >> + def init(self, dest, ssh=None, remotecmd=None, insecure=False, > > >> + encoding=None, configs=None): > > >> + args = util.cmdbuilder(b'init', dest, e=ssh, > > >> remotecmd=remotecmd, > > >> + insecure=insecure) > > >> + self.rawcommand(args) > > >> + > > >> + # become the client for this new hg repo > > >> + self._path = dest > > >> + self.close() > > >> + self._args += ['-R', dest] > > >> + self.open() > > > > > > Same here, dest may be a remote path. > > > > > > IMHO, methods of hgclient shouldn't have such magic. Instead, we can > > > provide a utility function on top of plain init(), e.g. > > > > > > client = client.hgclient() > > > client.init(dest) > > > return open(dest) > > > > That is semantically identical to my patch. > > > > See __init__.py:11 and client.py:49 of the unpatched source. > > > > However that does not work for the call back case. Here is how I use init. > > > > c = hglib.open( None ) > > c.setcbout(cbout) > > c.setcberr(cbout) > > c.init(b'path/for/new/repo') > > c.status() # works on new repo > > > > This works for the local path case. And this for clone: > > c = hglib.open( None ) > > c.setcbout(cbout) > > c.setcberr(cbout) > > c.setcbprompt(cbprompt) > > c.clone(b'http://example.com/hgrepo', b'path/for/new/repo > > <http://example.com/hgrepo',%20b'path/for/new/repo>') c.status() # > > works on new repo > > > > > > Maybe we can provide it as a replacement for hglib.init(). > > > > > > def init(): > > > try: > > > _initbyhgclient() > > > > > > except 'exception raised if command server failed to start': > > > falls back to the original init() > > > > I choose to leave the hglib.init and hglib.clone code untouched for as its > > uses of it cannot care about call backs. > > Maybe they could take a callback as an argument? You will recall that where I started and you pointed out that it should be set once as state of hgclient(). I'll genreate a new set of patches for this and post once I see that test.py is happy. Barry
FYI: My code that uses this patch is in SCM Workbench. Here is the code I use for init() and clone(): https://github.com/barry-scott/scm-workbench/blob/master/Source/Hg/ wb_hg_project.py#L65 Here is the object that handles the call back interface and map into my GUI's world: https://github.com/barry-scott/scm-workbench/blob/master/Source/Hg/ wb_hg_project.py#L448 Barry
On Mon, 24 Oct 2016 15:57:30 +0100, Barry Scott wrote: > On Monday, 24 October 2016 22:30:08 BST Yuya Nishihara wrote: > > On Mon, 24 Oct 2016 11:04:39 +0100, Barry Scott wrote: > > > >> + def setcbprompt(self, cbprompt): > > > >> + """ > > > >> + cbprompt is used to reply to prompts by the server > > > >> + It receives the max number of bytes to return and the > > > >> + contents of stdout received so far. > > > >> + > > > >> + Call with None to stop getting call backs. > > > >> + > > > >> + cbprompt is never called from merge() or import_() > > > >> + which already handle the prompt. > > > >> + """ > > > >> + self._cbprompt = cbprompt > > > > > > > > Nit: The "cb" prefix doesn't make sense here because there's little > > > > possibility of name conflicts. > > > > > > True but it is descriptive. > > > > Hmm, but it introduces new naming style, right? I have no strong opinion > > about this, but "prompt" seems just fine because we already have one. > > prompt is not part of the public API. rawcommand() is a public API. hgclient() exposes APIs from two different levels. > > > > And I think your setprotocoltrace() covers setcbout/err. Do we still > > > > need them?> > > > They solve two distinct problems. > > > > > > protocol tracing provides a way to see the low level, almost unprocessed > > > messages between client and server. Its purpose is to allow inspection > > > and debugging of the protocol. > > > > > > cbout and cderr provide a user of hglib with the post processed output and > > > error information. > > > > > > In production my GUI will use cdout and cderr often, but never protocol > > > trace. But when I cannot figure out what is happening I turn on protocol > > > trace to get an insight into how hglib works. That is, for example, how I > > > found that password: is sent on the ‘e’ channel. > > > > > > Conclusion yes we need both. > > > > Okay, I don't think they are required, but I agree they are semantically > > different from setprotocoltrace(). > > > > Another option is to provide an interface to set a single callback object > > that handles o/e/I/L channels altogether, so we could provide a helper class > > to translate o+L and e+L sequences to .prompt() and .password() > > respectively. > > I've assumed that its better to leave the hglib patch simple. > > As we talked about before a getcredentials call back would be great. > But That needs hg itself to change. I sugest that we stay with this API > until that is possible. Yeah, that's why I suggested you to extend prompt() to take err output. That wouldn't be neat, but was the simplest change I could think of. Since we're introducing more functions, it might be worth thinking a better interface. > > > > Also, dest may be a remote (ssh) path. > > > > > > I do not see how my patch breaks things in that case. I do not assume the > > > dest is a local path. > > > > If dest isn't a local path, open() with ['-R', dest] would fail. > > I'll drop the reopen code. Sounds fine. > > > I choose to leave the hglib.init and hglib.clone code untouched for as its > > > uses of it cannot care about call backs. > > > > Maybe they could take a callback as an argument? > > You will recall that where I started and you pointed out that it should be set > once as state of hgclient(). Yes, that's because hgclient has many commands which can potentially need a callback. I think that's okay to pass a callback which will be set to new hgclient object created by open()/init()/clone(). Anyway, as you don't use hglib.init() and .clone(), we don't need to discuss that further.
Patch
diff -r 6f15cb7cc9cb -r 12c96b1d8dff hglib/client.py --- a/hglib/client.py Mon Jul 18 23:40:45 2016 -0500 +++ b/hglib/client.py Sat Oct 22 18:32:53 2016 +0100 @@ -45,6 +45,7 @@ def __init__(self, path, encoding, configs, connect=True): self._args = [hglib.HGPATH, 'serve', '--cmdserver', 'pipe', '--config', 'ui.interactive=True'] + self._path = path if path: self._args += ['-R', path] if configs: @@ -59,9 +60,40 @@ # include the hidden changesets if True self.hidden = None + self._cbout = None + self._cberr = None + self._cbprompt = None + if connect: self.open() + def setcbout(self, cbout): + """ + cbout is a function that will be called with the stdout data of + the command as it runs. Call with None to stop getting call backs. + """ + self._cbout = cbout + + def setcberr(self, cberr): + """ + cberr is a function that will be called with the stderr data of + the command as it runs.Call with None to stop getting call backs. + """ + self._cberr = cberr + + def setcbprompt(self, cbprompt): + """ + cbprompt is used to reply to prompts by the server + It receives the max number of bytes to return and the + contents of stdout received so far. + + Call with None to stop getting call backs. + + cbprompt is never called from merge() or import_() + which already handle the prompt. + """ + self._cbprompt = cbprompt + def __enter__(self): if self.server is None: self.open() @@ -131,7 +163,6 @@ while True: channel, data = self._readchannel() - # input channels if channel in inchannels: writeblock(inchannels[channel](data)) @@ -164,9 +195,25 @@ It receives the max number of bytes to return """ out, err = BytesIO(), BytesIO() - outchannels = {b('o') : out.write, b('e') : err.write} + outchannels = {} + if self._cbout is None: + outchannels[b('o')] = out.write + else: + def out_handler(data): + out.write(data) + self._cbout(data) + outchannels[b('o')] = out_handler + if self._cberr is None: + outchannels[b('e')] = err.write + else: + def err_handler(data): + err.write(data) + self._cberr(data) + outchannels[b('e')] = err_handler inchannels = {} + if prompt is None: + prompt = self._cbprompt if prompt is not None: def func(size): reply = prompt(size, out.getvalue()) @@ -524,7 +571,8 @@ return out def clone(self, source=b('.'), dest=None, branch=None, updaterev=None, - revrange=None): + revrange=None, pull=False, uncompressed=False, ssh=None, + remotecmd=None, insecure=False, encoding=None, configs=None): """ Create a copy of an existing repository specified by source in a new directory dest. @@ -536,9 +584,30 @@ revrange - include the specified changeset """ args = cmdbuilder(b('clone'), source, dest, b=branch, - u=updaterev, r=revrange) + u=updaterev, r=revrange, pull=pull, + uncompresses=uncompressed, e=ssh, + remotecmd=remotecmd, insecure=insecure) self.rawcommand(args) + if self._path is None: + self._path = dest + # become the client for the cloned hg repo + self.close() + self._args += ['-R', dest] + self.open() + + def init(self, dest, ssh=None, remotecmd=None, insecure=False, + encoding=None, configs=None): + args = util.cmdbuilder(b'init', dest, e=ssh, remotecmd=remotecmd, + insecure=insecure) + self.rawcommand(args) + + # become the client for this new hg repo + self._path = dest + self.close() + self._args += ['-R', dest] + self.open() + def commit(self, message=None, logfile=None, addremove=False, closebranch=False, date=None, user=None, include=None, exclude=None, amend=False):