Messages in this thread Patch in this message | | | Subject | Re: [PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config | From | Taeung Song <> | Date | Tue, 10 May 2016 20:33:04 +0900 |
| |
Hi, Arnaldo :)
On 05/10/2016 02:17 AM, Arnaldo Carvalho de Melo wrote: > Em Mon, May 09, 2016 at 08:41:47PM +0900, Taeung Song escreveu: >> ui_browser__color_config() set foreground and background >> colors values in ui_browser__colorsets. > > "ground colors" sounds strange, I guess referreing to them as "*colors" > or "{back,fore}ground colors" is more appropriate, replace "gcolors" > with "colors" too, please.
I got it.
>> But it can be reused by other functions so make ui_browser__config_gcolors() >> bringing it from ui_browser__color_config(). >> >> Cc: Namhyung Kim <namhyung@kernel.org> >> Cc: Jiri Olsa <jolsa@kernel.org> >> Signed-off-by: Taeung Song <treeze.taeung@gmail.com> >> --- >> tools/perf/ui/browser.c | 43 ++++++++++++++++++++++++++----------------- >> 1 file changed, 26 insertions(+), 17 deletions(-) >> >> diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c >> index af68a9d..a477867 100644 >> --- a/tools/perf/ui/browser.c >> +++ b/tools/perf/ui/browser.c >> @@ -553,12 +553,33 @@ static struct ui_browser_colorset { >> } >> }; >> >> +static int ui_browser__config_gcolors(struct ui_browser_colorset *ui_browser_color, >> + const char *value) >> +{ >> + char *fg = NULL, *bg; >> + >> + fg = strdup(value); >> + if (fg == NULL) >> + return -1; >> + >> + bg = strchr(fg, ','); >> + if (bg == NULL) { >> + free(fg); >> + return -1; >> + } >> + >> + *bg = '\0'; > > Isn't the above strtok()? > >> + while (isspace(*++bg)); > > Isn't this ltrim()?
I modified it like below and retested it !
diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c index a477867..c905445 100644 --- a/tools/perf/ui/browser.c +++ b/tools/perf/ui/browser.c @@ -553,8 +553,8 @@ static struct ui_browser_colorset { } };
-static int ui_browser__config_gcolors(struct ui_browser_colorset *ui_browser_color, - const char *value) +static int ui_browser__config_colors(struct ui_browser_colorset *ui_browser_color, + const char *value) { char *fg = NULL, *bg;
@@ -562,14 +562,12 @@ static int ui_browser__config_gcolors(struct ui_browser_colorset *ui_browser_col if (fg == NULL) return -1;
- bg = strchr(fg, ','); + bg = strtok(fg, ","); if (bg == NULL) { free(fg); return -1; } - - *bg = '\0'; - while (isspace(*++bg)); + ltrim(bg);
ui_browser_color->fg = fg; ui_browser_color->bg = bg; @@ -591,7 +589,7 @@ static int ui_browser__color_config(const char *var, const char *value, if (strcmp(ui_browser__colorsets[i].name, name) != 0) continue;
- ret = ui_browser__config_gcolors(&ui_browser__colorsets[i], value); + ret = ui_browser__config_colors(&ui_browser__colorsets[i], value); break; }
I'll send this modified patch with v2
Thanks, Taeung
| |