Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1396966 > unrolled thread

[PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config

Started byTaeung Song <treeze.taeung@gmail.com>
First post2016-05-09 13:50 +0200
Last post2016-05-10 13:40 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config Taeung Song <treeze.taeung@gmail.com> - 2016-05-09 13:50 +0200
    Re: [PATCH 2/4] perf tools: Separate out code setting ground colors  from ui_browser__color_config Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-05-09 19:20 +0200
      Re: [PATCH 2/4] perf tools: Separate out code setting ground colors  from ui_browser__color_config Taeung Song <treeze.taeung@gmail.com> - 2016-05-10 13:40 +0200

#1396966 — [PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config

FromTaeung Song <treeze.taeung@gmail.com>
Date2016-05-09 13:50 +0200
Subject[PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config
Message-ID<rwRRg-4H5-13@gated-at.bofh.it>
ui_browser__color_config() set foreground and background
colors values in ui_browser__colorsets.
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';
+	while (isspace(*++bg));
+
+	ui_browser_color->fg = fg;
+	ui_browser_color->bg = bg;
+	return 0;
+}
 
 static int ui_browser__color_config(const char *var, const char *value,
 				    void *data __maybe_unused)
 {
-	char *fg = NULL, *bg;
-	int i;
+	int i, ret;
 
 	/* same dir for all commands */
 	if (prefixcmp(var, "colors.") != 0)
@@ -570,23 +591,11 @@ static int ui_browser__color_config(const char *var, const char *value,
 		if (strcmp(ui_browser__colorsets[i].name, name) != 0)
 			continue;
 
-		fg = strdup(value);
-		if (fg == NULL)
-			break;
-
-		bg = strchr(fg, ',');
-		if (bg == NULL)
-			break;
-
-		*bg = '\0';
-		while (isspace(*++bg));
-		ui_browser__colorsets[i].bg = bg;
-		ui_browser__colorsets[i].fg = fg;
-		return 0;
+		ret = ui_browser__config_gcolors(&ui_browser__colorsets[i], value);
+		break;
 	}
 
-	free(fg);
-	return -1;
+	return ret;
 }
 
 void ui_browser__argv_seek(struct ui_browser *browser, off_t offset, int whence)
-- 
2.5.0

[toc] | [next] | [standalone]


#1397199 — Re: [PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2016-05-09 19:20 +0200
SubjectRe: [PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config
Message-ID<rwX0C-1y9-13@gated-at.bofh.it>
In reply to#1396966
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.

> 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()?

> +
> +	ui_browser_color->fg = fg;
> +	ui_browser_color->bg = bg;
> +	return 0;
> +}
>  
>  static int ui_browser__color_config(const char *var, const char *value,
>  				    void *data __maybe_unused)
>  {
> -	char *fg = NULL, *bg;
> -	int i;
> +	int i, ret;
>  
>  	/* same dir for all commands */
>  	if (prefixcmp(var, "colors.") != 0)
> @@ -570,23 +591,11 @@ static int ui_browser__color_config(const char *var, const char *value,
>  		if (strcmp(ui_browser__colorsets[i].name, name) != 0)
>  			continue;
>  
> -		fg = strdup(value);
> -		if (fg == NULL)
> -			break;
> -
> -		bg = strchr(fg, ',');
> -		if (bg == NULL)
> -			break;
> -
> -		*bg = '\0';
> -		while (isspace(*++bg));

strtok + ltrim

> -		ui_browser__colorsets[i].bg = bg;
> -		ui_browser__colorsets[i].fg = fg;
> -		return 0;
> +		ret = ui_browser__config_gcolors(&ui_browser__colorsets[i], value);
> +		break;
>  	}
>  
> -	free(fg);
> -	return -1;
> +	return ret;
>  }
>  
>  void ui_browser__argv_seek(struct ui_browser *browser, off_t offset, int whence)
> -- 
> 2.5.0

[toc] | [prev] | [next] | [standalone]


#1398008 — Re: [PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config

FromTaeung Song <treeze.taeung@gmail.com>
Date2016-05-10 13:40 +0200
SubjectRe: [PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config
Message-ID<rxeb8-1wO-15@gated-at.bofh.it>
In reply to#1397199
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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web