Re: [JGIT PATCH 4/6] Add QuotedString class to handle C-style quoting rules

From: Robin Rosenberg <robin.rosenberg@dewire.com>
Date: Dec 10, 2008, 23:22 UTC
Message-ID: <200812110022.03719.robin.rosenberg@dewire.com>
In-Reply-To: <1228946751-12708-5-git-send-email-spearce@spearce.org>

onsdag 10 december 2008 23:05:49 skrev Shawn O. Pearce:

> Git patch files can contain file names which are quoted using the
> C language quoting rules.  In order to correctly create or parse

Should we maybe call this Git-style since we really do not care about C (which version btw?).

Show 11 quoted lines

> QuotedString itself is an abstract API so callers can be passed a
> quoting style based on the context of where their output will be
> used, and multiple styles could be supported.  This may be useful
> if jgit ever grows a "git for-each-ref" style of output where Perl,
> Python, Tcl and Bourne style quoting might be necessary.
> 
> References through the singleton QuotedString.C should be able to
> bypass the virtual function table, as the specific type is mentioned
> in the field declaration and that type is final.  A good JIT should
> be able to remove the abstraction costs when the caller has hardcoded
> the quoting style.

Making two interfaces is better. We may share the implementation initially, but parsing file names in Git patches and parsing C strings are different operations.

Show 5 quoted lines

> +	public void testQuote_OctalAll() {
> +		assertQuote("\1", "\\001");
> +		assertQuote("~", "\\176");
> +		assertQuote("\u00ff", "\\303\\277"); // \u00ff in UTF-8
> +	}

What do we do with non-UTF8 names? I think we should follow the logic we use when parsing commits and paths in other places.

> +
> +	public void testDequote_UnknownEscapeQ() {
> +		assertDequote("\\q", "\\q");
> +	}

Would Git generate this style in a name?

Show 12 quoted lines

> +			quote[' '] = 0;
> +			quote['+'] = 0;
> +			quote[','] = 0;
> +			quote['-'] = 0;
> +			quote['.'] = 0;
> +			quote['/'] = 0;
> +			quote['='] = 0;
> +			quote['_'] = 0;
> +			quote['^'] = 0;
> +
> +			quote['\u0007'] = 'a';
> +			quote['\b'] = 'b';

\e = esc

Show 10 quoted lines

> +				default:
> +					// Any other code is taken literally.
> +					//
> +					r[rPtr++] = '\\';
> +					r[rPtr++] = in[inPtr - 1];
> +					continue;
> +				}
> +			}
> +
> +			return decode(Constants.CHARSET, r, 0, rPtr);

Importing methods really obscures things. Please qualify with class name of RawparseUtils here instead.

-- robin