grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Scripting support (PATCH)


From: Marco Gerards
Subject: Re: Scripting support (PATCH)
Date: Sun, 30 Oct 2005 13:17:13 +0100
User-agent: Gnus/5.1007 (Gnus v5.10.7) Emacs/21.4 (gnu/linux)

"Yoshinori K. Okuji" <address@hidden> writes:

> On Sunday 30 October 2005 10:45 am, Marco Gerards wrote:
>> Right, I felt it hard to explain the direction I'd like to take on
>> this list.  So I have implemented the core functionality because that
>> definitely has to be right.
>
> I don't know what direction you would like yet. It is hard to figure out all 
> of your ideas from the implementation. I only know that it is differently 
> implemented.

Sure, but I have explained it in detail in the emails I sent to the
list a week ago.  My most important point was that I did not want to
use pcode.

>> > - What are disadvantages in Vladimir's implementation?
>>
>> In my opinion it is not clean enough and does not integrate with GRUB
>> like I would like it to.  And Vladimir's patch was way too hard to
>> maintain when I started working on this a week ago.
>
> Can you elaborate on those points? I still don't understand.

That was about the previous patch Vladimir wrote.  His current patch
is based on my code, AFAICS.

I had a review of his previous ( > 1 week ago) code, and had some
problems with it:

- It was very hard to understand.
- It was all handwritten (later bison was used, which is a good
  thing).
- Poorly documented.
- Little reuse or adoption of existing GRUB code.
- No well known design and execution model.  I just based my design on
  the model bash uses.

>> > - What are disadvantages in Marco's implementation?
>> >
>> > Fewer features are implemented. The kernel is bloated.
>>
>> What do you mean with the kernel is bloated?  My patch moves a lot of
>> responsibility to commands.  I would say it is less bloated.
>
> I mentioned this, because your parser is built into the kernel. This is 
> unnecessary, as the rescue mode does not need fancy features.

It is build into normal mode.  And even much is not put in normal
mode, but in commands like `['.

The thing you see in the kernel is a replacement for the command to
split lines into arguments.  The old code for this was removed.  So
some code was removed and some was added, the end result is about the
same, but more reusable code.

--
Marco





reply via email to

[Prev in Thread] Current Thread [Next in Thread]