about i#123: "rework referencing variables in filters"

[Vlad Skvortsov]

Ivan Glushkov wrote:
> I've just commited some changes to fix i#123 "rework referencing
> variables in filters" (r1486 - r1491)
>
> Some things that confuse me:
> 1.
>    a. Before changes: if someone calls globals.get_username(opts, db)
> and current user not in db.cfg.users, function prints error message
> and make sys.exit(1)
>    b. After changes: the same behaviour if you specify the second
> parameter of get_username() (it is now not a Database object, but a
> list of database users). If it isn't specified (None by default),
> function doesn't check something and doesn't exit with 1 if user is
> not from 'db.cfg.users' list. This check is non-obvious, and i don't
> like this.
>
> The second variant is used when we get username for command 'list'. In
> this case we don't need the user to be the correct database user (we
> just want to list issues, don't change smth)
>   

I've addressed this in my comments on r1486.

> 2.
>   filters now reference to variables only with the delimiter "DT:".
> Script doesn't check existence of old delimiter "$".
>   

Same here. Generally, -1 on that.

> 3.
>   I "used string.Template.safe_substitute()" instead of
> "string.Template.substitute()" to avoid errors if some variables will
> stay unreferenced in filters. It is right decision from one side: i
> will be able to use ditrack even if someone wrote incorrect filter.
>
> But from the other side we should check correctness of all our
> filters, shouldn't we? (or we might just print  warnings ?)
>   

Yes, I'd by any means prefer to be issued warning at the least.

-- 
Vlad Skvortsov, vss at 73rus.com, http://vss.73rus.com