Re: Readline patch

[Yoshinori K. Okuji]

Hi Marco,

On Saturday 29 November 2003 21:42, Marco Gerards wrote:
> Included with this mail is a patch to add basic readline like
> functionality.  This patch adds tab completion and a history buffer.

It's nice.

> 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. :)

> 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.

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?"

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.

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.

Okuji