Subject: Re: [PATCH v2 08/20] remote-curl: accept empty line as terminator

From: Jonathan Nieder <[email-masked]>
Date: Mon, 20 Jun 2011 02:55:41 -0500

Hi,

Dmitry Ivankov wrote:

> As a next step, won't it be better to use "exit\n" or "quit\n"
> as a terminator?

If we were starting over, a "done" command (or "quit" or some other
self-descriptive name) would be a no-brainer.  Unfortunately, there
are existing remote helpers out there --- based on a web search:

 - mediawiki
 - ccnx
 - couch
 - cvs
 - bzr
 - hg

Presumably most already ignore a final blank line and do not check for
"quit".

One possibility would be to emit "done" instead of a blank line if the
remote helper has declared a "done" capability.  But what does it buy?

Side note: a "done" capability doesn't sound like a bad idea, though,
for another reason.  The transport-helper could tell fast-import to
expect a "done" command at the end when importing from a remote helper
declaring it, to catch situations in which the pipe prematurely closes
(for example, because the remote helper has segfaulted).

> I don't see a convention of terminating on a
> blank line in docs,

Yes, this would be nice to document.

> only on EOF.  Also I can imagine a blank
> line being read in a case of communication error

A spurious NL, NL, EOF sequence does not sound likely to me.  If the
command stream is passing through a noisy channel, there are worse
corruptions to worry about (e.g., fetching to the wrong ref).