Re: [PATCH] wrap long help lines, take two

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

 



2009/12/15 Michal Marek <mmarek@xxxxxxx>:
> On 15.12.2009 07:46, Vadim Bendebury (вб) wrote:
>> Tested by running
>>
>> mkdir ../build/powerpc
>> ARCH=powerpc make menuconfig O=../build/powerpc
>
> Did you test this exact patch? :) I'm getting
>  HOSTCC  scripts/kconfig/zconf.tab.o

That's embarrassing, I was wondering if it would compile and then
forgot to check why it does not complain.. I never did anything with
git before, this is and missed the fact that the previous change was
lingering in my tree. This is taken care of now.

> In file included from scripts/kconfig/zconf.tab.c:2452:
> /home/mmarek/linux-2.6/scripts/kconfig/expr.c: In function
> ‘expr_print_gstr_helper’:
> /home/mmarek/linux-2.6/scripts/kconfig/expr.c:1103: warning: implicit
> declaration of function ‘getmaxx’
> /home/mmarek/linux-2.6/scripts/kconfig/expr.c:1103: error: ‘stdscr’
> undeclared (first use in this function)
> /home/mmarek/linux-2.6/scripts/kconfig/expr.c:1103: error: (Each
> undeclared identifier is reported only once
> /home/mmarek/linux-2.6/scripts/kconfig/expr.c:1103: error: for each
> function it appears in.)
> make[2]: *** [scripts/kconfig/zconf.tab.o] Error 1
>
> Anyway, it's better than the last version, but see comments below.
>
>> diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
>> index 2aa49e8..203eb6d 100644
>> --- a/scripts/kconfig/expr.c
>> +++ b/scripts/kconfig/expr.c
>> @@ -1099,9 +1099,25 @@ void expr_fprint(struct expr *e, FILE *out)
>>
>>  static void expr_print_gstr_helper(void *data, struct symbol *sym,
>> const char *str)
>>  {
>> -       str_append((struct gstr*)data, str);
>> +       struct gstr *gs = (struct gstr*)data;
>> +       unsigned extra_length = strlen(str) + (sym ? 4 : 0);
>> +       const char *last_cr = strrchr(gs->s, '\n');
>> +       unsigned screen_width = getmaxx(stdscr) - 10;
>> +       unsigned last_line_length;
>> +
>> +       if (!last_cr) {
>> +               last_cr = gs->s;
>> +       }
>> +
>> +       last_line_length = strlen(gs->s) - (last_cr - gs->s);
>
> You can avoid recomputing all this if you store the state in a struct
> that gets passed as the data argument:
>
> struct somename {
>  struct gstr *gstr;
>  int width;
>  /* other bookkeeping stuff like last_line_length */
> }
>
> Then expr_gstr_print would just initialize an instance of this structure
> and pass it to expr_print as the callback argument.

I first thought about it, but decided against it, because the way it
is now it is a much more contained change. Yes, recalculating string
length every time is excessive, but in this particular case it is
harmless.I am also worried of the string modified elsewhere bypassing
the print helper function, which would take things out of sync and
cause corrupted data.

 Please take another look at this, I can change it, but I think a
smaller modification has merits.

> Also, computing the
> desired width should be done in mconf and passed to expr_gstr_print as
> an argument. First, the other config programs (conf, qconf and gconf)
> should not depend on ncurses (they can pass a zero, meaning no
> wrapping), second the getmaxx(stdscr) - 10 logic really belongs to
> mconf. If we make use of this feature in nconf, then there the right
> number be getmaxx(stdscr) - 12 or whatever, depending on the exact layout.
>
good point, I'll take care of this.

cheers,
/vb


> Michal
>
>> +
>> +       if ((last_line_length + extra_length) > screen_width) {
>> +               str_append(gs, "\\\n");
>> +       }
>> +
>> +       str_append(gs, str);
>>         if (sym)
>> -               str_printf((struct gstr*)data, " [=%s]",
>> sym_get_string_value(sym));
>> +               str_printf(gs, " [=%s]", sym_get_string_value(sym));
>>  }
>>
>>  void expr_gstr_print(struct expr *e, struct gstr *gs)
>> --
>> 1.5.4.3
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux&nblp;USB Development]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite Secrets]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux