From: Jakub Narebski <jnareb@gmail.com>
Date: Fri, 09 Oct 2009 19:27:35 GMT
Subject: Re: [RFC PATCH 1/4] Document the HTTP transport protocol
Message-ID: <m3eipcgyfv.fsf@localhost.localdomain>
In-Reply-To: <1255065768-10428-2-git-send-email-spearce@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> +	empty_list     = PKT-LINE(id SP "capabilities^{}" NUL cap_list LF)
> +
> +	non_empty_list = PKT-LINE(id SP name NUL cap_list LF)
> +	                 *ref_record
> +
> +	cap_list      = *(SP capability) SP

Errr... are you sure?  Because from examples it looks like cap_list
(capabilities list) is a list of space *separated* capabilities, while
the above requires also both leading and trailing space.  Shouldn't it
be

	cap_list      = capability *(SP capability)

Also the format for capability is not defined; I guess only 
a-z, 0-9, '-' and '_' are allowed in capability name.


BTW. is it possible to not have capability list?

> +	HEX           = "0".."9" | "a".."f"

Do you plan allowing also upper case letters, while server and client
SHOULD use lowercase?  Because if you do, then RFC 5234 which defines
ABNF you seem to be using here has HEXDIG defined.

> +	NL            = <US-ASCII NUL, null (0)>

Why not NUL?

> +	LF            = <US-ASCII LF,  linefeed (10)>
> +	SP            = <US-ASCII SP,  horizontal-tab (9)>
                                       ^^^^^^^^^^^^^^-- o'rly?

Those are pre-defined in ABNF, e.g.

	SP             =  %x20

> +References
> +----------
> +
> +link:http://www.ietf.org/rfc/rfc1738.txt[RFC 1738: Uniform Resource Locators (URL)]
> +link:http://www.ietf.org/rfc/rfc2616.txt[RFC 2616: Hypertext Transfer Protocol -- HTTP/1.1]

You should also reference the following RFCs:
 * "RFC 5234: Augmented BNF for Syntax Specifications: ABNF"
 * "RFC 2119: Key words for use in RFCs to Indicate Requirement Levels"

-- 
Jakub Narebski
Poland
ShadeHawk on #git