Re: Readline patch

[Marco Gerards]

Hi,

Thanks for the quick reply!

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

> > Tab completion works, for the user, just like tab completion on GRUB.
> > I have added one nice feature so not only the partition type will be
> > shown when completing partitions but also the partition name (Jeff
> > Bailey suggested this feature).
> 
> Then, please add a credit for Jeff into somewhere. :)

Sure. :)

> > There was a small problem with my last patch.  For some reason
> > ncurses doesn't correctly report the backspace key.  I have included
> > the fix in this patch.
> 
> That's right. GRUB has the same workaround.

Oh, really?  I didn't know.  I will have a look at how this works in
GRUB to see if it is fixed in a better way...

> BTW, this is not serious, but:
> 
> > +  int __attribute__ ((__regparm__ (2)))
> > +    add_completion (const char *comp, const char *match, const char
> > *what, +                void (*print_completion) (char *))
> > +    {
> 
> I think this code would suprise those who don't know the background of 
> PUPA very much. They would say, "What is this strange attribute at 
> all?"

You are right, I will add some comments.

> And, this code is even bad for other architectures, as regparm is only 
> supported by i386 and x86-64. Also, you don't need this workaround, if 
> you use a good version of GCC.

Right.

> Therefore, this would be better:
> 
> 1. Define a macro, like NESTED_FUNC_ATTR.
> 2. Set the macro to "__attribute__ ((__regparm__ (2)))", only if regparm 
> is supported by GCC and GCC is buggy. Otherwise, set it to an empty 
> value.
> 
> I think the 2nd step could be done in the configure script, although I'm 
> not sure how to check if GCC is buggy.

I agree.  I think I can check this with a test program.

These changes are easy to make and I will (hopefully) send in a
revised patch on Tuesday.

Thanks,
Marco