Re: [mdb-dev] mdb-sql: Readline Completion Support

[Michael W. <mw...@it...>]

Hi

Since I am basically just a lurker on this list, feel free to ignore
these comments:

1.)  It's generally easier to review patches if each patch does one
thing.  i.e. you should keep your case insensitive SQL patch separate
from your completion patch and attach both to your message if necessary.

2.)  Gratuitous whitespace changes get in the way of what your patch is
doing.  The reviewer has to waste time looking through the whitespace
changes to see if you made any other changes in that section besides
just the whitespace changes.

[snip]
>  int yywrap()
>  {
> -	return 1;
> +  return 1;

e.g. here.  There's no functional change.  This is just noise.

>  void yyerror(char *s)
>  {
> -	fprintf(stderr,"Error at Line : %s near %s\n", s, yytext);
> +  fprintf(stderr,"Error at Line : %s near %s\n", s, yytext);

And here too.

>  #if 0
>  int main(int argc, char **argv)
>  {
>  int i;
>  
> -	g_sql = mdb_sql_init();
> -	yyin = stdin;
> -	if (yyparse()) {
> -		fprintf(stderr, "Couldn't parse SQL\n");
> -		exit(1);
> -	}
> -	mdb_sql_dump(g_sql);
> -	mdb_sql_exit(g_sql);	
> +  g_sql = mdb_sql_init();
> +  yyin = stdin;
> +  if (yyparse()) {
> +    fprintf(stderr, "Couldn't parse SQL\n");
> +    exit(1);
> +  }
> +  mdb_sql_dump(g_sql);
> +  mdb_sql_exit(g_sql);  
[snip]

And the same here.

-- 
Michael Wood <mw...@it...>