[PATCH v3] blame: add support for --[no-]progress option

[Edmundo Carmona Antoranz]

* created struct progress_info in builtin/blame.c
  this struct holds the information used to display progress so
  that only one additional parameter is passed to
  found_guilty_entry().

* --[no-]progress option is also inherited by git-annotate.

Signed-off-by: Edmundo Carmona Antoranz 
---
 Documentation/blame-options.txt |  7 +++
 Documentation/git-blame.txt |  3 ++-
 builtin/blame.c | 37 +
 3 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 760eab7..ef642b9 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -69,6 +69,13 @@ include::line-range-format.txt[]
iso format is used. For supported values, see the discussion
of the --date option at linkgit:git-log[1].
 
+--[no-]progress::
+   Progress status is reported on the standard error stream
+   by default when it is attached to a terminal. This flag
+   enables progress reporting even if not attached to a
+   terminal. Progress information won't be displayed if using
+   `--porcelain` or `--incremental`.
+
 -M||::
Detect moved or copied lines within a file. When a commit
moves or copies a block of lines (e.g. the original file
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index e6e947c..b0154c2 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 [verse]
 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] 
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
-   [--abbrev=] [ | --contents  | --reverse ] [--] 

+   [--[no-]progress] [--abbrev=] [ | --contents  | 
--reverse ]
+   [--] 
 
 DESCRIPTION
 ---
diff --git a/builtin/blame.c b/builtin/blame.c
index 83612f5..374ea7c 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -28,6 +28,7 @@
 #include "line-range.h"
 #include "line-log.h"
 #include "dir.h"
+#include "progress.h"
 
 static char blame_usage[] = N_("git blame [] [] [] 
[--] ");
 
@@ -50,6 +51,7 @@ static int incremental;
 static int xdl_opts;
 static int abbrev = -1;
 static int no_whole_file_rename;
+static int show_progress;
 
 static struct date_mode blame_date_mode = { DATE_ISO8601 };
 static size_t blame_date_width;
@@ -127,6 +129,11 @@ struct origin {
char path[FLEX_ARRAY];
 };
 
+struct progress_info {
+   struct progress *progress;
+   int blamed_lines;
+};
+
 static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b, long ctxlen,
  xdl_emit_hunk_consume_func_t hunk_func, void *cb_data)
 {
@@ -1746,7 +1753,8 @@ static int emit_one_suspect_detail(struct origin 
*suspect, int repeat)
  * The blame_entry is found to be guilty for the range.
  * Show it in incremental output.
  */
-static void found_guilty_entry(struct blame_entry *ent)
+static void found_guilty_entry(struct blame_entry *ent,
+  struct progress_info * pi)
 {
if (incremental) {
struct origin *suspect = ent->suspect;
@@ -1758,6 +1766,10 @@ static void found_guilty_entry(struct blame_entry *ent)
write_filename_info(suspect->path);
maybe_flush_or_die(stdout, "stdout");
}
+   if (show_progress) {
+   pi->blamed_lines += ent->num_lines;
+   display_progress(pi->progress, pi->blamed_lines);
+   }
 }
 
 /*
@@ -1768,6 +1780,13 @@ static void assign_blame(struct scoreboard *sb, int opt)
 {
struct rev_info *revs = sb->revs;
struct commit *commit = prio_queue_get(>commits);
+   struct progress_info pi;
+
+   if (show_progress) {
+   pi.progress = start_progress_delay(_("Blaming lines"),
+  sb->num_lines, 50, 1);
+   pi.blamed_lines = 0;
+   }
 
while (commit) {
struct blame_entry *ent;
@@ -1809,7 +1828,7 @@ static void assign_blame(struct scoreboard *sb, int opt)
suspect->guilty = 1;
for (;;) {
struct blame_entry *next = ent->next;
-   found_guilty_entry(ent);
+   found_guilty_entry(ent, );
if (next) {
ent = next;
continue;
@@ -1825,6 +1844,9 @@ static void assign_blame(struct scoreboard *sb, int opt)
if (DEBUG) /* sanity */
sanity_check_refcnt(sb);
}
+
+   if (show_progress)
+   stop_progress();
 }
 
 static const char *format_time(unsigned long time, const char *tz_str,
@@ -2520,6 +2542,7 @@ int cmd_blame(int argc, const char **argv, const char 
*prefix)
OPT_BOOL('b', NULL, _boundary, N_("Show 

Re: [PATCH v3] blame: add support for --[no-]progress option

[Edmundo Carmona Antoranz]

On Sun, Nov 22, 2015 at 11:12 PM, Edmundo Carmona Antoranz
 wrote:
> +struct progress_info {
> +   struct progress *progress;
> +   int blamed_lines;
> +};
> +

This is valid, right? Adding 2 more parameters to found_guilty_entry()
sounded like an overkill so I joined them into a new struct.

Thanks in advance.
--
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 v2] blame: add support for --[no-]progress option

[Junio C Hamano]

Edmundo Carmona Antoranz  writes:

> Will also affect annotate

Is that a good thing?  In any case, make it understandable without
the title line (i.e. make it a full sentence, ending with a full
stop).

> + if (progress) {
> + for (next = suspect->suspects; next != 
> NULL;
> +  next = next->next)
> + blamed_lines += next->num_lines;
> + display_progress(progress, 
> blamed_lines);
> + }

Is this math and the placement of the code correct?  It would
probably be more obvious if this hunk is in found_guilty_entry(),
which is already the dedicated function in which we report about a
group of lines whose ultimate origin has become clear.

> @@ -2830,11 +2851,11 @@ parse_done:
>  
>   read_mailmap(, NULL);
>  
> + assign_blame(, opt);
> +
>   if (!incremental)
>   setup_pager();
>  
> - assign_blame(, opt);
> -
>   free(final_commit_name);
>  
>   if (incremental)

Two comments.

 * How does this interact with incremental or porcelain blame?
   Shouldn't progress be turned off when these modes are in use?

 * Shouldn't progress be turned off if the result comes very
   quickly, using start_progress_delay()?

--
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 v2] blame: add support for --[no-]progress option

[Edmundo Carmona Antoranz]

Nice to see you back, Junio!


On Sun, Nov 22, 2015 at 8:08 PM, Junio C Hamano  wrote:
> Edmundo Carmona Antoranz  writes:
>
>> Will also affect annotate
>
> Is that a good thing?  In any case, make it understandable without
> the title line (i.e. make it a full sentence, ending with a full
> stop).
>

Will make the explanation a little more verbose. About being a bad
thing, I don't see how, it's just the same functionality. You think I
should turn it off if using annotate?

>> + if (progress) {
>> + for (next = suspect->suspects; next != 
>> NULL;
>> +  next = next->next)
>> + blamed_lines += 
>> next->num_lines;
>> + display_progress(progress, 
>> blamed_lines);
>> + }
>
> Is this math and the placement of the code correct?  It would
> probably be more obvious if this hunk is in found_guilty_entry(),
> which is already the dedicated function in which we report about a
> group of lines whose ultimate origin has become clear.
>

I'll see what I can do about it.

>
> Two comments.
>
>  * How does this interact with incremental or porcelain blame?
>Shouldn't progress be turned off when these modes are in use?

Given that they are supposed to be for machine consumption, I'll turn
progress off is using one of them.

>
>  * Shouldn't progress be turned off if the result comes very
>quickly, using start_progress_delay()?
>

Ok. Default values as in checkout? 50, 1?
--
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 v1] blame: add support for --[no-]progress option

[Edmundo Carmona Antoranz]

I think I found where to calculate the number of blamed lines without
having to do it from scratch every cycle. It's where the list of
sb->ent is getting its nodes pointed around here:

for (;;) {
struct blame_entry *next = ent->next;
found_guilty_entry(ent);
if (next) {
ent = next;
continue;
}
ent->next = sb->ent;
sb->ent = suspect->suspects;
suspect->suspects = NULL;
break;
}

So the function I added to blame.c, it's gone.
--
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 v3] ls-files: Add eol diagnostics

[Sebastian Schuberth]

On 21.11.2015 08:36, Torsten Bögershausen wrote:

> git ls-files --eol gives an output like this:
> 
> i/text-no-eol   w/text-no-eol   attr/text=auto t/t5100/empty

I'm sorry if this has been discussed before, but hav you considered to use a 
header line and omit the prefixed from the columns instead? Like

index working tree attributesfile

binarybinary   -text t/test-binary-2.png
text-lf   text-lf  eol=lft/t5100/rfc2047-info-0007
text-lf   text-crlfeol=crlf  doit.bat
text-crlf-lf  text-crlf-lf   locale/XX.po

I believe this would be both easier to read for humans, and easier to parse for 
scripts that e.g. want to compare line endings in the index and working tree.

> +stats_ascii () {
> + case "$1" in

[...]

> + *)
> + echo huh $1
> + ;;

Personally, I'm not a big fan of supposedly funny output like this. How about 
printing a proper message rather than "huh", even for cases that should not 
happen?

-- 
Sebastian Schuberth
--
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 v2] blame: add support for --[no-]progress option

[Edmundo Carmona Antoranz]

Will also affect annotate

Signed-off-by: Edmundo Carmona Antoranz 
---
 Documentation/blame-options.txt |  7 +++
 Documentation/git-blame.txt |  9 -
 builtin/blame.c | 25 +++--
 3 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 760eab7..43f4f08 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -69,6 +69,13 @@ include::line-range-format.txt[]
iso format is used. For supported values, see the discussion
of the --date option at linkgit:git-log[1].
 
+--[no-]progress::
+   Progress status is reported on the standard error stream
+   by default when it is attached to a terminal. This flag
+   enables progress reporting even if not attached to a
+   terminal.
+
+
 -M||::
Detect moved or copied lines within a file. When a commit
moves or copies a block of lines (e.g. the original file
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index e6e947c..2e63397 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 [verse]
 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] 
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
-   [--abbrev=] [ | --contents  | --reverse ] [--] 

+   [--[no-]progress] [--abbrev=] [ | --contents  | 
--reverse ]
+   [--] 
 
 DESCRIPTION
 ---
@@ -88,6 +89,12 @@ include::blame-options.txt[]
abbreviated object name, use +1 digits. Note that 1 column
is used for a caret to mark the boundary commit.
 
+--[no-]progress::
+   Progress status is reported on the standard error stream
+   by default when it is attached to a terminal. This flag
+   enables progress reporting even if not attached to a
+   terminal.
+
 
 THE PORCELAIN FORMAT
 
diff --git a/builtin/blame.c b/builtin/blame.c
index 83612f5..480bb2d 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -28,6 +28,7 @@
 #include "line-range.h"
 #include "line-log.h"
 #include "dir.h"
+#include "progress.h"
 
 static char blame_usage[] = N_("git blame [] [] [] 
[--] ");
 
@@ -50,6 +51,7 @@ static int incremental;
 static int xdl_opts;
 static int abbrev = -1;
 static int no_whole_file_rename;
+static int show_progress;
 
 static struct date_mode blame_date_mode = { DATE_ISO8601 };
 static size_t blame_date_width;
@@ -1768,6 +1770,11 @@ static void assign_blame(struct scoreboard *sb, int opt)
 {
struct rev_info *revs = sb->revs;
struct commit *commit = prio_queue_get(>commits);
+   struct progress * progress = NULL;
+   int blamed_lines = 0;
+
+   if (show_progress)
+   progress = start_progress(_("Blaming lines"), sb->num_lines);
 
while (commit) {
struct blame_entry *ent;
@@ -1814,6 +1821,12 @@ static void assign_blame(struct scoreboard *sb, int opt)
ent = next;
continue;
}
+   if (progress) {
+   for (next = suspect->suspects; next != 
NULL;
+next = next->next)
+   blamed_lines += next->num_lines;
+   display_progress(progress, 
blamed_lines);
+   }
ent->next = sb->ent;
sb->ent = suspect->suspects;
suspect->suspects = NULL;
@@ -1825,6 +1838,9 @@ static void assign_blame(struct scoreboard *sb, int opt)
if (DEBUG) /* sanity */
sanity_check_refcnt(sb);
}
+
+   if (progress)
+   stop_progress();
 }
 
 static const char *format_time(unsigned long time, const char *tz_str,
@@ -2520,6 +2536,7 @@ int cmd_blame(int argc, const char **argv, const char 
*prefix)
OPT_BOOL('b', NULL, _boundary, N_("Show blank SHA-1 for 
boundary commits (Default: off)")),
OPT_BOOL(0, "root", _root, N_("Do not treat root commits 
as boundaries (Default: off)")),
OPT_BOOL(0, "show-stats", _stats, N_("Show work cost 
statistics")),
+   OPT_BOOL(0, "progress", _progress, N_("Force progress 
reporting")),
OPT_BIT(0, "score-debug", _option, N_("Show output score 
for blame entries"), OUTPUT_SHOW_SCORE),
OPT_BIT('f', "show-name", _option, N_("Show original 
filename (Default: auto)"), OUTPUT_SHOW_NAME),
OPT_BIT('n', "show-number", _option, N_("Show original 
linenumber (Default: off)"), OUTPUT_SHOW_NUMBER),
@@ -2555,6 +2572,7 @@ int cmd_blame(int argc, const char **argv, const char 
*prefix)
 
   

Re: [PATCH v1] blame: add support for --[no-]progress option

[Edmundo Carmona Antoranz]

On Sat, Nov 21, 2015 at 11:27 PM, Edmundo Carmona Antoranz
 wrote:
> -static void assign_blame(struct scoreboard *sb, int opt)
> +static void assign_blame(struct scoreboard *sb, int opt, int show_progress)
>
> Would it be better to include show_progress as a binary flag in opt
> instead of a separate variable?

What is the point of having 'global' variable and not using it as such?

I'm sending a second version of the patch where I'm taking care of
most of my previous comments. Let's hope that one holds water.

Thanks in advance.
--
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: [RFC] OS X El Capitan + Xcode ships without SSL header?!

[Dair Grant]

> On 21 Nov 2015, at 18:58, Lars Schneider  wrote:
> 
> I installed OpenSSL with brew, added the include path and it works.
> 
> Can anyone confirm?

That is correct. Xcode’s copy of OpenSSL has been deprecated since 10.7 and was 
removed for 10.11 in Xcode 7:

   https://lists.apple.com/archives/macnetworkprog/2015/Jun/msg00025.html


-dair--
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 v2] blame: add support for --[no-]progress option

[Johannes Sixt]

Am 22.11.2015 um 17:02 schrieb Edmundo Carmona Antoranz:

Will also affect annotate

Signed-off-by: Edmundo Carmona Antoranz 
---
  Documentation/blame-options.txt |  7 +++
  Documentation/git-blame.txt |  9 -
  builtin/blame.c | 25 +++--
  3 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 760eab7..43f4f08 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -69,6 +69,13 @@ include::line-range-format.txt[]
iso format is used. For supported values, see the discussion
of the --date option at linkgit:git-log[1].

+--[no-]progress::
+   Progress status is reported on the standard error stream
+   by default when it is attached to a terminal. This flag
+   enables progress reporting even if not attached to a
+   terminal.
+
+
  -M||::
Detect moved or copied lines within a file. When a commit
moves or copies a block of lines (e.g. the original file
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index e6e947c..2e63397 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,8 @@ SYNOPSIS
  [verse]
  'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] 
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
-   [--abbrev=] [ | --contents  | --reverse ] [--] 

+   [--[no-]progress] [--abbrev=] [ | --contents  | --reverse 
]
+   [--] 


You add the option to to the synopsis of git-blame.txt, but not to 
git-annotate.txt.




  DESCRIPTION
  ---
@@ -88,6 +89,12 @@ include::blame-options.txt[]
abbreviated object name, use +1 digits. Note that 1 column
is used for a caret to mark the boundary commit.

+--[no-]progress::
+   Progress status is reported on the standard error stream
+   by default when it is attached to a terminal. This flag
+   enables progress reporting even if not attached to a
+   terminal.
+


Any particular reason you add this text twice? As can be seen on the 
hunk header, git-blame.txt includes blame-options.txt.


-- Hannes

--
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 v2] blame: add support for --[no-]progress option

[Edmundo Carmona Antoranz]

Hey, Johannes!

An oversight! git-annotate.txt took me to blame-options.txt and I
didn't notice it's also pointed to from git-blame.txt. Let's wait for
some comments about the coding (or blessings) so that I can send
another patch.

Thanks!

On Sun, Nov 22, 2015 at 1:58 PM, Johannes Sixt  wrote:
> Am 22.11.2015 um 17:02 schrieb Edmundo Carmona Antoranz:
>>
>> Will also affect annotate
>>
>> Signed-off-by: Edmundo Carmona Antoranz 
>> ---
>>   Documentation/blame-options.txt |  7 +++
>>   Documentation/git-blame.txt |  9 -
>>   builtin/blame.c | 25 +++--
>>   3 files changed, 38 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/blame-options.txt
>> b/Documentation/blame-options.txt
>> index 760eab7..43f4f08 100644
>> --- a/Documentation/blame-options.txt
>> +++ b/Documentation/blame-options.txt
>> @@ -69,6 +69,13 @@ include::line-range-format.txt[]
>> iso format is used. For supported values, see the discussion
>> of the --date option at linkgit:git-log[1].
>>
>> +--[no-]progress::
>> +   Progress status is reported on the standard error stream
>> +   by default when it is attached to a terminal. This flag
>> +   enables progress reporting even if not attached to a
>> +   terminal.
>> +
>> +
>>   -M||::
>> Detect moved or copied lines within a file. When a commit
>> moves or copies a block of lines (e.g. the original file
>> diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
>> index e6e947c..2e63397 100644
>> --- a/Documentation/git-blame.txt
>> +++ b/Documentation/git-blame.txt
>> @@ -10,7 +10,8 @@ SYNOPSIS
>>   [verse]
>>   'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w]
>> [--incremental]
>> [-L ] [-S ] [-M] [-C] [-C] [-C]
>> [--since=]
>> -   [--abbrev=] [ | --contents  | --reverse ]
>> [--] 
>> +   [--[no-]progress] [--abbrev=] [ | --contents  |
>> --reverse ]
>> +   [--] 
>
>
> You add the option to to the synopsis of git-blame.txt, but not to
> git-annotate.txt.
>
>>
>>   DESCRIPTION
>>   ---
>> @@ -88,6 +89,12 @@ include::blame-options.txt[]
>> abbreviated object name, use +1 digits. Note that 1 column
>> is used for a caret to mark the boundary commit.
>>
>> +--[no-]progress::
>> +   Progress status is reported on the standard error stream
>> +   by default when it is attached to a terminal. This flag
>> +   enables progress reporting even if not attached to a
>> +   terminal.
>> +
>
>
> Any particular reason you add this text twice? As can be seen on the hunk
> header, git-blame.txt includes blame-options.txt.
>
> -- Hannes
>
--
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: What's cooking in git.git (Nov 2015, #03; Fri, 20)

[Karthik Nayak]

>
> * kn/for-each-branch-remainder (2015-10-02) 9 commits
>  - branch: implement '--format' option
>  - branch: use ref-filter printing APIs
>  - ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
>  - ref-filter: introduce format_ref_array_item()
>  - ref-filter: adopt get_head_description() from branch.c
>  - ref-filter: modify "%(objectname:short)" to take length
>  - ref-filter: add support for %(path) atom
>  - ref-filter: implement %(if:equals=) and %(if:notequals=)
>  - ref-filter: implement %(if), %(then), and %(else) atoms
>
>  More unification among "branch -l", "tag -l" and "for-each-ref --format".
>
>  Expecting a reroll.
>  ($gmane/278926)
>

This series is supposed to follow this:
http://thread.gmane.org/gmane.comp.version-control.git/281180 which I
recently sent.
So replace "for-each-branch-remainder" with this in "Whats cooking"?

-- 
Regards,
Karthik Nayak
--
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