On Tue, Apr 30, Josef Reidinger wrote:
> > I do not think it makes sense to allow straneg characters in a symbol.
> > I would make the constructor of YCPSymbol to that it does not accept
> > invalid characters in a symbol. To not add unneeded checking to the
> > case where YCPSymbol constructor is called from Parser (where validity
> > of strict is encured anyway), I added a boolean defaulting to false
> > to YCPSymbol constructor. If it is true constructor checks string
> > and restricts it to valid characters. Only change in yast2-xml would
> > be to call constructor with true as second parameter and check
> > constructed Symbol content, if it is not what is expected treat the
> > entry as YCPString (would would happend anyway if e.g. type string
> > is mistyped).
>
> Well, few comments to diff. Choose to filter out invalid characters is
> valid approach. Problem is that code don't filter out invalid characters
> if I read code properly. Code just choose first substring with valid
> characters. Code with `test=1 construct `test instead of `test1 . So I
> think it should be properly documented if it is intention and better
> function name should be choosen.
Indeed, it returns the match of the regular expression used in
scanner.ll for symbol ("([[:alpha:]_][[:alnum:]_]+|[[:alpha:]][[:alnum:]_]*)"
at start of string.
So there is not really valid or invalid characters (e.g. "_22" is valid,
"22_" is invalid, "_" is invalid, "__" is valid). I recognized this while
implementing it, but I consider the name more or less ok, it returns the
valid part of given string.
> Also I think that you should inside
> constructor call y2warning or y2error that some characters are invalid
> and not let it on constructor user. And last but not least default value
Indeed a y2warning would make sense.
> for constructor should be true from my point of view ( so by
> default filter out character ). So other code that use
> SymbolConstructor like various bindings get automatic valid symbol. We
> should be by default on safe side.
I would assume most constructed YCPSymbol values are from parsing valid
ycp code and not from xml-to-ycp, therefore the default. It does not
impose added overhead and keeps the previous behavior on all unchanged
code.
Tschuess,
Thomas Fehr
--
Thomas Fehr, SuSE Linux Products GmbH, Maxfeldstr. 5, 90409 Nuernberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)
Tel: +49-911-74053-0, Fax: +49-911-74053-482, Email: [email protected]
GPG public key available.
--
To unsubscribe, e-mail: [email protected]
To contact the owner, e-mail: [email protected]
