[ditrack commit] r938 - src/trunk/DITrack

[Vlad Skvortsov]

I have a few comments (below).

oleg at ditrack.org wrote:
> Author: oleg
> Date: 2006-12-04 11:02:24 -0800 (Mon, 04 Dec 2006)
> New Revision: 938
>
> Modified:
>    src/trunk/DITrack/DB.py
> Log:
> Added handling of syntax error of a filter definition during the reading of filter config file
> Changed data type of the variable using for storage a filter conditions
>
>
> Modified: src/trunk/DITrack/DB.py
> ===================================================================
> --- src/trunk/DITrack/DB.py	2006-11-30 09:58:38 UTC (rev 937)
> +++ src/trunk/DITrack/DB.py	2006-12-04 19:02:24 UTC (rev 938)
> @@ -456,16 +456,18 @@
>  		    + s + "' definition")
>  
>  	    filter = filters[s].strip()
> -            
> +
>  	    if not filter:
>                  raise CorruptedDB_UnparseableFiltersError("Empty filter "
>  		    + s + "' definition")
> +	    try:
> +		self.items[s] = Filter(filter)
> +	    except (FilterIsPredefinedError, FilterExpressionError):
> +                raise CorruptedDB_UnparseableFiltersError("Syntax error in filter "
> +		    + s + "' definition")
>  
> -	    self.items[s] = Filter(filter)
> +    has_key = __contains__   
>  
> -    has_key = __contains__
> -
> -
>  class Filter:
>      def __init__(self, str):
>  	"Initialize filter by parsing the expression passed"
> @@ -480,18 +482,26 @@
>  	# Split into subclauses.
>  	clauses = filter(lambda x: len(x), str.split(","))
>  
> -	self.conditions = []
> -	condition_re = re.compile("^(.*[^!=])(!?=)(.*)$")
> +	self.conditions = {}
> +	condition_re = re.compile("^(?P<param>.*[^!=])(?P<condition>!?=)(?P<value>.*)$")
>   

I do understand that it wasn't introduced in this particular commit. 
However, it seems that the expression should look like

^(?P<param>[^!=]*)(?P<condition>!?=)(?P<value>.*)$

M?

>  
> +	i = 0
>  	for c in clauses:
>  	    m = condition_re.match(c)
>  	    if not m:
>  		raise FilterExpressionError(c)
>  
> +	    condition = m.groupdict()
> +
> +	    if len(condition) != 3:
> +		raise FilterExpressionError(c)
> +	    
>  	    # Perform substitutions.
> -	    value = self.substitute(m.group(3))
> +	    condition['value'] = self.substitute(condition['value'])
>  
> -	    self.conditions.append((m.group(1), m.group(2), value))
> +	    self.conditions[i] = condition
> +	    
> +	    i = i + 1
>   

Is there a particular reason for using a dictionary instead of plain array?
>  
>      def substitute(self, str):
>  	
> @@ -527,12 +537,12 @@
>  	for c in self.conditions:
>  	    assert(len(c) == 3)
>  
> -	    if c[1] == "=":
> -		if issue.info[c[0]] != c[2]: return None
> -	    elif c[1] == "!=":
> -		if issue.info[c[0]] == c[2]: return None
> +	    if c['condition'] == "=":
> +		if issue.info[c['param']] != c['value']: return None
> +	    elif c['condition'] == "!=":
> +		if issue.info[c['param']] == c['value']: return None
>  	    else:
> -		raise NotImplemented(c[1])
> +		raise NotImplemented(c['condition'])
>  
>  	return True
>  
>
> _______________________________________________
> Commit mailing list
> Commit at lists.ditrack.org
> http://lists.ditrack.org/mailman/listinfo/commit
>   


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