* [PATCH UI experiment] diffstat: annotate/highlight new or removed files
@ 2012-10-15 14:35 Nguyễn Thái Ngọc Duy
2012-10-15 21:03 ` Junio C Hamano
0 siblings, 1 reply; 3+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2012-10-15 14:35 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
diffstat does not show whether a file is added or deleted. I know
--summary does. But the problem with --summary is it makes me look for
information of a file in two places: diffstat and summary. And with a
commit that adds/removes a lot, showing both --stat --summary can be
long.
This patch adds "(new)", "(gone)" or "(new mode)" to diffstat, with
highlight, to easily catch file additions/removals. The extra text is
chosen to be short enough so that it won't take up too much space for
path names:
.gitignore | 1 +
Makefile | 3 +
t/t3070-wildmatch.sh (new) | 188 ++++++++++++++++++++++++
t/t3070/wildtest.txt (gone) | 165 ---------------------
test-wildmatch.c (new) | 14 ++
wildmatch.c | 5 +-
6 files changed, 210 insertions(+), 166 deletions(-)
I don't put creation modes in there too because most of the time it
does not matter much to me and I could look down to --summary for mode
verification. But we could put "(new+x)" for 0755 and just "(new)" for
0644. "(new mode)" then could become "(+x)", "(-x)" or something like
that.
Coloring is to me an improvement over --summary. Probably the main
point. Without it, perhaps it's not worth putting extra text to
diffstat.
Single patch for easy testing. Test suite probably breaks.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
diff.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++-----------------
diff.h | 3 ++-
utf8.c | 33 +++++++++++++++++++++++++++
utf8.h | 2 ++
4 files changed, 97 insertions(+), 22 deletions(-)
diff --git a/diff.c b/diff.c
index 35d3f07..f9217e6 100644
--- a/diff.c
+++ b/diff.c
@@ -45,6 +45,7 @@ static char diff_colors[][COLOR_MAXLEN] = {
GIT_COLOR_YELLOW, /* COMMIT */
GIT_COLOR_BG_RED, /* WHITESPACE */
GIT_COLOR_NORMAL, /* FUNCINFO */
+ GIT_COLOR_YELLOW, /* STATUSINFO */
};
static int parse_diff_color_slot(const char *var, int ofs)
@@ -65,6 +66,8 @@ static int parse_diff_color_slot(const char *var, int ofs)
return DIFF_WHITESPACE;
if (!strcasecmp(var+ofs, "func"))
return DIFF_FUNCINFO;
+ if (!strcasecmp(var+ofs, "status"))
+ return DIFF_STATUSINFO;
return -1;
}
@@ -1223,7 +1226,8 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
}
}
-static char *pprint_rename(const char *a, const char *b)
+static char *pprint_rename(struct diff_options *options,
+ const char *a, const char *b)
{
const char *old = a;
const char *new = b;
@@ -1237,7 +1241,9 @@ static char *pprint_rename(const char *a, const char *b)
if (qlen_a || qlen_b) {
quote_c_style(a, &name, NULL, 0);
- strbuf_addstr(&name, " => ");
+ strbuf_addf(&name, " %s=>%s ",
+ diff_get_color_opt(options, DIFF_STATUSINFO),
+ diff_get_color_opt(options, DIFF_RESET));
quote_c_style(b, &name, NULL, 0);
return strbuf_detach(&name, NULL);
}
@@ -1278,13 +1284,19 @@ static char *pprint_rename(const char *a, const char *b)
strbuf_grow(&name, pfx_length + a_midlen + b_midlen + sfx_length + 7);
if (pfx_length + sfx_length) {
strbuf_add(&name, a, pfx_length);
- strbuf_addch(&name, '{');
+ strbuf_addf(&name, "%s{%s",
+ diff_get_color_opt(options, DIFF_STATUSINFO),
+ diff_get_color_opt(options, DIFF_RESET));
}
strbuf_add(&name, a + pfx_length, a_midlen);
- strbuf_addstr(&name, " => ");
+ strbuf_addf(&name, " %s=>%s ",
+ diff_get_color_opt(options, DIFF_STATUSINFO),
+ diff_get_color_opt(options, DIFF_RESET));
strbuf_add(&name, b + pfx_length, b_midlen);
if (pfx_length + sfx_length) {
- strbuf_addch(&name, '}');
+ strbuf_addf(&name, "%s}%s",
+ diff_get_color_opt(options, DIFF_STATUSINFO),
+ diff_get_color_opt(options, DIFF_RESET));
strbuf_add(&name, a + len_a - sfx_length, sfx_length);
}
return strbuf_detach(&name, NULL);
@@ -1300,6 +1312,7 @@ struct diffstat_t {
unsigned is_unmerged:1;
unsigned is_binary:1;
unsigned is_renamed:1;
+ char status;
uintmax_t added, deleted;
} **files;
};
@@ -1357,7 +1370,8 @@ static int scale_linear(int it, int width, int max_change)
static void show_name(FILE *file,
const char *prefix, const char *name, int len)
{
- fprintf(file, " %s%-*s |", prefix, len, name);
+ fprintf(file, " %s%-*s |", prefix,
+ len + strlen_ansi(name), name);
}
static void show_graph(FILE *file, char ch, int cnt, const char *set, const char *reset)
@@ -1370,7 +1384,8 @@ static void show_graph(FILE *file, char ch, int cnt, const char *set, const char
fprintf(file, "%s", reset);
}
-static void fill_print_name(struct diffstat_file *file)
+static void fill_print_name(struct diff_options *options,
+ struct diffstat_file *file)
{
char *pname;
@@ -1379,14 +1394,32 @@ static void fill_print_name(struct diffstat_file *file)
if (!file->is_renamed) {
struct strbuf buf = STRBUF_INIT;
- if (quote_c_style(file->name, &buf, NULL, 0)) {
+ if (quote_c_style(file->name, &buf, NULL, 0) ||
+ file->status) {
+ const char *str = NULL;
+ switch (file->status) {
+ case DIFF_STATUS_ADDED:
+ str = "new";
+ break;
+ case DIFF_STATUS_DELETED:
+ str = "gone";
+ break;
+ case DIFF_STATUS_TYPE_CHANGED:
+ str = "new mode";
+ break;
+ }
+ if (str)
+ strbuf_addf(&buf, " (%s%s%s)",
+ diff_get_color_opt(options, DIFF_STATUSINFO),
+ str,
+ diff_get_color_opt(options, DIFF_RESET));
pname = strbuf_detach(&buf, NULL);
} else {
pname = file->name;
strbuf_release(&buf);
}
} else {
- pname = pprint_rename(file->from_name, file->name);
+ pname = pprint_rename(options, file->from_name, file->name);
}
file->print_name = pname;
}
@@ -1474,8 +1507,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
count++; /* not shown == room for one more */
continue;
}
- fill_print_name(file);
- len = strlen(file->print_name);
+ fill_print_name(options, file);
+ len = strlen_no_ansi(file->print_name);
if (max_len < len)
max_len = len;
@@ -1599,7 +1632,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
* "scale" the filename
*/
len = name_width;
- name_len = strlen(name);
+ name_len = strlen_no_ansi(name);
if (name_width < name_len) {
char *slash;
prefix = "...";
@@ -1737,7 +1770,7 @@ static void show_numstat(struct diffstat_t *data, struct diff_options *options)
"%"PRIuMAX"\t%"PRIuMAX"\t",
file->added, file->deleted);
if (options->line_termination) {
- fill_print_name(file);
+ fill_print_name(options, file);
if (!file->is_renamed)
write_name_quoted(file->name, options->file,
options->line_termination);
@@ -2397,13 +2430,15 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
struct diff_filespec *two,
struct diffstat_t *diffstat,
struct diff_options *o,
- int complete_rewrite)
+ int complete_rewrite,
+ char status)
{
mmfile_t mf1, mf2;
struct diffstat_file *data;
int same_contents;
data = diffstat_add(diffstat, name_a, name_b);
+ data->status = status;
if (!one || !two) {
data->is_unmerged = 1;
@@ -3118,7 +3153,8 @@ static void run_diffstat(struct diff_filepair *p, struct diff_options *o,
if (DIFF_PAIR_UNMERGED(p)) {
/* unmerged */
- builtin_diffstat(p->one->path, NULL, NULL, NULL, diffstat, o, 0);
+ builtin_diffstat(p->one->path, NULL, NULL, NULL,
+ diffstat, o, 0, 0);
return;
}
@@ -3133,7 +3169,8 @@ static void run_diffstat(struct diff_filepair *p, struct diff_options *o,
if (p->status == DIFF_STATUS_MODIFIED && p->score)
complete_rewrite = 1;
- builtin_diffstat(name, other, p->one, p->two, diffstat, o, complete_rewrite);
+ builtin_diffstat(name, other, p->one, p->two, diffstat,
+ o, complete_rewrite, p->status);
}
static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
@@ -4118,10 +4155,12 @@ static void show_mode_change(FILE *file, struct diff_filepair *p, int show_name,
}
}
-static void show_rename_copy(FILE *file, const char *renamecopy, struct diff_filepair *p,
- const char *line_prefix)
+static void show_rename_copy(FILE *file, const char *renamecopy,
+ struct diff_filepair *p,
+ const char *line_prefix,
+ struct diff_options *options)
{
- char *names = pprint_rename(p->one->path, p->two->path);
+ char *names = pprint_rename(options, p->one->path, p->two->path);
fprintf(file, " %s %s (%d%%)\n", renamecopy, names, similarity_index(p));
free(names);
@@ -4149,11 +4188,11 @@ static void diff_summary(struct diff_options *opt, struct diff_filepair *p)
break;
case DIFF_STATUS_COPIED:
fputs(line_prefix, file);
- show_rename_copy(file, "copy", p, line_prefix);
+ show_rename_copy(file, "copy", p, line_prefix, opt);
break;
case DIFF_STATUS_RENAMED:
fputs(line_prefix, file);
- show_rename_copy(file, "rename", p, line_prefix);
+ show_rename_copy(file, "rename", p, line_prefix, opt);
break;
default:
if (p->score) {
diff --git a/diff.h b/diff.h
index a658f85..c609512 100644
--- a/diff.h
+++ b/diff.h
@@ -167,7 +167,8 @@ enum color_diff {
DIFF_FILE_NEW = 5,
DIFF_COMMIT = 6,
DIFF_WHITESPACE = 7,
- DIFF_FUNCINFO = 8
+ DIFF_FUNCINFO = 8,
+ DIFF_STATUSINFO = 9
};
const char *diff_get_color(int diff_use_color, enum color_diff ix);
#define diff_get_color_opt(o, ix) \
diff --git a/utf8.c b/utf8.c
index a544f15..10823fd 100644
--- a/utf8.c
+++ b/utf8.c
@@ -317,6 +317,39 @@ static size_t display_mode_esc_sequence_len(const char *s)
return p - s;
}
+size_t strlen_no_ansi(const char *s)
+{
+ size_t len = 0;
+ for (;;) {
+ size_t skip;
+
+ while ((skip = display_mode_esc_sequence_len(s)))
+ s += skip;
+ if (!*s)
+ break;
+ len++;
+ s++;
+ }
+ return len;
+}
+
+size_t strlen_ansi(const char *s)
+{
+ size_t len = 0;
+ for (;;) {
+ size_t skip;
+
+ while ((skip = display_mode_esc_sequence_len(s))) {
+ s += skip;
+ len += skip;
+ }
+ if (!*s)
+ break;
+ s++;
+ }
+ return len;
+}
+
/*
* Wrap the text, if necessary. The variable indent is the indent for the
* first line, indent2 is the indent for all other lines.
diff --git a/utf8.h b/utf8.h
index 3c0ae76..fd4cfca 100644
--- a/utf8.h
+++ b/utf8.h
@@ -3,6 +3,8 @@
typedef unsigned int ucs_char_t; /* assuming 32bit int */
+size_t strlen_no_ansi(const char *s);
+size_t strlen_ansi(const char *s);
int utf8_width(const char **start, size_t *remainder_p);
int utf8_strwidth(const char *string);
int is_utf8(const char *text);
--
1.8.0.rc2.11.g2b79d01
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH UI experiment] diffstat: annotate/highlight new or removed files
2012-10-15 14:35 [PATCH UI experiment] diffstat: annotate/highlight new or removed files Nguyễn Thái Ngọc Duy
@ 2012-10-15 21:03 ` Junio C Hamano
2012-10-16 11:15 ` Nguyen Thai Ngoc Duy
0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2012-10-15 21:03 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> diffstat does not show whether a file is added or deleted. I know
> --summary does. But the problem with --summary is it makes me look for
> information of a file in two places: diffstat and summary. And with a
> commit that adds/removes a lot, showing both --stat --summary can be
> long.
>
> This patch adds "(new)", "(gone)" or "(new mode)" to diffstat, with
> highlight, to easily catch file additions/removals. The extra text is
> chosen to be short enough so that it won't take up too much space for
> path names:
>
> .gitignore | 1 +
> Makefile | 3 +
> t/t3070-wildmatch.sh (new) | 188 ++++++++++++++++++++++++
> t/t3070/wildtest.txt (gone) | 165 ---------------------
> test-wildmatch.c (new) | 14 ++
> wildmatch.c | 5 +-
> 6 files changed, 210 insertions(+), 166 deletions(-)
>
> I don't put creation modes in there too because most of the time it
> does not matter much to me and I could look down to --summary for mode
> verification. But we could put "(new+x)" for 0755 and just "(new)" for
> 0644. "(new mode)" then could become "(+x)", "(-x)" or something like
> that.
>
> Coloring is to me an improvement over --summary. Probably the main
> point. Without it, perhaps it's not worth putting extra text to
> diffstat.
It is kind of surprising that you did not choose to paint new in
green and gone in red, and rather paint everything in yellow.
I personally think the above in monochrome is fairly easy to read;
with coloring, it might become too distracting, though.
Just a nit, "new mode" is too similar to "new". Everything is "new"
in the sense that they have "new contents"; it may be better phrased
without saying "new" but giving a stress on "changed".
Thanks for a fun patch. I am not strongly against it.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH UI experiment] diffstat: annotate/highlight new or removed files
2012-10-15 21:03 ` Junio C Hamano
@ 2012-10-16 11:15 ` Nguyen Thai Ngoc Duy
0 siblings, 0 replies; 3+ messages in thread
From: Nguyen Thai Ngoc Duy @ 2012-10-16 11:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Tue, Oct 16, 2012 at 4:03 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Coloring is to me an improvement over --summary. Probably the main
>> point. Without it, perhaps it's not worth putting extra text to
>> diffstat.
>
> It is kind of surprising that you did not choose to paint new in
> green and gone in red, and rather paint everything in yellow.
The colors of line addition and deletion? Nice. I just wanted to make
sure these stand out, or at least not easily mistaken as part of path
names.
> I personally think the above in monochrome is fairly easy to read;
> with coloring, it might become too distracting, though.
Hmm.. maybe. I'm probably too excited to see the distraction just yet.
I will try it out for longer time, see if I change my mind.
> Just a nit, "new mode" is too similar to "new". Everything is "new"
> in the sense that they have "new contents"; it may be better phrased
> without saying "new" but giving a stress on "changed".
I think "new mode" should be replaced by the actual mode change (e.g.
"+x", "-x", or "mode +x", "mode -x"). Not too long and quite clear
what it does.
--
Duy
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-10-16 11:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-15 14:35 [PATCH UI experiment] diffstat: annotate/highlight new or removed files Nguyễn Thái Ngọc Duy
2012-10-15 21:03 ` Junio C Hamano
2012-10-16 11:15 ` Nguyen Thai Ngoc Duy
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).