hoaproject/Ruler

Space or no-space in expressions

beberlei opened this issue · 10 comments

It is a difference to have foo="bar" or foo = "bar" when evaluating this grammar. Is there a way to have both the same?

Hywan commented

Hello :-),

I don't think there is any difference. Both should be supported.

@Hywan thants for the quick reply.

It renders different models var_dump((string)$model); from interpret():

$model->expression =
    $model->{'='}(
        $model->variable('method'),
        'POST'
    );
$model = new \Hoa\Ruler\Model();
$model->expression =
    $model->variable('method="POST"');

@Hywan i should mention, we are not using ruler to execute, but we are visiting the AST ourselves to convert it into Elasticsearch query.

Hywan commented

Indeed, I can reproduce the bug. I don't have right now to fix it, but probably on Friday. Is it working for you?

Hywan commented
$ echo 'foo = "bar"' | vendor/bin/hoa compiler:pp vendor/hoa/ruler/Grammar.pp 0 -s
 #  namespace     token name            token value                     offset
-------------------------------------------------------------------------------
 0  default       identifier            foo                                  0
 1  default       identifier            =                                    4
 2  default       string                "bar"                                6
 3  default       EOF                                                       12

$ echo 'foo="bar"' | vendor/bin/hoa compiler:pp vendor/hoa/ruler/Grammar.pp 0 -s
 #  namespace     token name            token value                     offset
-------------------------------------------------------------------------------
 0  default       identifier            foo="bar"                            0
 1  default       EOF                                                       10

I suppose I know how to fix that.

@Hywan don't worry or feel the need to commit to a time frame, i have a "workaround" for now :-)

Hywan commented

The trick is easy actually:

%token identifier [^\s\(\)\[\],\.]+

We must change the token to be:

- %token  identifier    [^\s\(\)\[\],\.]+
+ %token  identifier    [^\s\(\)\[\],\.=]+

We should include also common operators. That's because an identifier can be anything.

I tried to copy the grammar with the small change, but it leads to a new error:

Unrecognized token "=" at line 1 and column 9:
release = 1
Hywan commented

Sorry, had to fix other higher priority bugs. I'll come back to you as soon as possible :-).

Hywan commented

Sorry, had to fix other higher priority bugs. I'll come back to you as soon as possible :-).

Try with:

%token identifier %token  identifier    ((=|!=|>|>=|<|<=)|[^\s\(\)\[\],\.=!<>]+)

The problem is that both the operands and the operator are represented by the identifier token. To be the most flexible and free possible, the space is the main separator between the tokens. I never thought about your usecase, and yes, it can be annoying. The identifier is slightly modified to cut over =, !=, >, >=, <, and <=. I don't like being specific in this case, it looks like a hack, but I guess we have no choice.