Re: [PATCH] branch: colour upstream branches
Felipe Contreras felipe.contre...@gmail.com writes:
It's hard to see them among so much output otherwise.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
builtin/branch.c | 27 ---
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 00d17d2..a1cdc29 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -417,32 +417,45 @@ static void fill_tracking_info(struct strbuf *stat,
const char *branch_name,
int ours, theirs;
char *ref = NULL;
struct branch *branch = branch_get(branch_name);
+ char fancy[80];
if (!stat_tracking_info(branch, ours, theirs)) {
if (branch branch-merge branch-merge[0]-dst
- show_upstream_ref)
- strbuf_addf(stat, [%s] ,
- shorten_unambiguous_ref(branch-merge[0]-dst, 0));
+ show_upstream_ref) {
+ ref = shorten_unambiguous_ref(branch-merge[0]-dst, 0);
+ if (want_color(branch_use_color))
+ strbuf_addf(stat, [%s%s%s] ,
+ GIT_COLOR_BLUE, ref,
GIT_COLOR_RESET);
AFAICS you are hardcoding the color? The other bits of branch colors
are configurable through branch.color.*, so why not this?
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: colour upstream branches
On Sun, Apr 14, 2013 at 11:46 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
+ char fancy[80];
- if (show_upstream_ref)
+ if (show_upstream_ref) {
ref = shorten_unambiguous_ref(branch-merge[0]-dst, 0);
+ if (want_color(branch_use_color))
+ snprintf(fancy, sizeof(fancy), %s%s%s,
+ GIT_COLOR_BLUE, ref, GIT_COLOR_RESET);
+ else
+ strncpy(fancy, ref, sizeof(fancy));
+ }
+
Please use strbuf for fancy. I also agree with Thomas that the color
should not be hard coded.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: colour upstream branches
On Sun, Apr 14, 2013 at 5:31 PM, Duy Nguyen pclo...@gmail.com wrote:
On Sun, Apr 14, 2013 at 11:46 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
+ char fancy[80];
- if (show_upstream_ref)
+ if (show_upstream_ref) {
ref = shorten_unambiguous_ref(branch-merge[0]-dst, 0);
+ if (want_color(branch_use_color))
+ snprintf(fancy, sizeof(fancy), %s%s%s,
+ GIT_COLOR_BLUE, ref,
GIT_COLOR_RESET);
+ else
+ strncpy(fancy, ref, sizeof(fancy));
+ }
+
Please use strbuf for fancy.
Why? We would need to initialize and free it. What's the advantage?
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: colour upstream branches
On Mon, Apr 15, 2013 at 9:22 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Sun, Apr 14, 2013 at 5:31 PM, Duy Nguyen pclo...@gmail.com wrote:
On Sun, Apr 14, 2013 at 11:46 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
+ char fancy[80];
- if (show_upstream_ref)
+ if (show_upstream_ref) {
ref = shorten_unambiguous_ref(branch-merge[0]-dst, 0);
+ if (want_color(branch_use_color))
+ snprintf(fancy, sizeof(fancy), %s%s%s,
+ GIT_COLOR_BLUE, ref,
GIT_COLOR_RESET);
+ else
+ strncpy(fancy, ref, sizeof(fancy));
+ }
+
Please use strbuf for fancy.
Why? We would need to initialize and free it. What's the advantage?
From a quick glance, I don't see any gurantee that ref (plus ansi
codes) will always fit in 80 bytes.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: colour upstream branches
On Sun, Apr 14, 2013 at 6:46 PM, Duy Nguyen pclo...@gmail.com wrote:
On Mon, Apr 15, 2013 at 9:22 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Sun, Apr 14, 2013 at 5:31 PM, Duy Nguyen pclo...@gmail.com wrote:
On Sun, Apr 14, 2013 at 11:46 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
+ char fancy[80];
- if (show_upstream_ref)
+ if (show_upstream_ref) {
ref = shorten_unambiguous_ref(branch-merge[0]-dst, 0);
+ if (want_color(branch_use_color))
+ snprintf(fancy, sizeof(fancy), %s%s%s,
+ GIT_COLOR_BLUE, ref,
GIT_COLOR_RESET);
+ else
+ strncpy(fancy, ref, sizeof(fancy));
+ }
+
Please use strbuf for fancy.
Why? We would need to initialize and free it. What's the advantage?
From a quick glance, I don't see any gurantee that ref (plus ansi
codes) will always fit in 80 bytes.
Would changing it to 1024 (MAXREFLEN) fix it?
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: colour upstream branches
On Mon, Apr 15, 2013 at 9:54 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Sun, Apr 14, 2013 at 6:46 PM, Duy Nguyen pclo...@gmail.com wrote:
On Mon, Apr 15, 2013 at 9:22 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Sun, Apr 14, 2013 at 5:31 PM, Duy Nguyen pclo...@gmail.com wrote:
On Sun, Apr 14, 2013 at 11:46 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
+ char fancy[80];
- if (show_upstream_ref)
+ if (show_upstream_ref) {
ref = shorten_unambiguous_ref(branch-merge[0]-dst, 0);
+ if (want_color(branch_use_color))
+ snprintf(fancy, sizeof(fancy), %s%s%s,
+ GIT_COLOR_BLUE, ref,
GIT_COLOR_RESET);
+ else
+ strncpy(fancy, ref, sizeof(fancy));
+ }
+
Please use strbuf for fancy.
Why? We would need to initialize and free it. What's the advantage?
From a quick glance, I don't see any gurantee that ref (plus ansi
codes) will always fit in 80 bytes.
Would changing it to 1024 (MAXREFLEN) fix it?
You still need to take ansi codes into account. I think it's easier to
just use strbuf.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: colour upstream branches
On Sun, Apr 14, 2013 at 7:55 PM, Duy Nguyen pclo...@gmail.com wrote:
On Mon, Apr 15, 2013 at 9:54 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Sun, Apr 14, 2013 at 6:46 PM, Duy Nguyen pclo...@gmail.com wrote:
On Mon, Apr 15, 2013 at 9:22 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Sun, Apr 14, 2013 at 5:31 PM, Duy Nguyen pclo...@gmail.com wrote:
On Sun, Apr 14, 2013 at 11:46 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
+ char fancy[80];
- if (show_upstream_ref)
+ if (show_upstream_ref) {
ref = shorten_unambiguous_ref(branch-merge[0]-dst, 0);
+ if (want_color(branch_use_color))
+ snprintf(fancy, sizeof(fancy), %s%s%s,
+ GIT_COLOR_BLUE, ref,
GIT_COLOR_RESET);
+ else
+ strncpy(fancy, ref, sizeof(fancy));
+ }
+
Please use strbuf for fancy.
Why? We would need to initialize and free it. What's the advantage?
From a quick glance, I don't see any gurantee that ref (plus ansi
codes) will always fit in 80 bytes.
Would changing it to 1024 (MAXREFLEN) fix it?
You still need to take ansi codes into account. I think it's easier to
just use strbuf.
I'm not sure what you mean. If there was an issue with snprintf, then
there would be with this in refs.c:
if (recursion MAXDEPTH || strlen(refname) MAXREFLEN)
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: colour upstream branches
On Mon, Apr 15, 2013 at 11:24 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Sun, Apr 14, 2013 at 7:55 PM, Duy Nguyen pclo...@gmail.com wrote:
On Mon, Apr 15, 2013 at 9:54 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Sun, Apr 14, 2013 at 6:46 PM, Duy Nguyen pclo...@gmail.com wrote:
On Mon, Apr 15, 2013 at 9:22 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Sun, Apr 14, 2013 at 5:31 PM, Duy Nguyen pclo...@gmail.com wrote:
On Sun, Apr 14, 2013 at 11:46 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
+ char fancy[80];
- if (show_upstream_ref)
+ if (show_upstream_ref) {
ref = shorten_unambiguous_ref(branch-merge[0]-dst, 0);
+ if (want_color(branch_use_color))
+ snprintf(fancy, sizeof(fancy), %s%s%s,
+ GIT_COLOR_BLUE, ref,
GIT_COLOR_RESET);
+ else
+ strncpy(fancy, ref, sizeof(fancy));
+ }
+
Please use strbuf for fancy.
Why? We would need to initialize and free it. What's the advantage?
From a quick glance, I don't see any gurantee that ref (plus ansi
codes) will always fit in 80 bytes.
Would changing it to 1024 (MAXREFLEN) fix it?
You still need to take ansi codes into account. I think it's easier to
just use strbuf.
I'm not sure what you mean. If there was an issue with snprintf, then
there would be with this in refs.c:
if (recursion MAXDEPTH || strlen(refname) MAXREFLEN)
I mean GIT_COLOR_BLUE and GIT_COLOR_RESET take some bytes from
fancy. In the stretched case where ref takes all MAXREFLEN, then
fancy must be a little bit bigger. Why do we need to think hard about
the size of fancy when strbuf would solve it nicely?
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] branch: colour upstream branches
On Sun, Apr 14, 2013 at 8:31 PM, Duy Nguyen pclo...@gmail.com wrote:
On Mon, Apr 15, 2013 at 11:24 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Sun, Apr 14, 2013 at 7:55 PM, Duy Nguyen pclo...@gmail.com wrote:
On Mon, Apr 15, 2013 at 9:54 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Sun, Apr 14, 2013 at 6:46 PM, Duy Nguyen pclo...@gmail.com wrote:
On Mon, Apr 15, 2013 at 9:22 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Sun, Apr 14, 2013 at 5:31 PM, Duy Nguyen pclo...@gmail.com wrote:
On Sun, Apr 14, 2013 at 11:46 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
+ char fancy[80];
- if (show_upstream_ref)
+ if (show_upstream_ref) {
ref = shorten_unambiguous_ref(branch-merge[0]-dst,
0);
+ if (want_color(branch_use_color))
+ snprintf(fancy, sizeof(fancy), %s%s%s,
+ GIT_COLOR_BLUE, ref,
GIT_COLOR_RESET);
+ else
+ strncpy(fancy, ref, sizeof(fancy));
+ }
+
Please use strbuf for fancy.
Why? We would need to initialize and free it. What's the advantage?
From a quick glance, I don't see any gurantee that ref (plus ansi
codes) will always fit in 80 bytes.
Would changing it to 1024 (MAXREFLEN) fix it?
You still need to take ansi codes into account. I think it's easier to
just use strbuf.
I'm not sure what you mean. If there was an issue with snprintf, then
there would be with this in refs.c:
if (recursion MAXDEPTH || strlen(refname) MAXREFLEN)
I mean GIT_COLOR_BLUE and GIT_COLOR_RESET take some bytes from
fancy. In the stretched case where ref takes all MAXREFLEN, then
fancy must be a little bit bigger. Why do we need to think hard about
I don't see that as a big issue, since it's a matter of finding the
maximum length of those, but the argument from Jeff convinces me; I
would not like to add extra code for the snprintf corner cases.
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
