Re: [Monotone-devel] Re: netsync connection info cleanup

[Thomas Keller]

Am 09.06.10 19:01, schrieb Timothy Brownawell:
> On 04/28/2010 06:06 AM, Thomas Keller wrote:
>> Am 28.04.2010 12:36, schrieb Thomas Moschny:
>>> Am Sun, 18 Apr 2010 20:49:37 +0200
>>> schrieb Thomas Keller<address@hidden>:
>>>
>>>> Secondly, I'd actively deprecate any branch name which does not match
>>>> the following regular expression, i.e. by throwing a warning when a
>>>> branch cert which a different value is created:
>>>>
>>>>     ^([\w\d]+([-][\w\d]+)*)(\.[\w\d]+([-][\w\d]+)*)*$
>>>
>>> Sounds good to me, but maybe we have to ask our users, whether that'd
>>> be ok for them. And we should still allow them to use other branch names
>>> if they wish so (because technically, there's no need for such a
>>> restriction).
>>
>> Depending on the actual URI scheme we'll be using, we can further adapt
>> the regular expression above, like f.e. if the URI scheme will look like
>> this:
>>
>>     scheme://address@hidden/db?include,-exclude,include
> 
> This is in the nvm.connection_info_cleanup branch now. Are there any
> objections to merging it? It doesn't include any string changes.

I understand that it might be a good idea to have this in before
0.99/1.0.0 because it breaks BC a bit (well, if anybody used this
feature at all) and because you need to know the proper URI scheme for
usher as well, but I was just about to start hacking on this branch
again and wanted to do a couple of other things on my way. So I wouldn't
mind to see the whole thing in 1.1.0 or later.

>> this should be sufficient I think (untested):
>>
>>     ^[^-,*][^,*]+$
> 
> I think we also don't like '+' and '%' due to urlencoding. 

Apropos '+' - this shouldn't be needed - I forgot to exclude whitespace
above. I agree with '%' and would also add '?'. The above regex also
disallowed single char branch names, so this should work better:

        ^[^-,*%\s][^,*%\s]*$

We could still disallow '+' if we'd want to make inclusion also
explicit, but some people disagreed on this. Personally I won't mind.

> Any
> objections to requiring an --allow-discouraged-branch-names option to
> create branch certs that don't match /^[^-,*+%][^,*+%]*$/?

Hrm - should we really disallow them by default? Another option could be
to just issue a warning and let the user go ahead. Wireing in the code
which errors out in an invalid case and correctly rolling back might be
cumbersome, given the fact that we have a couple of places from which we
create branch certs (approve, commit -b, cert, automate cert, setup,
import, cvs_import, ...)

> I guess this would have to be after this upcoming release, due to the
> new translatable strings it would have.

Yes, definitely.

Thomas.

-- 
GPG-Key 0x160D1092 | address@hidden | http://thomaskeller.biz
Please note that according to the EU law on data retention, information
on every electronic information exchange might be retained for a period
of six months or longer: http://www.vorratsdatenspeicherung.de/?lang=en

signature.asc
Description: OpenPGP digital signature