-
Notifications
You must be signed in to change notification settings - Fork 408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Run <if> value by ts_search:subst/2 #311
Conversation
I first thought about adding a flag to enable the substitution (like with Another option would be to a new/alternative attribute beside the literal "value". The current solution feels the most natural too me though. What do you think, @nniclausse? |
ping @nniclausse |
maybe we can add a subst flag during the xml parsing, if we detect the %%xxx%% pattern ? if we don't do that, every use of an 'if' will call ts_search:subst even if we don't need substitutions. |
I like that idea! I was first thinking about I'll try to give this a shot. |
Can you take another look, @nniclausse? I'm not sure if this approach is sufficient. I'm simply peeking into the value and if I see |
Could you take another look, @nniclausse? |
src/tsung_controller/ts_config.erl
Outdated
@@ -503,7 +503,13 @@ parse(_Element = #xmlElement{name='if', attributes=Attrs,content=Content}, | |||
NewConf = lists:foldl(fun parse/2, Conf#config{curid=Id+1}, Content), | |||
NewId = NewConf#config.curid, | |||
?LOGF("endif in session ~p as id ~p",[CurS#session.id,NewId+1],?INFO), | |||
InitialAction = {ctrl_struct, {if_start, Rel, VarName, list_to_binary(Value) , NewId+1}}, | |||
SubstitutionFlag = case string:str(Value, "%%") of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should use re:run(Value, "%%.+%%")
instead if think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. I thought string:str
because that's recommended for performance reasons, but since we are only parsing the configuration, that should not be a issue to begin with.
I'll change this.
Do you have anything else on the new approach?
This PR will run the
value
of<if>
byts_search:subst/2
to make it work with variables as well.For example, if you need to compare a variable against another variable that you've extracted from a previous request, you can now do that.
Example:
Fixes #168