lkml.org 
[lkml]   [2012]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v4]menuconfig: Add Save button
Date
Wang, All,

On Saturday 01 December 2012 Wang YanQing wrote:
> If menuconfig have a Save button like alternative
> .config editors, xconfig, nconfig, etc.We will have
> a obvious benefit when use menuconfig just like
> when we use others, we can save our .config quickly
> and conveniently.
>
> This patch add the Save button for menuconfig.

To make the behavior consistent, may I suggest that you push the
concept even a bit further:
- add a "Load" *and* a "Save" buttons
- get rid of the top-level "Load an Alternate Configuration File"
and "Save an Alternate Configuration File" entries

For the "Save" and "Load" buttons, please mimick how the nconf frontend
behaves: prompt for the file to save to/load from, and prefill it with
the last filename that was entered (".config" per default).

Of course, two patches for that: one for the "Save/Load" buttons, one
to get rid of the top-level entries afterward.

Note: I do not state that your use-case is valid or not. That's your
call how you configure and test your kernel. However, I think that
having a consistent behavior across the frontends is a very nice goal
to pursue.

See other comments in-lined, below.

> V1-V2: Rewrite the most code to make it more correct
> V2-V3: Fix the behavior of conf_message_callback when exit.
> V3-V4:
> 1: Move Buttons a little left to make them look like symmetrical.
> 2: Exchange the position between Save button and Exit button.
> 3: Use conf_save instead of conf_write to make we can save
> an alternate configuration file with Save button.
>
> Signed-off-by: Wang YanQing <udknight@gmail.com>
> ---
> scripts/kconfig/lxdialog/menubox.c | 21 +++++++++---------
> scripts/kconfig/mconf.c | 45 ++++++++++++++++++++++++++++++--------
> 2 files changed, 47 insertions(+), 19 deletions(-)
>
> diff --git a/scripts/kconfig/lxdialog/menubox.c b/scripts/kconfig/lxdialog/menubox.c
> index 1d60473..b445bda 100644
> --- a/scripts/kconfig/lxdialog/menubox.c
> +++ b/scripts/kconfig/lxdialog/menubox.c
> @@ -26,7 +26,7 @@
> *
> * *) A bugfix for the Page-Down problem
> *
> - * *) Formerly when I used Page Down and Page Up, the cursor would be set
> + * *) Formerly when I used Page Down and Page Up, the cursor would be set

Space-damage. Don't send trailing-space fixes, or at least add a quick
note for it at the end of the commit log (eg.: "remove trailing space
while at it").

> * to the first position in the menu box. Now lxdialog is a bit
> * smarter and works more like other menu systems (just have a look at
> * it).
> @@ -154,12 +154,13 @@ static void print_arrows(WINDOW * win, int item_no, int scroll, int y, int x,
> */
> static void print_buttons(WINDOW * win, int height, int width, int selected)
> {
> - int x = width / 2 - 16;
> + int x = width / 2 - 20;
> int y = height - 2;
>
> print_button(win, gettext("Select"), y, x, selected == 0);
> - print_button(win, gettext(" Exit "), y, x + 12, selected == 1);
> + print_button(win, gettext(" Save "), y, x + 12, selected == 1);
> print_button(win, gettext(" Help "), y, x + 24, selected == 2);
> + print_button(win, gettext(" Exit "), y, x + 36, selected == 3);

The re-ordering should be in a separate patch.
It makes it difficult to separate the code for the new feature from
the churn due to the re-ordering.

Regards,
Yann E. MORIN.

--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'


\
 
 \ /
  Last update: 2012-12-02 19:21    [W:0.037 / U:0.416 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site