[PATCH] Split of normal mode (version 2)

[PATCH] Split of normal mode (version 2)

[Bean]

[-- Attachment #1: Type: text/plain, Size: 497 bytes --]

Hi,

This new patch make some changes based on the discussion of previous patch.

1, Move script engine to script/sh (sh.mod)
2, Move generic menu code to menu (menu.mod)
3, Move text menu viewer to menu/text (textmenu.mod)
4, Move misc function to lib (misc.mod)
5, Move setjmp to lib (setjmp.mod)

Now normal.mod only contains the reader code. To configure script
engine and viewer, you should add these lines at the beginning of
grub.cfg:

insmod sh
handler parser sh
insmod textmenu

-- 
Bean

[-- Attachment #2: split_2.zip --]
[-- Type: application/zip, Size: 41561 bytes --]

Re: [PATCH] Split of normal mode (version 2)

[Yoshinori K. Okuji]

On Tuesday 31 March 2009 02:41:14 Bean wrote:
> Hi,
>
> This new patch make some changes based on the discussion of previous patch.
>
> 1, Move script engine to script/sh (sh.mod)
> 2, Move generic menu code to menu (menu.mod)
> 3, Move text menu viewer to menu/text (textmenu.mod)
> 4, Move misc function to lib (misc.mod)
> 5, Move setjmp to lib (setjmp.mod)

I don't agree on the last two. Also, I don't like that you have just removed 
the rescue command.

> Now normal.mod only contains the reader code. To configure script
> engine and viewer, you should add these lines at the beginning of
> grub.cfg:
>
> insmod sh
> handler parser sh
> insmod textmenu

I prefer a more sophisticated approach (note: I hate manual loading).

For example, we can allow a config file to have a shebang, like "#!sh". If not 
specified, GRUB can assume that "sh" is used, and load it automatically. This 
kind of technique could even allow for using different languages in one 
setup.

For textmenu, I think it makes sense to have a command "textmenu". Just 
like "boot", GRUB can execute "textmenu" implicitly if a config file defines 
any menu entry but does not execute any menu command. This way, textmenu is 
automatically loaded.

Regards,
Okuji

Re: [PATCH] Split of normal mode (version 2)

[Bean]

On Sat, Apr 4, 2009 at 2:40 AM, Yoshinori K. Okuji <okuji@enbug.org> wrote:
> On Tuesday 31 March 2009 02:41:14 Bean wrote:
>> Hi,
>>
>> This new patch make some changes based on the discussion of previous patch.
>>
>> 1, Move script engine to script/sh (sh.mod)
>> 2, Move generic menu code to menu (menu.mod)
>> 3, Move text menu viewer to menu/text (textmenu.mod)
>> 4, Move misc function to lib (misc.mod)
>> 5, Move setjmp to lib (setjmp.mod)
>
> I don't agree on the last two. Also, I don't like that you have just removed
> the rescue command.

Hi,

The reason for a separate misc.mod is that it contains pure function
and has no side effect, so other modules can use them freely. In the
other hand, normal.mod does a lot of tasks in the init function, it's
not a good place for common function. Besides, there are already
helper function in the lib directory, like crc and hexdump, it's not
piratical to put each one in a new module, we might just merge them
into one helper module.

The problem with setjmp is that it's platform dependent, so if we keep
it in normal.mod, we must define it in very platform rmk file. But now
setjmp is not used in normal.mod anymore, we could move it out, and
thus make normal.mod platform independent.

>> Now normal.mod only contains the reader code. To configure script
>> engine and viewer, you should add these lines at the beginning of
>> grub.cfg:
>>
>> insmod sh
>> handler parser sh
>> insmod textmenu
>
> I prefer a more sophisticated approach (note: I hate manual loading).
>
> For example, we can allow a config file to have a shebang, like "#!sh". If not
> specified, GRUB can assume that "sh" is used, and load it automatically. This
> kind of technique could even allow for using different languages in one
> setup.
>
> For textmenu, I think it makes sense to have a command "textmenu". Just
> like "boot", GRUB can execute "textmenu" implicitly if a config file defines
> any menu entry but does not execute any menu command. This way, textmenu is
> automatically loaded.

I'm thinking about using environment variable to set handler, for
example, we could set it like this:

set parser=sh
set menu=textmenu
set terminal_output=gfxterm

We could use variable hooks to load the modules on demand.

For textmenu, we could add some test in normal.mod. After the main
config file is parsed, if it still doesn't contain a menu viewer, we
load textmenu automatically.

-- 
Bean

Re: [PATCH] Split of normal mode (version 2)

[Yoshinori K. Okuji]

On Saturday 04 April 2009 04:49:36 Bean wrote:
> On Sat, Apr 4, 2009 at 2:40 AM, Yoshinori K. Okuji <okuji@enbug.org> wrote:
> > On Tuesday 31 March 2009 02:41:14 Bean wrote:
> >> Hi,
> >>
> >> This new patch make some changes based on the discussion of previous
> >> patch.
> >>
> >> 1, Move script engine to script/sh (sh.mod)
> >> 2, Move generic menu code to menu (menu.mod)
> >> 3, Move text menu viewer to menu/text (textmenu.mod)
> >> 4, Move misc function to lib (misc.mod)
> >> 5, Move setjmp to lib (setjmp.mod)
> >
> > I don't agree on the last two. Also, I don't like that you have just
> > removed the rescue command.
>
> Hi,
>
> The reason for a separate misc.mod is that it contains pure function
> and has no side effect, so other modules can use them freely. In the
> other hand, normal.mod does a lot of tasks in the init function, it's
> not a good place for common function. Besides, there are already
> helper function in the lib directory, like crc and hexdump, it's not
> piratical to put each one in a new module, we might just merge them
> into one helper module.

We can just put them in the normal.mod. What is wrong? Frankly, your argument 
reminds me of the old discussion about monolithic vs. micro kernels...

> The problem with setjmp is that it's platform dependent, so if we keep
> it in normal.mod, we must define it in very platform rmk file. But now
> setjmp is not used in normal.mod anymore, we could move it out, and
> thus make normal.mod platform independent.

setjmp is required for the switch between rescue mode and normal mode. Don't 
remove it. "Hey, this is easier to maintain" is not a good reason to drop an 
useful feature. If your intention is to reduce the maintenance cost, you can 
simply define a variable for common files in common.rmk and use it in each 
machine-specific rmk file.

> >> Now normal.mod only contains the reader code. To configure script
> >> engine and viewer, you should add these lines at the beginning of
> >> grub.cfg:
> >>
> >> insmod sh
> >> handler parser sh
> >> insmod textmenu
> >
> > I prefer a more sophisticated approach (note: I hate manual loading).
> >
> > For example, we can allow a config file to have a shebang, like "#!sh".
> > If not specified, GRUB can assume that "sh" is used, and load it
> > automatically. This kind of technique could even allow for using
> > different languages in one setup.
> >
> > For textmenu, I think it makes sense to have a command "textmenu". Just
> > like "boot", GRUB can execute "textmenu" implicitly if a config file
> > defines any menu entry but does not execute any menu command. This way,
> > textmenu is automatically loaded.
>
> I'm thinking about using environment variable to set handler, for
> example, we could set it like this:
>
> set parser=sh
> set menu=textmenu
> set terminal_output=gfxterm
>
> We could use variable hooks to load the modules on demand.
>
> For textmenu, we could add some test in normal.mod. After the main
> config file is parsed, if it still doesn't contain a menu viewer, we
> load textmenu automatically.

I agree that terminal_output (and so terminal_input as well) can be 
implemented as environment variables, but it is very questionable whether a 
parser or a menu interface can be considered as an environment variable. An 
important question is how you would determine the name of a module from the 
name of a parser or a menu interface.

Regards,
Okuji

Re: [PATCH] Split of normal mode (version 2)

[phcoder]

> We can just put them in the normal.mod. What is wrong? Frankly, your argument 
> reminds me of the old discussion about monolithic vs. micro kernels...
Yes, it's quite similar and IMO it's better to have more small modules. 
It also forces to code in an abstratc way which simplifies the maintaining
> setjmp is required for the switch between rescue mode and normal mode. 

It isn't. You can just call the corresponding function. What's wrong 
with such approach?


-- 

Regards
Vladimir 'phcoder' Serbinenko

Re: [PATCH] Split of normal mode (version 2)

[Colin D Bennett]

[-- Attachment #1: Type: text/plain, Size: 1963 bytes --]

On Sat, 04 Apr 2009 00:19:46 +0200
phcoder <phcoder@gmail.com> wrote:

> > setjmp is required for the switch between rescue mode and normal mode. 
> 
> It isn't. You can just call the corresponding function. What's wrong 
> with such approach?

So you could have something like

-------------
void grub_main ()
{
  //...
  // Try to start up in normal mode
  normal ();
}

void normal ()
{
  // do stuff
  // User asked for rescue mode:
  rescue ();
}

void rescue ()
{
  // do stuff
  // User asked for normal mode:
  normal ();
}
-------------

What if you switch back and forth between normal and rescue mode
many times in a row?  The stack will grow with each call and eventually
the stack will overflow and Bad Things will happen.

Granted, you'd have to switch many times to overflow the stack, but it
is a sub-optimal situation.  You could have something like:

-------------
// In this context, grub_main acts as a dispatcher
// so that normal and rescue mode can be switched without
// unbounded growing of the stack.

enum mode { RESCUE, NORMAL };

void
grub_main ()
{
  enum mode next_mode;
  //...

  // Try to start up in normal mode
  next_mode = NORMAL;
  while (1)
  {
    switch (next_mode)
    {
      case RESCUE:
        next_mode = rescue ();
        break;
      case NORMAL:
        next_mode = normal ();
        break;
      default:
        // Panic!
    }
  }
}

enum mode
normal ()
{
  // do stuff
  // User asked for rescue mode:
  return RESCUE;
}

enum mode
rescue ()
{
  // do stuff
  // User asked for normal mode:
  return NORMAL;
}
-------------


Now you could also return function pointers instead of using
switch/case with enum constants, but it's the same concept.  Then
setjmp is another similar way to do it without implementing a central
dispatcher like grub_main above.

At least that's how I think of it.

Regards,
Colin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] Split of normal mode (version 2)

[Yoshinori K. Okuji]

On Saturday 04 April 2009 08:02:40 Colin D Bennett wrote:
> What if you switch back and forth between normal and rescue mode
> many times in a row?  The stack will grow with each call and eventually
> the stack will overflow and Bad Things will happen.

Yes.

> Now you could also return function pointers instead of using
> switch/case with enum constants, but it's the same concept.  Then
> setjmp is another similar way to do it without implementing a central
> dispatcher like grub_main above.

Exactly. For those who know the concept of continuation, setjmp/longjmp is 
very handy. This makes some kind of programs to be written easily and simply.

Regards,
Okuji

Re: [PATCH] Split of normal mode (version 2)

[Bean]

On Sat, Apr 4, 2009 at 4:12 AM, Yoshinori K. Okuji <okuji@enbug.org> wrote:
> On Saturday 04 April 2009 04:49:36 Bean wrote:
>> On Sat, Apr 4, 2009 at 2:40 AM, Yoshinori K. Okuji <okuji@enbug.org> wrote:
>> > On Tuesday 31 March 2009 02:41:14 Bean wrote:
>> >> Hi,
>> >>
>> >> This new patch make some changes based on the discussion of previous
>> >> patch.
>> >>
>> >> 1, Move script engine to script/sh (sh.mod)
>> >> 2, Move generic menu code to menu (menu.mod)
>> >> 3, Move text menu viewer to menu/text (textmenu.mod)
>> >> 4, Move misc function to lib (misc.mod)
>> >> 5, Move setjmp to lib (setjmp.mod)
>> >
>> > I don't agree on the last two. Also, I don't like that you have just
>> > removed the rescue command.
>>
>> Hi,
>>
>> The reason for a separate misc.mod is that it contains pure function
>> and has no side effect, so other modules can use them freely. In the
>> other hand, normal.mod does a lot of tasks in the init function, it's
>> not a good place for common function. Besides, there are already
>> helper function in the lib directory, like crc and hexdump, it's not
>> piratical to put each one in a new module, we might just merge them
>> into one helper module.
>
> We can just put them in the normal.mod. What is wrong? Frankly, your argument
> reminds me of the old discussion about monolithic vs. micro kernels...
>

One of the problem for normal.mod dependency is its side effect. For
example, currently ls.mod depend on normal.mod just for
grub_normal_print_device_info. If we want to embed ls.mod in core.img,
we must embedded normal.mod as well, along with a lot of
initialization actions that we may not want.

>> The problem with setjmp is that it's platform dependent, so if we keep
>> it in normal.mod, we must define it in very platform rmk file. But now
>> setjmp is not used in normal.mod anymore, we could move it out, and
>> thus make normal.mod platform independent.
>
> setjmp is required for the switch between rescue mode and normal mode. Don't
> remove it. "Hey, this is easier to maintain" is not a good reason to drop an
> useful feature. If your intention is to reduce the maintenance cost, you can
> simply define a variable for common files in common.rmk and use it in each
> machine-specific rmk file.
>

No, setjmp is not required for the switch between rescue mode and
normal mode anymore. In fact, to enter rescue mode, you use:

handler reader rescue

To switch back to normal mode:

handler reader normal

It is not recursive, so no need for setjmp.

>> >> Now normal.mod only contains the reader code. To configure script
>> >> engine and viewer, you should add these lines at the beginning of
>> >> grub.cfg:
>> >>
>> >> insmod sh
>> >> handler parser sh
>> >> insmod textmenu
>> >
>> > I prefer a more sophisticated approach (note: I hate manual loading).
>> >
>> > For example, we can allow a config file to have a shebang, like "#!sh".
>> > If not specified, GRUB can assume that "sh" is used, and load it
>> > automatically. This kind of technique could even allow for using
>> > different languages in one setup.
>> >
>> > For textmenu, I think it makes sense to have a command "textmenu". Just
>> > like "boot", GRUB can execute "textmenu" implicitly if a config file
>> > defines any menu entry but does not execute any menu command. This way,
>> > textmenu is automatically loaded.
>>
>> I'm thinking about using environment variable to set handler, for
>> example, we could set it like this:
>>
>> set parser=sh
>> set menu=textmenu
>> set terminal_output=gfxterm
>>
>> We could use variable hooks to load the modules on demand.
>>
>> For textmenu, we could add some test in normal.mod. After the main
>> config file is parsed, if it still doesn't contain a menu viewer, we
>> load textmenu automatically.
>
> I agree that terminal_output (and so terminal_input as well) can be
> implemented as environment variables, but it is very questionable whether a
> parser or a menu interface can be considered as an environment variable. An
> important question is how you would determine the name of a module from the
> name of a parser or a menu interface.

We can generate a handler.lst by scanning source for
grub_parser_register, etc, just like commands.lst:

parser.sh: sh
reader.normal: normal
menu.textmenu: textmenu
terminal_outout.gfxterm: gfxterm

We can then read it in normal.mod and set the hooks for all handler classes.

-- 
Bean

Re: [PATCH] Split of normal mode (version 2)

[Yoshinori K. Okuji]

On Saturday 04 April 2009 14:06:18 Bean wrote:
> One of the problem for normal.mod dependency is its side effect. For
> example, currently ls.mod depend on normal.mod just for
> grub_normal_print_device_info. If we want to embed ls.mod in core.img,
> we must embedded normal.mod as well, along with a lot of
> initialization actions that we may not want.

Right, if we can completely merge the ls on rescue mode and that on normal 
mode. For now, I am not sure if this is feasible, so I prefer to keep a poor 
version of ls for rescue mode, which does not require functions in 
normal.mod, until you prove that that is feasible.

> We can generate a handler.lst by scanning source for
> grub_parser_register, etc, just like commands.lst:
>
> parser.sh: sh
> reader.normal: normal
> menu.textmenu: textmenu
> terminal_outout.gfxterm: gfxterm
>
> We can then read it in normal.mod and set the hooks for all handler
> classes.

I like if you can merge all kinds of handlers this way, thus eliminating the 
command.lst and the fs.lst.

But I still think that having commands for parsers and menus, since you can 
do:

grub> sh /boot/grub/foo.sh
grub> textmenu
grub> gfxmenu

They won't be very useful for most people, but could be useful for 
development.

Regards,
Okuji

Re: [PATCH] Split of normal mode (version 2)

[Bean]

On Sun, Apr 5, 2009 at 10:33 PM, Yoshinori K. Okuji <okuji@enbug.org> wrote:
> On Saturday 04 April 2009 14:06:18 Bean wrote:
>> One of the problem for normal.mod dependency is its side effect. For
>> example, currently ls.mod depend on normal.mod just for
>> grub_normal_print_device_info. If we want to embed ls.mod in core.img,
>> we must embedded normal.mod as well, along with a lot of
>> initialization actions that we may not want.
>
> Right, if we can completely merge the ls on rescue mode and that on normal
> mode. For now, I am not sure if this is feasible, so I prefer to keep a poor
> version of ls for rescue mode, which does not require functions in
> normal.mod, until you prove that that is feasible.
>

Hi,

Currently, this is implemented using priority list. The commands has a
field that indicate its priority. For normal command, the value is 0,
for extended command, it's 1. So if two version of ls is loaded, the
extended ls would be used. All commands are in a unify set, so you can
run normal commads/extended commands in either rescue or normal mode.

>> We can generate a handler.lst by scanning source for
>> grub_parser_register, etc, just like commands.lst:
>>
>> parser.sh: sh
>> reader.normal: normal
>> menu.textmenu: textmenu
>> terminal_outout.gfxterm: gfxterm
>>
>> We can then read it in normal.mod and set the hooks for all handler
>> classes.
>
> I like if you can merge all kinds of handlers this way, thus eliminating the
> command.lst and the fs.lst.
>
> But I still think that having commands for parsers and menus, since you can
> do:
>
> grub> sh /boot/grub/foo.sh
> grub> textmenu
> grub> gfxmenu
>
> They won't be very useful for most people, but could be useful for
> development.

Yeah, that's possible. We can register a generic command to do the task.

-- 
Bean

Re: [PATCH] Split of normal mode (version 2)

[Yoshinori K. Okuji]

On Monday 06 April 2009 00:02:59 Bean wrote:
> On Sun, Apr 5, 2009 at 10:33 PM, Yoshinori K. Okuji <okuji@enbug.org> wrote:
> > On Saturday 04 April 2009 14:06:18 Bean wrote:
> >> One of the problem for normal.mod dependency is its side effect. For
> >> example, currently ls.mod depend on normal.mod just for
> >> grub_normal_print_device_info. If we want to embed ls.mod in core.img,
> >> we must embedded normal.mod as well, along with a lot of
> >> initialization actions that we may not want.
> >
> > Right, if we can completely merge the ls on rescue mode and that on
> > normal mode. For now, I am not sure if this is feasible, so I prefer to
> > keep a poor version of ls for rescue mode, which does not require
> > functions in normal.mod, until you prove that that is feasible.
>
> Hi,
>
> Currently, this is implemented using priority list. The commands has a
> field that indicate its priority. For normal command, the value is 0,
> for extended command, it's 1. So if two version of ls is loaded, the
> extended ls would be used. All commands are in a unify set, so you can
> run normal commads/extended commands in either rescue or normal mode.

Great. As long as we can use a smaller (so more stupid) version of ls, I have 
no objection.

Regards,
Okuji

Re: [PATCH] Split of normal mode (version 2)

[Bean]

[-- Attachment #1: Type: text/plain, Size: 864 bytes --]

Hi,

This is another update of the patch.

1, Now completion.c is in menu.mod, and menu_viewer.c is in misc.mod,
the reason for the switch is to allow configfile to depend on misc.mod
only.
2, Auto generate handler.lst file, which contain module information
for handlers. normal.mod uses it to register commands to set active
handler, for example:

parser.sh
menu_viewer.text
terminal_output.gfxterm

3, configfile now support an optional parameter to specify the script
engine, for example:

configfile /aa.cfg sh

When configfile returns, the script engine would be restored to the
previous value. This is useful for switching script engine. For
example, you can parse a file in another language, then switch back to
sh for the rest of grub.cfg.

4, normal.mod set the default parser and menu viewer before parsing grub.cfg:
parser.sh
menu_viewer.text

-- 
Bean

[-- Attachment #2: split_3.zip --]
[-- Type: application/zip, Size: 72730 bytes --]

Re: [PATCH] Split of normal mode (version 2)

[Yoshinori K. Okuji]

On Tuesday 07 April 2009 01:39:23 Bean wrote:
> Hi,
>
> This is another update of the patch.

Thank you. I will review your patch and send comments tonight.

Regards,
Okuji

> 1, Now completion.c is in menu.mod, and menu_viewer.c is in misc.mod,
> the reason for the switch is to allow configfile to depend on misc.mod
> only.
> 2, Auto generate handler.lst file, which contain module information
> for handlers. normal.mod uses it to register commands to set active
> handler, for example:
>
> parser.sh
> menu_viewer.text
> terminal_output.gfxterm
>
> 3, configfile now support an optional parameter to specify the script
> engine, for example:
>
> configfile /aa.cfg sh
>
> When configfile returns, the script engine would be restored to the
> previous value. This is useful for switching script engine. For
> example, you can parse a file in another language, then switch back to
> sh for the rest of grub.cfg.
>
> 4, normal.mod set the default parser and menu viewer before parsing
> grub.cfg: parser.sh
> menu_viewer.text

Re: [PATCH] Split of normal mode (version 2)

[Yoshinori K. Okuji]

On Tuesday 07 April 2009 01:39:23 Bean wrote:
> Hi,
>
> This is another update of the patch.
>
> 1, Now completion.c is in menu.mod, and menu_viewer.c is in misc.mod,
> the reason for the switch is to allow configfile to depend on misc.mod
> only.

I think the name "misc.mod" is ugly. Can you think about a better name?

> 2, Auto generate handler.lst file, which contain module information
> for handlers. normal.mod uses it to register commands to set active
> handler, for example:
>
> parser.sh
> menu_viewer.text
> terminal_output.gfxterm

Great.

> 3, configfile now support an optional parameter to specify the script
> engine, for example:
>
> configfile /aa.cfg sh
>
> When configfile returns, the script engine would be restored to the
> previous value. This is useful for switching script engine. For
> example, you can parse a file in another language, then switch back to
> sh for the rest of grub.cfg.

I object to the syntax, but not to the idea. "configfile" is GRUB-specific, so 
it might be acceptable. But IIRC the underlying function is shared 
with "source", right? In Bourne Shell, "source FILE ARG" means that the file 
FILE is executed with a positional argument ARG. So it is not intuitive to 
specify a parser this way.

I proposed using a shebang some days ago. Was it so bad?

> 4, normal.mod set the default parser and menu viewer before parsing
> grub.cfg: parser.sh
> menu_viewer.text


Regards,
Okuji

Re: [PATCH] Split of normal mode (version 2)

[Bean]

On Fri, Apr 10, 2009 at 7:49 AM, Yoshinori K. Okuji <okuji@enbug.org> wrote:
> On Tuesday 07 April 2009 01:39:23 Bean wrote:
>> Hi,
>>
>> This is another update of the patch.
>>
>> 1, Now completion.c is in menu.mod, and menu_viewer.c is in misc.mod,
>> the reason for the switch is to allow configfile to depend on misc.mod
>> only.
>
> I think the name "misc.mod" is ugly. Can you think about a better name?
>

Hi,

Perhaps lib.mod, or helper.mod ?

>> 2, Auto generate handler.lst file, which contain module information
>> for handlers. normal.mod uses it to register commands to set active
>> handler, for example:
>>
>> parser.sh
>> menu_viewer.text
>> terminal_output.gfxterm
>
> Great.
>
>> 3, configfile now support an optional parameter to specify the script
>> engine, for example:
>>
>> configfile /aa.cfg sh
>>
>> When configfile returns, the script engine would be restored to the
>> previous value. This is useful for switching script engine. For
>> example, you can parse a file in another language, then switch back to
>> sh for the rest of grub.cfg.
>
> I object to the syntax, but not to the idea. "configfile" is GRUB-specific, so
> it might be acceptable. But IIRC the underlying function is shared
> with "source", right? In Bourne Shell, "source FILE ARG" means that the file
> FILE is executed with a positional argument ARG. So it is not intuitive to
> specify a parser this way.

Yeah, I don't like the syntax either, but we need to find a way to
distinguish between the "configfile" and "source". "source" operates
on the current environment, while "configfile" create a new
environment.

>
> I proposed using a shebang some days ago. Was it so bad?

I like that idea too, but it needs to change grub_file_getline a
little bit as currently it treats lines starting with '#' as comment,
so we don't have a chance to examine the contents behind it.

Perhaps we can combine this with configfile/source. For example, when
it reads a file starts with #!, it automatically switch parser, and
restore back to the original parser on exit. This way, we don't need
to hack the configfile/source command at all.

-- 
Bean

Re: [PATCH] Split of normal mode (version 2)

[Bean]

[-- Attachment #1: Type: text/plain, Size: 473 bytes --]

Hi,

Another update for the patch:

sync with svn r2074
misc bug fixes
change build script for i386-efi, i386-coreboot, i386-ieee1275 and
x86_64-efi as well as i386-pc, grub-emu now builds properly for
i386-pc.
support the use of #! in the script file. It would switch to the
selected parser, read the file, then restore to the original parser
before returning.
configfile/source now back to previous syntax, as there is no need to
change parser in here anymore.

-- 
Bean

[-- Attachment #2: split_4.zip --]
[-- Type: application/zip, Size: 77975 bytes --]

Re: [PATCH] Split of normal mode (version 2)

[Yoshinori K. Okuji]

On Saturday 11 April 2009 05:17:43 Bean wrote:
> Hi,
>
> Another update for the patch:
>
> sync with svn r2074
> misc bug fixes
> change build script for i386-efi, i386-coreboot, i386-ieee1275 and
> x86_64-efi as well as i386-pc, grub-emu now builds properly for
> i386-pc.
> support the use of #! in the script file. It would switch to the
> selected parser, read the file, then restore to the original parser
> before returning.
> configfile/source now back to previous syntax, as there is no need to
> change parser in here anymore.

I am afraid that this patch is getting too big to review. Honestly, I would 
like you to check in some parts quickly, and the rest still requires more 
discussion. Anyway, I think it is a good practice to check in one thing at 
one time, so splitting the patch is a good thing. For example:

- handler unification -> one patch
- config embedding -> one patch
- parser separation -> one patch
- viewer separation -> one patch
- misc module -> one patch
- etc.

Due to the volume, I am not certain if I have really read everything. :(

Regards,
Okuji

Re: [PATCH] Split of normal mode (version 2)

[Bean]

On Sat, Apr 11, 2009 at 5:50 PM, Yoshinori K. Okuji <okuji@enbug.org> wrote:
> On Saturday 11 April 2009 05:17:43 Bean wrote:
>> Hi,
>>
>> Another update for the patch:
>>
>> sync with svn r2074
>> misc bug fixes
>> change build script for i386-efi, i386-coreboot, i386-ieee1275 and
>> x86_64-efi as well as i386-pc, grub-emu now builds properly for
>> i386-pc.
>> support the use of #! in the script file. It would switch to the
>> selected parser, read the file, then restore to the original parser
>> before returning.
>> configfile/source now back to previous syntax, as there is no need to
>> change parser in here anymore.
>
> I am afraid that this patch is getting too big to review. Honestly, I would
> like you to check in some parts quickly, and the rest still requires more
> discussion. Anyway, I think it is a good practice to check in one thing at
> one time, so splitting the patch is a good thing. For example:
>
> - handler unification -> one patch
> - config embedding -> one patch
> - parser separation -> one patch
> - viewer separation -> one patch
> - misc module -> one patch
> - etc.
>
> Due to the volume, I am not certain if I have really read everything. :(

Hi,

All right.

-- 
Bean