Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757513AbZJSUM4 (ORCPT ); Mon, 19 Oct 2009 16:12:56 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1757458AbZJSUM4 (ORCPT ); Mon, 19 Oct 2009 16:12:56 -0400 Received: from pfepb.post.tele.dk ([195.41.46.236]:46043 "EHLO pfepb.post.tele.dk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757490AbZJSUMu (ORCPT ); Mon, 19 Oct 2009 16:12:50 -0400 Date: Mon, 19 Oct 2009 22:12:52 +0200 From: Sam Ravnborg To: Nir Tzachar Cc: linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org, zippel@linux-m68k.org Subject: Re: [PATCH] nconfig v4 Message-ID: <20091019201252.GA14977@merkur.ravnborg.org> References: <1255357820-7122-1-git-send-email-nir.tzachar@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1255357820-7122-1-git-send-email-nir.tzachar@gmail.com> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 68651 Lines: 2390 On Mon, Oct 12, 2009 at 04:30:20PM +0200, Nir Tzachar wrote: > Hello. > > This is a reworked version of nconfig, an ncurses based replacement for > menuconfig. The first submission was approximately a year ago. The current patch > is applicable to the current git tree. > > Below is the list of issues which were previously raised, and how > they were resolved. Furthermore, there are 2 open issues, which are not > party poppers. > > The main concern previously raised was the issue of colors. The current version > tries as much as possible to use colors only for highlighting specific texts and > the default terminal colors for everything else. This should be more > acceptable. > > Fixes: > 1) We now use as much of the default fore/back colors of the terminal as > possible (most of the text). Color is used only to highlight buttons, etc. > 2) nconf added to 'make help' > 3) All windows are automatically centered and expanded for better view. > 4) Cursor is seen correctly when using "screen". > 5) Instructions are hidden behind a popup window to maximize working area. > 6) nconf runs fine on 80x24 terminals. > 7) checkpatch.pl reports no problems (apart from a warning about a typedef) > 8) Cursor behaves correctly on input boxes. > 9) Code is split into 2 files, one for kconfig logic the other for gui logic. > > open issues: > > 1) hotkeys: I have yet failed to find a way to differently color letters inside > an ncurses menu. It seems color can only be set for the whole line. Hotkeys > are for now denoted by the first capital letter in the menu item. > 2) on unsporting terminals the following keys generate an : page up, page > down, home, end, del, insert. Pressing one of them is equivalent to . > Only happens when TERM=vt100 on my machine. Hi Nir. I have used nconfig for a while now and I prefer it over menuconfig. nconfig tries to be a clone of menuconfig in meny respects failing to address some of the shortcomings. Some of the caveats I have with nconfig (and with menuconfig): - We have the buttons in the bottom of the sceen. What I would like to see is that the ",\n" > +" , and \n" > +"\n" > +"o To get help with an item, use the cursor keys to highlight \n" > +" and Press .\n" > +"\n" > +" Shortcut: Press or .\n" > +"\n" > +"\n" > +"Radiolists (Choice lists)\n" > +"-----------\n" > +"o Use the cursor keys to select the option you wish to set and press\n" > +" or the .\n" > +"\n" > +" Shortcut: Press the first letter of the option you wish to set then\n" > +" press or .\n" > +"\n" > +"o To see available help for the item, use the cursor keys to highlight\n" > +" and Press .\n" > +"\n" > +" Shortcut: Press or .\n" > +"\n" > +" Also, the and cursor keys will cycle between ", > + " ", > + " ", > + "", > + NULL > +}; > + > +/* this array is used to implement hot keys. it is updated in item_make and > + * resetted in clean_items. It would be better to use a hash, but lets keep it > + * simple... */ > +#define MAX_SAME_KEY MAX_MENU_ITEMS > +struct { > + int count; > + int ptrs[MAX_MENU_ITEMS]; > +} hotkeys[1<<(sizeof(char)*8)]; > + > +static void conf(struct menu *menu); > +static void conf_choice(struct menu *menu); > +static void conf_string(struct menu *menu); > +static void conf_load(void); > +static void conf_save(void); > +static void show_help(struct menu *menu); > + > +static void update_current_btn(void) > +{ > + int index = item_index(current_item(btns_menu)); > + switch (index) { > + case 0: > + current_btn = BTN_SELECT; > + break; > + case 1: > + current_btn = BTN_EXIT; > + break; > + case 2: > + current_btn = BTN_HELP; > + break; > + case 3: > + current_btn = BTN_INSTS; > + break; > + } > +} > + > + > +static void clean_items(void) > +{ > + int i; > + for (i = 0; curses_menu_items[i]; i++) > + free_item(curses_menu_items[i]); > + bzero(curses_menu_items, sizeof(curses_menu_items)); > + bzero(k_menu_items, sizeof(k_menu_items)); > + bzero(hotkeys, sizeof(hotkeys)); > + items_num = 0; > +} > + > +/* return the index of the next hot item, or -1 if no such item exists */ > +int get_next_hot(int c) > +{ > + static int hot_index; > + static int hot_char; > + > + if (c < 0 || c > 255 || hotkeys[c].count <= 0) > + return -1; > + > + if (hot_char == c) { > + hot_index = (hot_index+1)%hotkeys[c].count; > + return hotkeys[c].ptrs[hot_index]; > + } else { > + hot_char = c; > + hot_index = 0; > + return hotkeys[c].ptrs[0]; > + } > +} > + > +/* can the char c be a hot key? no, if c is a common shortcut used elsewhere */ > +int canbhot(char c) > +{ > + return isalnum(c) && c != 'm' && c != 'M' && c != 'h' && c != 'H' && > + c != 'n' && c != 'N' && c != 's' && c != '?'; > +} > + > +/* check if str already contains a hot key. */ > +int is_hot(int index) > +{ > + return k_menu_items[index].is_hot; > +} > + > +/* find the first possible hot key, and mark it. > + * index is the index of the item in the menu > + * return 0 on success*/ > +int make_hot(char *dest, int len, const char *org, int index) > +{ > + int position = -1; > + int i; > + int tmp; > + int c; > + int org_len = strlen(org); > + > + if (org == NULL || is_hot(index)) > + return 1; > + > + /* make sure not to make hot keys out of markers. > + * find where to start looking for a hot key > + */ > + i = 0; > + /* skip white space */ > + while (i < org_len && org[i] == ' ') > + i++; > + if (i == org_len) > + return -1; > + /* if encountering '(' or '<' or '[', find the match and look from there > + **/ > + if (org[i] == '[' || org[i] == '<' || org[i] == '(') { > + i++; > + for (; i < org_len; i++) > + if (org[i] == ']' || org[i] == '>' || org[i] == ')') > + break; > + } > + if (i == org_len) > + return -1; > + for (; i < org_len; i++) { > + if (canbhot(org[i]) && org[i-1] != '<' && org[i-1] != '(') { > + position = i; > + break; > + } > + } > + if (position == -1) > + return 1; > + > + /* ok, char at org[position] should be a hot key to this item */ > + c = tolower(org[position]); > + tmp = hotkeys[c].count; > + hotkeys[c].ptrs[tmp] = index; > + hotkeys[c].count++; > + /* > + snprintf(dest, len, "%.*s(%c)%s", position, org, org[position], > + &org[position+1]); > + */ > + /* make org[position] uppercase, and all leading letter small case */ > + strncpy(dest, org, len); > + for (i = 0; i < position; i++) > + dest[i] = tolower(dest[i]); > + dest[position] = toupper(dest[position]); > + k_menu_items[index].is_hot = 1; > + return 0; > +} > + > +/* Make a new item. Add a hotkey mark in the first possible letter. > + * As ncurses does not allow any attributes inside menue item, we use mark the > + * hot key as the first capitalized letter in the string */ > +void item_make(void *usrptr, char tag, const char *fmt, ...) > +{ > + va_list ap; > + char tmp_str[256]; > + > + if (items_num > MAX_MENU_ITEMS-1) > + return; > + > + bzero(&k_menu_items[items_num], sizeof(k_menu_items[0])); > + k_menu_items[items_num].tag = tag; > + k_menu_items[items_num].usrptr = usrptr; > + > + va_start(ap, fmt); > + vsnprintf(tmp_str, sizeof(tmp_str), fmt, ap); > + va_end(ap); > + if (make_hot( > + k_menu_items[items_num].str, > + sizeof(k_menu_items[items_num].str), tmp_str, items_num) != 0) > + strncpy(k_menu_items[items_num].str, > + tmp_str, > + sizeof(k_menu_items[items_num].str)); > + > + curses_menu_items[items_num] = new_item( > + k_menu_items[items_num].str, > + k_menu_items[items_num].str); > + set_item_userptr(curses_menu_items[items_num], > + &k_menu_items[items_num]); > + items_num++; > + curses_menu_items[items_num] = NULL; > +} > + > +/* very hackish. adds a string to the last item added */ > +void item_add_str(const char *fmt, ...) > +{ > + va_list ap; > + int index = items_num-1; > + char new_str[256]; > + char tmp_str[256]; > + > + if (index < 0) > + return; > + > + va_start(ap, fmt); > + vsnprintf(new_str, sizeof(new_str), fmt, ap); > + va_end(ap); > + snprintf(tmp_str, sizeof(tmp_str), "%s%s", > + k_menu_items[index].str, new_str); > + if (make_hot(k_menu_items[index].str, > + sizeof(k_menu_items[index].str), tmp_str, index) != 0) > + strncpy(k_menu_items[index].str, > + tmp_str, > + sizeof(k_menu_items[index].str)); > + > + free_item(curses_menu_items[index]); > + curses_menu_items[index] = new_item( > + k_menu_items[index].str, > + k_menu_items[index].str); > + set_item_userptr(curses_menu_items[index], > + &k_menu_items[index]); > +} > + > +/* get the tag of the currently selected item */ > +char item_tag(void) > +{ > + ITEM *cur; > + struct mitem *mcur; > + > + cur = current_item(curses_menu); > + if (cur == NULL) > + return 0; > + mcur = (struct mitem *) item_userptr(cur); > + return mcur->tag; > +} > + > +int curses_item_index(void) > +{ > + return item_index(current_item(curses_menu)); > +} > + > +void *item_data(void) > +{ > + ITEM *cur; > + struct mitem *mcur; > + > + cur = current_item(curses_menu); > + mcur = (struct mitem *) item_userptr(cur); > + return mcur->usrptr; > + > +} > + > +int item_is_tag(char tag) > +{ > + return item_tag() == tag; > +} > + > +static char filename[PATH_MAX+1]; > +const char *set_config_filename(const char *config_filename) > +{ > + static char menu_backtitle[PATH_MAX+128]; > + int size; > + struct symbol *sym; > + > + sym = sym_lookup("KERNELVERSION", 0); > + sym_calc_value(sym); > + size = snprintf(menu_backtitle, sizeof(menu_backtitle), > + _("%s - Linux Kernel v%s Configuration"), > + config_filename, sym_get_string_value(sym)); > + if (size >= sizeof(menu_backtitle)) > + menu_backtitle[sizeof(menu_backtitle)-1] = '\0'; > + > + size = snprintf(filename, sizeof(filename), "%s", config_filename); > + if (size >= sizeof(filename)) > + filename[sizeof(filename)-1] = '\0'; > + return menu_backtitle; > +} > + > + > +static void search_conf(void) > +{ > + struct symbol **sym_arr; > + struct gstr res; > + char dialog_input_result[100]; > + char *dialog_input; > + int dres; > +again: > + dres = dialog_inputbox(main_window, > + _("Search Configuration Parameter"), > + _("Enter CONFIG_ (sub)string to search for " > + "(with or without \"CONFIG\")"), > + "", dialog_input_result, 99); > + switch (dres) { > + case 0: > + break; > + case 1: > + show_scroll_win(main_window, > + _("Search Configuration"), search_help); > + goto again; > + default: > + return; > + } > + > + /* strip CONFIG_ if necessary */ > + dialog_input = dialog_input_result; > + if (strncasecmp(dialog_input_result, "CONFIG_", 7) == 0) > + dialog_input += 7; > + > + sym_arr = sym_re_search(dialog_input); > + res = get_relations_str(sym_arr); > + free(sym_arr); > + show_scroll_win(main_window, > + _("Search Results"), str_get(&res)); > + str_free(&res); > +} > + > + > +static void build_conf(struct menu *menu) > +{ > + struct symbol *sym; > + struct property *prop; > + struct menu *child; > + int type, tmp, doint = 2; > + tristate val; > + char ch; > + > + > + if (!menu_is_visible(menu)) > + return; > + > + sym = menu->sym; > + prop = menu->prompt; > + if (!sym) { > + if (prop && menu != current_menu) { > + const char *prompt = menu_get_prompt(menu); > + switch (prop->type) { > + case P_MENU: > + child_count++; > + prompt = _(prompt); > + if (single_menu_mode) { > + item_make(menu, 'm', > + "%s%*c%s", > + menu->data ? "-->" : "++>", > + indent + 1, ' ', prompt); > + } else > + item_make(menu, 'm', > + " %*c%s --->", > + indent + 1, > + ' ', prompt); > + > + if (single_menu_mode && menu->data) > + goto conf_childs; > + return; > + case P_COMMENT: > + if (prompt) { > + child_count++; > + item_make(menu, ':', > + " %*c*** %s ***", > + indent + 1, ' ', > + _(prompt)); > + } > + break; > + default: > + if (prompt) { > + child_count++; > + item_make(menu, ':', "---%*c%s", > + indent + 1, ' ', > + _(prompt)); > + } > + } > + } else > + doint = 0; > + goto conf_childs; > + } > + > + type = sym_get_type(sym); > + if (sym_is_choice(sym)) { > + struct symbol *def_sym = sym_get_choice_value(sym); > + struct menu *def_menu = NULL; > + > + child_count++; > + for (child = menu->list; child; child = child->next) { > + if (menu_is_visible(child) && child->sym == def_sym) > + def_menu = child; > + } > + > + val = sym_get_tristate_value(sym); > + if (sym_is_changable(sym)) { > + switch (type) { > + case S_BOOLEAN: > + item_make(menu, 't', "[%c]", > + val == no ? ' ' : '*'); > + break; > + case S_TRISTATE: > + switch (val) { > + case yes: > + ch = '*'; > + break; > + case mod: > + ch = 'M'; > + break; > + default: > + ch = ' '; > + break; > + } > + item_make(menu, 't', "<%c>", ch); > + break; > + } > + } else { > + item_make(menu, def_menu ? 't' : ':', " "); > + } > + > + item_add_str("%*c%s", indent + 1, > + ' ', _(menu_get_prompt(menu))); > + if (val == yes) { > + if (def_menu) { > + item_add_str(" (%s)", > + _(menu_get_prompt(def_menu))); > + item_add_str(" --->"); > + if (def_menu->list) { > + indent += 2; > + build_conf(def_menu); > + indent -= 2; > + } > + } > + return; > + } > + } else { > + if (menu == current_menu) { > + item_make(menu, ':', > + "---%*c%s", indent + 1, > + ' ', _(menu_get_prompt(menu))); > + goto conf_childs; > + } > + child_count++; > + val = sym_get_tristate_value(sym); > + if (sym_is_choice_value(sym) && val == yes) { > + item_make(menu, ':', " "); > + } else { > + switch (type) { > + case S_BOOLEAN: > + if (sym_is_changable(sym)) > + item_make(menu, 't', "[%c]", > + val == no ? ' ' : '*'); > + else > + item_make(menu, 't', "-%c-", > + val == no ? ' ' : '*'); > + break; > + case S_TRISTATE: > + switch (val) { > + case yes: > + ch = '*'; > + break; > + case mod: > + ch = 'M'; > + break; > + default: > + ch = ' '; > + break; > + } > + if (sym_is_changable(sym)) { > + if (sym->rev_dep.tri == mod) > + item_make(menu, > + 't', "{%c}", ch); > + else > + item_make(menu, > + 't', "<%c>", ch); > + } else > + item_make(menu, 't', "-%c-", ch); > + break; > + default: > + tmp = 2 + strlen(sym_get_string_value(sym)); > + item_make(menu, 's', "(%s)", > + sym_get_string_value(sym)); > + tmp = indent - tmp + 4; > + if (tmp < 0) > + tmp = 0; > + item_add_str("%*c%s%s", tmp, ' ', > + _(menu_get_prompt(menu)), > + (sym_has_value(sym) || > + !sym_is_changable(sym)) ? "" : > + _(" (NEW)")); > + goto conf_childs; > + } > + } > + item_add_str("%*c%s%s", indent + 1, ' ', > + _(menu_get_prompt(menu)), > + (sym_has_value(sym) || !sym_is_changable(sym)) ? > + "" : _(" (NEW)")); > + if (menu->prompt->type == P_MENU) { > + item_add_str(" --->"); > + return; > + } > + } > + > +conf_childs: > + indent += doint; > + for (child = menu->list; child; child = child->next) > + build_conf(child); > + indent -= doint; > +} > + > +static void reset_menu(void) > +{ > + unpost_menu(curses_menu); > + unpost_menu(btns_menu); > + clean_items(); > +} > + > +/* adjust the menu to show this item. > + * prefer not to scroll the menu if possible*/ > +static void center_item(int selected_index) > +{ > + int toprow; > + int maxy, maxx; > + > + scale_menu(curses_menu, &maxy, &maxx); > + toprow = top_row(curses_menu); > + if (selected_index >= toprow && selected_index < toprow+maxy) { > + /* we can only move the selected item. no need to scroll */ > + set_current_item(curses_menu, > + curses_menu_items[selected_index]); > + } else { > + toprow = max(selected_index-maxy/2, 0); > + if (toprow >= item_count(curses_menu)-maxy) > + toprow = item_count(curses_menu)-mwin_max_lines; > + set_top_row(curses_menu, toprow); > + set_current_item(curses_menu, > + curses_menu_items[selected_index]); > + } > + post_menu(curses_menu); > + refresh_all_windows(main_window); > +} > + > +/* this function assumes reset_menu has been called before */ > +static void show_menu(const char *prompt, const char *instructions, > + int selected_index) > +{ > + int maxx, maxy; > + WINDOW *menu_window; > + > + current_instructions = instructions; > + > + wattrset(main_window, -1); > + > + wattrset(main_window, attributes[MAIN_MENU_BOX]); > + box(main_window, 0, 0); > + wattrset(main_window, attributes[MAIN_MENU_HEADING]); > + mvwprintw(main_window, 0, 3, " %s ", prompt); > + wattrset(main_window, attributes[NORMAL]); > + > + set_menu_items(curses_menu, curses_menu_items); > + > + /* position the menu at the middle of the screen */ > + scale_menu(curses_menu, &maxy, &maxx); > + maxx = min(maxx, mwin_max_cols); > + menu_window = derwin(main_window, > + mwin_max_lines, > + maxx, > + 2, > + (COLS-maxx)/2); > + keypad(menu_window, TRUE); > + set_menu_win(curses_menu, menu_window); > + set_menu_sub(curses_menu, menu_window); > + > + /* must reassert this after changing items, otherwise returns to a > + * default of 16 > + */ > + set_menu_format(curses_menu, mwin_max_lines, 1); > + center_item(selected_index); > + > + > + /* move lower menu btn to