Re: [PATCH v3] diff.c: keep arrow(=) on show_stats()'s shortened filename part to make rename visible.
Hello Duy
Thank you very much your suggestion.
As you suggested, I tried to reuse intermediate result of pprint_rename(),
instead of
parsing the output again.
I just posted the new patch as PATCH v4
Thanks !
---
Tsuneo Yoshioka (吉岡 恒夫)
yoshiokatsu...@gmail.com
On Oct 14, 2013, at 10:04 PM, Duy Nguyen pclo...@gmail.com wrote:
On Sun, Oct 13, 2013 at 3:48 AM, Yoshioka Tsuneo
yoshiokatsu...@gmail.com wrote:
git diff -M --stat can detect rename and show renamed file name like
foofoofoo = barbarbar, but if destination filename is long the line
is shortened like ...barbarbar so there is no way to know whether the
file is renamed or existed in the source commit.
This commit makes it visible like ...foo = ...bar.
Signed-off-by: Tsuneo Yoshioka yoshiokatsu...@gmail.com
---
diff.c | 58 +++---
1 file changed, 51 insertions(+), 7 deletions(-)
diff --git a/diff.c b/diff.c
index a04a34d..3aeaf3e 100644
--- a/diff.c
+++ b/diff.c
@@ -1643,13 +1643,57 @@ static void show_stats(struct diffstat_t *data,
struct diff_options *options)
len = name_width;
name_len = strlen(name);
if (name_width name_len) {
- char *slash;
- prefix = ...;
- len -= 3;
- name += name_len - len;
- slash = strchr(name, '/');
- if (slash)
- name = slash;
+ char *arrow = strstr(name, = );
+ if (arrow) {
This looks iffy. What if = is part of the path name?
file-is_renamed would be a more reliable sign. In that case I think
you just need an ellipsis version of pprint_rename() (i.e. drop the
result of previous pprint_rename() on the floor and create a new
string with ... and = in your pprint_ellipsis_rename or
something)
+ int prefix_len = (name_width - 4) / 2;
+ int f_omit;
+ int f_brace = 0;
+ char *pre_arrow = alloca(name_width + 10);
+ char *post_arrow = arrow + 4;
+ char *prefix_buf = alloca(name_width + 10);
+ char *pre_arrow_slash = NULL;
+
+ if (arrow - name prefix_len) {
+ prefix_len = (int)(arrow - name);
+ f_omit = 0;
+ } else {
+ prefix_len -= 3;
+ f_omit = 1;
+ if (name[0] == '{') {
+ prefix_len -= 1;
+ f_brace = 1;
+ }
+ }
+ prefix_len = ((prefix_len = 0) ? prefix_len
: 0);
+ strncpy(pre_arrow, arrow - prefix_len,
prefix_len);
+ pre_arrow[prefix_len] = '\0';
+ pre_arrow_slash = strchr(pre_arrow, '/');
+ if (f_omit pre_arrow_slash)
+ pre_arrow = pre_arrow_slash;
+ sprintf(prefix_buf, %s%s%s = , (f_brace ?
{ : ), (f_omit ? ... : ), pre_arrow);
+ prefix = prefix_buf;
+
+ if (strlen(post_arrow) name_width -
strlen(prefix)) {
+ char *post_arrow_slash = NULL;
+
+ post_arrow += strlen(post_arrow) -
(name_width - strlen(prefix) - 3);
+ strcat(prefix_buf, ...);
+ post_arrow_slash =
strchr(post_arrow, '/');
+ if (post_arrow_slash)
+ post_arrow =
post_arrow_slash;
+ name = post_arrow;
+ name_len = (int) (name_width -
strlen(prefix));
+ }
+ len -= strlen(prefix);
+ } else {
+ char *slash = NULL;
+ prefix = ...;
+ len -= 3;
+ name += name_len - len;
+ slash = strchr(name, '/');
+ if (slash)
+ name = slash;
+ }
}
if (file-is_binary) {
--
1.8.4.475.g867697c
--
To unsubscribe from this list:
Re: [PATCH v3] diff.c: keep arrow(=) on show_stats()'s shortened filename part to make rename visible.
On Sun, Oct 13, 2013 at 3:48 AM, Yoshioka Tsuneo
yoshiokatsu...@gmail.com wrote:
git diff -M --stat can detect rename and show renamed file name like
foofoofoo = barbarbar, but if destination filename is long the line
is shortened like ...barbarbar so there is no way to know whether the
file is renamed or existed in the source commit.
This commit makes it visible like ...foo = ...bar.
Signed-off-by: Tsuneo Yoshioka yoshiokatsu...@gmail.com
---
diff.c | 58 +++---
1 file changed, 51 insertions(+), 7 deletions(-)
diff --git a/diff.c b/diff.c
index a04a34d..3aeaf3e 100644
--- a/diff.c
+++ b/diff.c
@@ -1643,13 +1643,57 @@ static void show_stats(struct diffstat_t *data,
struct diff_options *options)
len = name_width;
name_len = strlen(name);
if (name_width name_len) {
- char *slash;
- prefix = ...;
- len -= 3;
- name += name_len - len;
- slash = strchr(name, '/');
- if (slash)
- name = slash;
+ char *arrow = strstr(name, = );
+ if (arrow) {
This looks iffy. What if = is part of the path name?
file-is_renamed would be a more reliable sign. In that case I think
you just need an ellipsis version of pprint_rename() (i.e. drop the
result of previous pprint_rename() on the floor and create a new
string with ... and = in your pprint_ellipsis_rename or
something)
+ int prefix_len = (name_width - 4) / 2;
+ int f_omit;
+ int f_brace = 0;
+ char *pre_arrow = alloca(name_width + 10);
+ char *post_arrow = arrow + 4;
+ char *prefix_buf = alloca(name_width + 10);
+ char *pre_arrow_slash = NULL;
+
+ if (arrow - name prefix_len) {
+ prefix_len = (int)(arrow - name);
+ f_omit = 0;
+ } else {
+ prefix_len -= 3;
+ f_omit = 1;
+ if (name[0] == '{') {
+ prefix_len -= 1;
+ f_brace = 1;
+ }
+ }
+ prefix_len = ((prefix_len = 0) ? prefix_len
: 0);
+ strncpy(pre_arrow, arrow - prefix_len,
prefix_len);
+ pre_arrow[prefix_len] = '\0';
+ pre_arrow_slash = strchr(pre_arrow, '/');
+ if (f_omit pre_arrow_slash)
+ pre_arrow = pre_arrow_slash;
+ sprintf(prefix_buf, %s%s%s = , (f_brace ?
{ : ), (f_omit ? ... : ), pre_arrow);
+ prefix = prefix_buf;
+
+ if (strlen(post_arrow) name_width -
strlen(prefix)) {
+ char *post_arrow_slash = NULL;
+
+ post_arrow += strlen(post_arrow) -
(name_width - strlen(prefix) - 3);
+ strcat(prefix_buf, ...);
+ post_arrow_slash = strchr(post_arrow,
'/');
+ if (post_arrow_slash)
+ post_arrow = post_arrow_slash;
+ name = post_arrow;
+ name_len = (int) (name_width -
strlen(prefix));
+ }
+ len -= strlen(prefix);
+ } else {
+ char *slash = NULL;
+ prefix = ...;
+ len -= 3;
+ name += name_len - len;
+ slash = strchr(name, '/');
+ if (slash)
+ name = slash;
+ }
}
if (file-is_binary) {
--
1.8.4.475.g867697c
--
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
--
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
Re: [PATCH v3] diff.c: keep arrow(=) on show_stats()'s shortened filename part to make rename visible.
Hi,
Yoshioka Tsuneo yoshiokatsu...@gmail.com writes:
git diff -M --stat can detect rename and show renamed file name like
foofoofoo = barbarbar, but if destination filename is long the line
is shortened like ...barbarbar so there is no way to know whether the
file is renamed or existed in the source commit.
Thanks for your patch! I think this is indeed something that should be
fixed.
Can you explain the algorithm chosen in the commit message or a block
comment in the code? I find it much easier to follow large code blocks
(like the one you added) with a prior notion of what it tries to do.
[As an aside, Documentation/SubmittingPatches says
The body should provide a meaningful commit message, which:
. explains the problem the change tries to solve, iow, what is wrong
with the current code without the change.
. justifies the way the change solves the problem, iow, why the
result with the change is better.
. alternate solutions considered but discarded, if any.
Observe that you explained the first item very well, but not the
others.]
This commit makes it visible like ...foo = ...bar.
Also, you should rewrite this to be in the imperative mood:
Make sure there is always an arrow, e.g., ...foo = ...bar.
or some such.
[Again from SubmittingPatches:
Describe your changes in imperative mood, e.g. make xyzzy do frotz
instead of [This patch] makes xyzzy do frotz or [I] changed xyzzy
to do frotz, as if you are giving orders to the codebase to change
its behaviour.]
Signed-off-by: Tsuneo Yoshioka yoshiokatsu...@gmail.com
---
diff.c | 58 +++---
1 file changed, 51 insertions(+), 7 deletions(-)
Can you add a test? Perhaps like the one below. (You can squash it
into your commit if you like it.)
Note that in the test, the generated line looks like this:
{..._does_not_fit_in_a_single_line = .../path1 | 0
I don't want to go all bikesheddey, but I think it's somewhat
unfortunate that the elided parts do not correspond to each other. In
particular, I think the closing brace should not be omitted. Perhaps
something like this would be ideal (making it up on the spot, don't
count characters):
{...a_single_line = ..._as_the_first}/path1 | 0
diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
index 2f327b7..03d6371 100755
--- a/t/t4001-diff-rename.sh
+++ b/t/t4001-diff-rename.sh
@@ -156,4 +156,16 @@ test_expect_success 'rename pretty print common prefix and
suffix overlap' '
test_i18ngrep d/f/{ = f}/e output
'
+test_expect_success 'rename of very long path shows =' '
+ mkdir long_dirname_that_does_not_fit_in_a_single_line
+ mkdir another_extremely_long_path_but_not_the_same_as_the_first
+ cp path1 long_dirname*/
+ git add long_dirname*/path1
+ test_commit add_long_pathname
+ git mv long_dirname*/path1 another_extremely_*/
+ test_commit move_long_pathname
+ git diff -M --stat HEAD^ HEAD output
+ test_i18ngrep =.*path1 output
+'
+
test_done
--
Thomas Rast
t...@thomasrast.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
[PATCH v3] diff.c: keep arrow(=) on show_stats()'s shortened filename part to make rename visible.
git diff -M --stat can detect rename and show renamed file name like
foofoofoo = barbarbar, but if destination filename is long the line
is shortened like ...barbarbar so there is no way to know whether the
file is renamed or existed in the source commit.
This commit makes it visible like ...foo = ...bar.
Signed-off-by: Tsuneo Yoshioka yoshiokatsu...@gmail.com
---
diff.c | 58 +++---
1 file changed, 51 insertions(+), 7 deletions(-)
diff --git a/diff.c b/diff.c
index a04a34d..3aeaf3e 100644
--- a/diff.c
+++ b/diff.c
@@ -1643,13 +1643,57 @@ static void show_stats(struct diffstat_t *data, struct
diff_options *options)
len = name_width;
name_len = strlen(name);
if (name_width name_len) {
- char *slash;
- prefix = ...;
- len -= 3;
- name += name_len - len;
- slash = strchr(name, '/');
- if (slash)
- name = slash;
+ char *arrow = strstr(name, = );
+ if (arrow) {
+ int prefix_len = (name_width - 4) / 2;
+ int f_omit;
+ int f_brace = 0;
+ char *pre_arrow = alloca(name_width + 10);
+ char *post_arrow = arrow + 4;
+ char *prefix_buf = alloca(name_width + 10);
+ char *pre_arrow_slash = NULL;
+
+ if (arrow - name prefix_len) {
+ prefix_len = (int)(arrow - name);
+ f_omit = 0;
+ } else {
+ prefix_len -= 3;
+ f_omit = 1;
+ if (name[0] == '{') {
+ prefix_len -= 1;
+ f_brace = 1;
+ }
+ }
+ prefix_len = ((prefix_len = 0) ? prefix_len :
0);
+ strncpy(pre_arrow, arrow - prefix_len,
prefix_len);
+ pre_arrow[prefix_len] = '\0';
+ pre_arrow_slash = strchr(pre_arrow, '/');
+ if (f_omit pre_arrow_slash)
+ pre_arrow = pre_arrow_slash;
+ sprintf(prefix_buf, %s%s%s = , (f_brace ?
{ : ), (f_omit ? ... : ), pre_arrow);
+ prefix = prefix_buf;
+
+ if (strlen(post_arrow) name_width -
strlen(prefix)) {
+ char *post_arrow_slash = NULL;
+
+ post_arrow += strlen(post_arrow) -
(name_width - strlen(prefix) - 3);
+ strcat(prefix_buf, ...);
+ post_arrow_slash = strchr(post_arrow,
'/');
+ if (post_arrow_slash)
+ post_arrow = post_arrow_slash;
+ name = post_arrow;
+ name_len = (int) (name_width -
strlen(prefix));
+ }
+ len -= strlen(prefix);
+ } else {
+ char *slash = NULL;
+ prefix = ...;
+ len -= 3;
+ name += name_len - len;
+ slash = strchr(name, '/');
+ if (slash)
+ name = slash;
+ }
}
if (file-is_binary) {
--
1.8.4.475.g867697c
--
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
[PATCH v3] diff.c: keep arrow(=) on show_stats()'s shortened filename part to make rename visible.
git diff -M --stat can detect rename and show renamed file name like
foofoofoo = barbarbar, but if destination filename is long the line
is shortened like ...barbarbar so there is no way to know whether the
file is renamed or existed in the source commit.
This commit makes it visible like ...foo = ...bar.
Signed-off-by: Tsuneo Yoshioka yoshiokatsu...@gmail.com
---
diff.c | 58 +++---
1 file changed, 51 insertions(+), 7 deletions(-)
diff --git a/diff.c b/diff.c
index a04a34d..3aeaf3e 100644
--- a/diff.c
+++ b/diff.c
@@ -1643,13 +1643,57 @@ static void show_stats(struct diffstat_t *data, struct
diff_options *options)
len = name_width;
name_len = strlen(name);
if (name_width name_len) {
- char *slash;
- prefix = ...;
- len -= 3;
- name += name_len - len;
- slash = strchr(name, '/');
- if (slash)
- name = slash;
+ char *arrow = strstr(name, = );
+ if (arrow) {
+ int prefix_len = (name_width - 4) / 2;
+ int f_omit;
+ int f_brace = 0;
+ char *pre_arrow = alloca(name_width + 10);
+ char *post_arrow = arrow + 4;
+ char *prefix_buf = alloca(name_width + 10);
+ char *pre_arrow_slash = NULL;
+
+ if (arrow - name prefix_len) {
+ prefix_len = (int)(arrow - name);
+ f_omit = 0;
+ } else {
+ prefix_len -= 3;
+ f_omit = 1;
+ if (name[0] == '{') {
+ prefix_len -= 1;
+ f_brace = 1;
+ }
+ }
+ prefix_len = ((prefix_len = 0) ? prefix_len :
0);
+ strncpy(pre_arrow, arrow - prefix_len,
prefix_len);
+ pre_arrow[prefix_len] = '\0';
+ pre_arrow_slash = strchr(pre_arrow, '/');
+ if (f_omit pre_arrow_slash)
+ pre_arrow = pre_arrow_slash;
+ sprintf(prefix_buf, %s%s%s = , (f_brace ?
{ : ), (f_omit ? ... : ), pre_arrow);
+ prefix = prefix_buf;
+
+ if (strlen(post_arrow) name_width -
strlen(prefix)) {
+ char *post_arrow_slash = NULL;
+
+ post_arrow += strlen(post_arrow) -
(name_width - strlen(prefix) - 3);
+ strcat(prefix_buf, ...);
+ post_arrow_slash = strchr(post_arrow,
'/');
+ if (post_arrow_slash)
+ post_arrow = post_arrow_slash;
+ name = post_arrow;
+ name_len = (int) (name_width -
strlen(prefix));
+ }
+ len -= strlen(prefix);
+ } else {
+ char *slash = NULL;
+ prefix = ...;
+ len -= 3;
+ name += name_len - len;
+ slash = strchr(name, '/');
+ if (slash)
+ name = slash;
+ }
}
if (file-is_binary) {
--
1.8.4.475.g867697c
--
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
