Re: [PATCH] Bug fix for parser

[Bean]

> > if grub_error is set, it will effect the parser, caused menu not to be
> > showed, etc.
>
> Right, but do you want to see a menu if not everything is correctly executed?

the problem is that it's not always an error when grub_errno is set,
for example, the test command use it to report whether or not two
strings equals.

> >> I do not like the name "script_1".
> >
> > what do you suggest ?
>
> Hopefully someone else knows a better name? :-)

how about script_init for the old script, and script here.

> >> >  commandblock:        '{'
> >> >                 {
> >> >                   grub_script_lexer_ref (state->lexerstate);
> >> > -                    grub_script_lexer_record_start (state->lexerstate);
> >>
> >> Ehm?  Can you explain this?  If I am not mistaken, this will result in
> >> a memory leak.
> >
> > I split commandblock from menuentry, now it's a standalone statement.
>
> Ah ok, so there is a grub_script_lexer_record_start elsewhere that
> forfills the same role?

yes, the grub_script_lexer_record_start is in menuentry.

> >> What was the problem here?
> >
> > I don't like the idea of deref in another statement, and now we can
> > use {} separately.
>
> Of deref?  Can you explain?  The idea of commandblock was that this
> structure might show up more often.  But I have no objections for now
> if a bug is fixed this way.

in the old implementation,

commandblock:   '{'
                  {
                    grub_script_lexer_ref (state->lexerstate);
                  }
                newlines commands '}'
                  {
                    grub_script_lexer_deref (state->lexerstate);
                    grub_script_lexer_record_start (state->lexerstate);
                    $$ = $4;
                  }
;

menuentry:      "menuentry" argument newlines commandblock
                  {
                    char *menu_entry;
                    menu_entry = grub_script_lexer_record_stop 
(state->lexerstate);
                    $$ = grub_script_create_cmdmenu (state, $2, menu_entry, 0);
                  }

it calls grub_script_lexer_record_start in commandblock, and
grub_script_lexer_record_stop in menuentry, but now it's both inside
menuentry. i think it's better this way. besides, if you use
commandblock alone in old implemenation, it will cause problem becuase
there is no matching grub_script_lexer_record_stop.

>
> Btw, does this patch incorporate the previous patch you sent in
> regarding scripting?

I didn't include the fix on the lexer, for example. ${AA}${BB} should
expand  to one token instead of two. I think it's better to use a
separate patch.

-- 
Bean