On 2008-12-31 09:07:51 +0100, Hannes Eder wrote:
> On Mon, Dec 29, 2008 at 10:21 PM, Karl Hasselström <kha@xxxxxxxxxxx> wrote:
>
> > There's a small inconsistency: you fail if the name contains "..",
> > but correct single bad characters.
>
> ".." is used to denote patch name ranges [<patch1>..<patch2>] for
> commands like "stg pop", "stg push", and so forth, therefore I think
> it is wise to exclude ".." from single patch names [<patch3>].
Yes. But what I meant was that it's a tad inconsistent to fail on some
illegal patch names, and correct others. You should either always fail
(my preference), or always correct.
> Maybe we should start defining what a 'valid' patch name has to look
> like, i.e. define
>
> def is_patch_name_valid(patchname)
Yes, exactly. This function could be called
1. when validating user input in e.g. "stg new";
2. in an assert at the end of the function that constructs a patch
name from the commit message; and
3. in an assert just before we try to actually create a patch with a
given name.
(2) and (3) aren't really necessary, of course; they're just there to
catch bugs.
If you define such a function, be liberal in forbidding stuff. It's
easy to relax the rules later, but hard to tighten them since we have
to deal with existing repositories with illegal patch names. It's
probably a good idea to look at what git allows in its ref names, and
additionally forbid "/" and anything else you can think of.
--
Karl Hasselström, kha@xxxxxxxxxxx
www.treskal.com/kalle
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
