[darcs-users] darcs patch: Resolve issue1373: make --token-chars [^ \t\n] work as...

[Tommy Pettersson]

On Sat, Mar 28, 2009 at 09:43:50PM +1100, Trent W.Buck wrote:
> Sat Mar 28 21:41:48 EST 2009  Trent W. Buck <trentbuck at gmail.com>
>   * Resolve issue1373: make --token-chars [^ \t\n] work as advertised.

I'm afraid this causes a regression. I am responsible for the
strange space check, and it's a cover-up of a deeper misfeature
in darcs' replace patch format, which I never got time to fix
properly. And as it clearly shows now, I did not document this
cover-up properly.

The reason spaces are not allowed in token chars is that the
replace patch format can not store tokens or token char
specifiers that contains spaces. That is the *literal* token
char specifier can not contain spaces, or darcs will fail to
parse it later. Another way to put it is that token char
specifiers with spaces in them causes darcs to create broken
patches.

So this fix should be rolled back ASAP!

When I think about it, my old space check is probably
incomplete, since a token char specifier of [^x] would permit
replace tokens with spaces in them, which would also create a
broken patch.


The underlying problem is that the replace patch format contains
three whitespace-separated strings, the token char specifier,
the replaced token, and the replace token. I think tabs and
newlines are escaped, but there is no quoting or escaping of
spaces, so they can not be allowed. A fix would probably require
a new replace patch format, which would require a new repo
format, or a new subfeature in the current repo format (if that
is possible).

There might be a tricky way to make a simple fix for this. If we
add \s or \_ or something in RegChars.hs as an escape for
spaces, and change the old space check error to inform about
this escape, things could work as expected. The only breakage
would be old replace patches that for some reason contains \c \_
or whatever. They would get a different effect when applied, but
I think that would be fair if someone used an "undefined"
escape.

There is also another limitation of the replace patch.The search for
replace tokens is done line by line. While tokens with
\n are accepted and used as advertised, there will never be any
\n:s in the text snippets the replace is performed on, so the
result of the replace will probably not be as expected.

While the first limitations with spaces could be fixed with a
new patch format or perhaps a new escape, this second \n
limitations should most likely not be "fixed", because that
would make the current commutation of hunks and replace patches
invalid, and to write new commutation rules that can commute
replace patches that can cross hunk boundaries would be a
complicated task.


-- 
Tommy Pettersson <ptp at lysator.liu.se>