Bean <[EMAIL PROTECTED]> writes:
>> > 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.
Agreed.
>> >> 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.
Fine for me. Besides that, we can always change this and in the
meanwhile these names will do.
>> >> > 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.
Good :-)
>> >> 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.
For now your approach is ok. But perhaps I will reintroduce this
properly some day. We'll see :-)
>> 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.
Right. I am completely unhappy with the lexer as it is now... :-/
The main reason for writing it manually is that flex depends on all
kinds of stuff I do not want in GRUB. Like reading files, etc.
Thanks a lot for fixing bugs related to scripting. Lately I am rather
busy and I was the only person who worked on it. More testing, better
error handling, etc is appriciated.
I still have an unfinished "test" command on my HD.
--
Marco
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
